Skip to content

Commit

Permalink
Fix adding disks to transient VMs
Browse files Browse the repository at this point in the history
TestMachinesDisks.testAddDiskDirPool's "transient VM" test case
ommediately opened the "Add Disk" dialog after undefining the VM. This
was a race condition which the test actually "won" in most cases: The
`UNDEFINED` event did not yet bubble through the UI, so the "Add Disk"
dialog still thought that the VM was persistent. Fix that by waiting
until the UI picked up the event and considers the VM transient.

This exposed a bug in the dialog's fill() function: It tried to check
the value of the "Permanent" checkbox, but that doesn't exist at all for
transient VMs. Fix up the logic and assert the latter for transient VMs.

Now that this test is working correctly, it uncovered a bug in the code:
Trying to add a disk to a transient VM fails with

> Requested operation is not valid: transient domains do not have any persistent config

because the `persistent` state was initialized to "true". Fix that.

https://issues.redhat.com/browse/RHEL-17677
  • Loading branch information
martinpitt committed Aug 23, 2024
1 parent a69ae8e commit d3bfeac
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 3 deletions.
2 changes: 1 addition & 1 deletion src/components/vm/disks/diskAdd.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ export const AddDiskModalBody = ({ disk, idPrefix, isMediaInsertion, vm, vms, su
size: 1,
unit: units.GiB.name,
volumeName: "",
permanent: true,
permanent: vm.persistent,
});
const [mode, setMode] = useState(isMediaInsertion ? CUSTOM_PATH : CREATE_NEW);
const [validate, setValidate] = useState(false);
Expand Down
10 changes: 8 additions & 2 deletions test/check-machines-disks
Original file line number Diff line number Diff line change
Expand Up @@ -640,8 +640,12 @@ class TestMachinesDisks(machineslib.VirtualMachinesCase):
b.select_from_dropdown(f"#vm-{self.vm_name}-disks-adddisk-existing-select-volume", self.volume_name)

# Configure persistency - by default the check box is checked
if not self.permanent:
b.set_checked(f"#vm-{self.vm_name}-disks-adddisk-permanent", False)
if self.persistent_vm:
if not self.permanent:
b.set_checked(f"#vm-{self.vm_name}-disks-adddisk-permanent", False)
else:
# checkbox not shown at all for transient VM
b.wait_not_present(f"#vm-{self.vm_name}-disks-adddisk-permanent")

# Check non-persistent VM cannot have permanent disk attached
if not self.persistent_vm:
Expand Down Expand Up @@ -1107,7 +1111,9 @@ class TestMachinesDisks(machineslib.VirtualMachinesCase):
b.click("#vm-subVmTest1-system-run")
b.wait_in_text("#vm-subVmTest1-system-state", "Running")
# Test disk attachment to non-persistent VM
b.wait_visible(".vm-top-panel[data-vm-transient=false]")
m.execute("virsh undefine subVmTest1")
b.wait_visible(".vm-top-panel[data-vm-transient=true]")
self.VMAddDiskDialog(
self,
pool_name='myPoolOne',
Expand Down

0 comments on commit d3bfeac

Please sign in to comment.