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..93067d816649 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 @@ -25,10 +25,12 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; -import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Objects; +import java.util.Set; import org.apache.cloudstack.backup.BackupAnswer; import org.apache.cloudstack.backup.RestoreBackupCommand; @@ -431,23 +433,30 @@ 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" }; - // 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); - // 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(); - if (result.first() == null || result.first() != 0 || StringUtils.isBlank(currentDevice)) { - throw new CloudRuntimeException(String.format("Failed to determine the device to attach the restored volume to on VM [%s].", vmName)); + // virsh runs on its own so its exit code is checked; behind a pipe only the last command's is. + // --details adds the Device column (disk/cdrom) used below to skip the cdrom slots. + String[] domblkCmd = new String[] { Script.getExecutableAbsolutePath("virsh"), "domblklist", "--domain", vmName, "--details" }; + Pair result = Script.executePipedCommands(Collections.singletonList(domblkCmd), 0); + if (result.first() == null || result.first() != 0 || result.second() == null) { + throw new CloudRuntimeException(String.format("Failed to list the disks of VM [%s] to determine the device to attach the restored volume to.", vmName)); } - char lastChar = currentDevice.charAt(currentDevice.length() - 1); - char incrementedChar = (char) (lastChar + 1); - return currentDevice.substring(0, currentDevice.length() - 1) + incrementedChar; + // Rows are " ". Only virtio (vd*) targets are collected: IDE/SATA + // disks cannot be hot-plugged. domblklist follows the domain XML order, where hot-plugged disks + // are appended, so the last target is not necessarily the highest one; pick the first free name. + Set usedDevices = new HashSet<>(); + for (String line : result.second().split("\\R")) { + String[] columns = line.trim().split("\\s+"); + if (columns.length >= 3 && "disk".equals(columns[1]) && columns[2].startsWith("vd")) { + usedDevices.add(columns[2]); + } + } + for (char c = 'a'; c <= 'z'; c++) { + String device = "vd" + c; + if (!usedDevices.contains(device)) { + return device; + } + } + throw new CloudRuntimeException(String.format("No free virtio device is left to attach the restored volume to on VM [%s].", vmName)); } private String getXmlForRbdDisk(KVMStoragePoolManager storagePoolMgr, PrimaryDataStoreTO volumePool, String volumePath, String deviceToAttachDiskTo) { 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..8d433874be3e 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 @@ -30,6 +30,7 @@ import java.nio.file.Path; import java.util.Arrays; import java.util.List; +import java.util.concurrent.atomic.AtomicReference; import org.apache.cloudstack.backup.BackupAnswer; import org.apache.cloudstack.backup.RestoreBackupCommand; @@ -628,42 +629,78 @@ private String[] captureAttachCommand(Storage.StoragePoolType poolType) throws E return captured[0]; } - @Test - public void testGetDeviceToAttachDiskTrimsOutputBeforeIncrementing() throws Exception { + private static String domblklistOutput(String... rows) { + String sep = System.lineSeparator(); + StringBuilder output = new StringBuilder(" Type Device Target Source").append(sep) + .append("------------------------------------------------").append(sep); + for (String row : rows) { + output.append(row).append(sep); + } + return output.toString(); + } + + private String getDeviceToAttachDiskFor(String domblklistOutput) throws Exception { try (MockedStatic