Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disk add dialog: Change format together with the storage pool #1331

Merged
merged 2 commits into from
Nov 29, 2023
Merged

Disk add dialog: Change format together with the storage pool #1331

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5704536
test: Fix device race condition in testAddDiskCustomPath
martinpitt Nov 28, 2023
eaf797d
Disk add dialog: Change format together with the storage pool
martinpitt Nov 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 22 additions & 13 deletions src/components/vm/disks/diskAdd.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,19 @@ export const AddDiskModalBody = ({ disk, idPrefix, isMediaInsertion, vm, vms, su
}), [defaultPool, mode, vm]);
const storagePoolName = diskParams.storagePool?.name;

const getPoolFormatAndDevice = (pool, volName) => {
const params = { format: getDefaultVolumeFormat(pool), device: "disk" };
if (['dir', 'fs', 'netfs', 'gluster', 'vstorage'].indexOf(pool.type) > -1) {
const volume = pool.volumes.find(vol => vol.name === volName);
if (volume?.format) {
params.format = volume.format;
if (volume.format === "iso")
params.device = "cdrom";
}
}
return params;
};

useEffect(() => {
// Refresh storage volume list before displaying the dialog.
// There are recently no Libvirt events for storage volumes and polling is ugly.
Expand Down Expand Up @@ -423,20 +436,9 @@ export const AddDiskModalBody = ({ disk, idPrefix, isMediaInsertion, vm, vms, su
return;
}

let format = getDefaultVolumeFormat(diskParams.storagePool);
let deviceType = "disk";
if (['dir', 'fs', 'netfs', 'gluster', 'vstorage'].indexOf(diskParams.storagePool.type) > -1) {
const volume = diskParams.storagePool.volumes.find(vol => vol.name === diskParams.existingVolumeName);
if (volume && volume.format) {
format = volume.format;
if (volume.format === "iso")
deviceType = "cdrom";
}
}
setDiskParams(diskParams => ({
...diskParams,
format,
device: deviceType,
...getPoolFormatAndDevice(diskParams.storagePool, diskParams.existingVolumeName),
}));
}, [storagePools, diskParams.storagePool, diskParams.existingVolumeName]);

Expand Down Expand Up @@ -513,7 +515,14 @@ export const AddDiskModalBody = ({ disk, idPrefix, isMediaInsertion, vm, vms, su
switch (key) {
case 'storagePoolName': {
const currentPool = storagePools.find(pool => pool.name === value && pool.connectionName === vm.connectionName);
setDiskParams(diskParams => ({ ...diskParams, storagePool: currentPool, existingVolumeName: getDefaultVolumeName(currentPool, vm) }));
const existingVolumeName = getDefaultVolumeName(currentPool, vm);

setDiskParams(diskParams => ({
...diskParams,
storagePool: currentPool,
existingVolumeName,
...getPoolFormatAndDevice(currentPool, existingVolumeName),
}));
break;
}
case 'mode': {
Expand Down
9 changes: 3 additions & 6 deletions test/check-machines-disks
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,9 @@ class TestMachinesDisks(VirtualMachinesCase):
# Type in file path
b.set_file_autocomplete_val(f"#vm-{self.vm_name}-disks-adddisk-file", self.file_path)
if self.device:
if self.file_path.endswith(".iso"):
# wait for auto-detection to finish
b.wait_val(f"#vm-{self.vm_name}-disks-adddisk-select-device", "cdrom")
b.select_from_dropdown(f"#vm-{self.vm_name}-disks-adddisk-select-device", self.device)
elif self.mode == "use-existing":
b.wait_visible(f"#vm-{self.vm_name}-disks-adddisk-existing-select-pool:enabled")
Expand Down Expand Up @@ -950,12 +953,6 @@ class TestMachinesDisks(VirtualMachinesCase):
if m.image in ["debian-testing"]:
self.allow_journal_messages(f'.* type=1400 .* apparmor="DENIED" operation="open" profile="libvirt.* name="{self.vm_tmpdir}.*')

# HACK: Switching the pool relies on useEffect() to update "Format:" to the default format of the new pool
# type; there is an intermediade rendering with the new pool and the old format. This is unfortunately
# hard to fix due to the useEffect() maze in the "Add disk" dialog. Until we clean this up, ignore this
# particular instance
self.allow_browser_errors("VolumeDetails internal error: format qcow2 is not valid for storage pool type disk")

@timeout(900)
def testAddDiskDirPool(self):
b = self.browser
Expand Down