Skip to content

Commit c737927

Browse files
committed
address copilot comments
1 parent 132f351 commit c737927

2 files changed

Lines changed: 10 additions & 5 deletions

File tree

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,8 @@ private boolean attachRbdVolumeToVm(KVMStoragePoolManager storagePoolMgr, String
432432

433433
private String getDeviceToAttachDisk(String vmName) {
434434
// --details adds a Type column so cdrom slots (hdc, hdd) can be filtered out — they sort
435-
// after the real disks, so without this the last row is always a cdrom, not a free device.
435+
// alphabetically ahead of virtio disks, so without this the selected row is a cdrom, not
436+
// an actual disk.
436437
String[] domblkCmd = new String[] { Script.getExecutableAbsolutePath("virsh"), "domblklist", "--domain", vmName, "--details" };
437438
// The commands are executed without a shell, so the awk program must be passed as a plain
438439
// argument. Keeping the quotes a shell would have stripped makes awk fail with

‎plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import java.nio.file.Path;
3131
import java.util.Arrays;
3232
import java.util.List;
33+
import java.util.concurrent.atomic.AtomicReference;
3334

3435
import org.apache.cloudstack.backup.BackupAnswer;
3536
import org.apache.cloudstack.backup.RestoreBackupCommand;
@@ -663,20 +664,23 @@ public void testGetDeviceToAttachDiskPassesUnquotedAwkProgram() throws Exception
663664
}
664665

665666
@Test
666-
public void testGetDeviceToAttachDiskFiltersOutCdromEntries() throws Exception {
667+
public void testGetDeviceToAttachDiskRequestsDomblklistDetails() throws Exception {
667668
try (MockedStatic<Script> scriptMock = mockStatic(Script.class)) {
668669
scriptMock.when(() -> Script.getExecutableAbsolutePath(anyString()))
669670
.thenAnswer(invocation -> invocation.getArgument(0));
670-
final List<String[]>[] captured = new List[1];
671+
final AtomicReference<List<String[]>> captured = new AtomicReference<>();
671672
scriptMock.when(() -> Script.executePipedCommands(anyList(), anyLong()))
672673
.thenAnswer(invocation -> {
673-
captured[0] = invocation.getArgument(0);
674+
captured.set(invocation.getArgument(0));
674675
return new Pair<>(0, "vdb" + System.lineSeparator());
675676
});
676677

677678
Assert.assertEquals("vdc", invokeGetDeviceToAttachDisk("test-vm"));
678679

679-
String[] domblkCmd = captured[0].get(0);
680+
// --details adds the Type column the awk program filters on (verified separately in
681+
// testGetDeviceToAttachDiskPassesUnquotedAwkProgram); without it cdrom rows can't be
682+
// told apart from real disks at all.
683+
String[] domblkCmd = captured.get().get(0);
680684
Assert.assertTrue("domblklist must request --details so the Type column is available to filter on",
681685
Arrays.asList(domblkCmd).contains("--details"));
682686
}

0 commit comments

Comments
 (0)