From 132f351df3037d015f02f9b79f68d0cfbdc605b5 Mon Sep 17 00:00:00 2001 From: Slavka Peleva Date: Tue, 29 Sep 2026 14:25:36 +0300 Subject: [PATCH 1/2] kvm: fix restore-attach picking a CD-ROM slot as the target device MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getDeviceToAttachDisk found the next free disk device by piping `virsh domblklist` through `tail -n 3 | head -n 1`, just grabbing a row by position without checking what device it actually was. domblklist sorts targets alphabetically, so a VM's empty IDE cdrom slots can sort after the real disks and get picked up instead — e.g. selecting hdc and incrementing it to hdd, an already-existing IDE target, instead of the next free virtio disk. This made hot-attaching a restored volume to a running VM fail with virsh attach-disk erroring out on the bogus target. This surfaced after #13101 ("Multiple CD-ROM / ISO Support Per VM"), which pre-allocates a second empty cdrom slot (hdc, hdd) at boot for every VM. With two cdrom rows now sorting ahead of the VM's disks, the tail/head window shifted enough to land on a cdrom row instead of a real disk — before that change, a single cdrom slot didn't push the window that far. Add --details to domblklist and filter to rows with Type=="disk" before taking the last one, so cdrom slots are excluded and the device name is always derived from an actual disk. --- .../LibvirtRestoreBackupCommandWrapper.java | 11 +++++---- ...ibvirtRestoreBackupCommandWrapperTest.java | 24 +++++++++++++++++-- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java index ccd0ec634525..4101be22d0f5 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java @@ -431,14 +431,15 @@ private boolean attachRbdVolumeToVm(KVMStoragePoolManager storagePoolMgr, String } private String getDeviceToAttachDisk(String vmName) { - String[] domblkCmd = new String[] { Script.getExecutableAbsolutePath("virsh"), "domblklist", "--domain", vmName }; - String[] tailCmd = new String[] { Script.getExecutableAbsolutePath("tail"), "-n", "3" }; - String[] headCmd = new String[] { Script.getExecutableAbsolutePath("head"), "-n", "1" }; + // --details adds a Type column so cdrom slots (hdc, hdd) can be filtered out — they sort + // after the real disks, so without this the last row is always a cdrom, not a free device. + String[] domblkCmd = new String[] { Script.getExecutableAbsolutePath("virsh"), "domblklist", "--domain", vmName, "--details" }; // The commands are executed without a shell, so the awk program must be passed as a plain // argument. Keeping the quotes a shell would have stripped makes awk fail with // "invalid char" and produce no output. - String[] awkCmd = new String[] { Script.getExecutableAbsolutePath("awk"), "{print $1}" }; - Pair result = Script.executePipedCommands(Arrays.asList(domblkCmd, tailCmd, headCmd, awkCmd), 0); + String[] awkCmd = new String[] { Script.getExecutableAbsolutePath("awk"), "$2==\"disk\"{print $3}" }; + String[] tailCmd = new String[] { Script.getExecutableAbsolutePath("tail"), "-n", "1" }; + Pair result = Script.executePipedCommands(Arrays.asList(domblkCmd, awkCmd, tailCmd), 0); // executePipedCommands appends a line separator to every line it reads, so the device // name has to be trimmed before the last character can be incremented. String currentDevice = result.second() == null ? "" : result.second().trim(); diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java index 3bdc23d27a15..9e9d52049478 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java @@ -655,10 +655,30 @@ public void testGetDeviceToAttachDiskPassesUnquotedAwkProgram() throws Exception invokeGetDeviceToAttachDisk("test-vm"); - String[] awkCmd = captured[0].get(captured[0].size() - 1); + String[] awkCmd = captured[0].get(1); // The commands are executed without a shell, so the program must carry no shell quotes. Assert.assertEquals("awk", awkCmd[0]); - Assert.assertEquals("{print $1}", awkCmd[1]); + Assert.assertEquals("$2==\"disk\"{print $3}", awkCmd[1]); + } + } + + @Test + public void testGetDeviceToAttachDiskFiltersOutCdromEntries() throws Exception { + try (MockedStatic