Skip to content

Commit

Permalink
machines: Fix default volume format detection in Disk Add dialog
Browse files Browse the repository at this point in the history
Previously we were correctly calling the getDefaultVolumeFormat when
opening the dialog, thus the default format for the preselected pool was
correctly detected. However, when a user changed so some other pool,
we didn't re-detect the default format, which was problematic if the
user didn't change the preseleted format afterwards.

This commit also adjusts the relevant test which didn't fail before
because the problematic pool was first on the list so the
getDefaultVolumeFormat was correctly called in the dialog constructor.

Relevant to https://bugzilla.redhat.com/show_bug.cgi?id=1833198

Closes #14201
Cherry-picked from master commit 2cd9bf.
  • Loading branch information
KKoukiou authored and martinpitt committed Jun 6, 2020
1 parent 0eb98c4 commit 93ab723
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 6 deletions.
10 changes: 9 additions & 1 deletion pkg/machines/components/diskAdd.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -428,10 +428,19 @@ class AddDiskModalBody extends React.Component {

onValueChanged(key, value) {
let stateDelta = {};
const { storagePools, vm } = this.props;

switch (key) {
case 'storagePoolName': {
const currentPool = storagePools.find(pool => pool.name === value && pool.connectionName === vm.connectionName);
const prevPool = storagePools.find(pool => pool.name === this.state.storagePoolName && pool.connectionName === vm.connectionName);

this.setState({ storagePoolName: value });
// Reset the format only when the Format selection dropdown changes entries - otherwise just keep the old selection
// All pool types apart from 'disk' have either 'raw' or 'qcow2' format
if ((currentPool.type == 'disk' && prevPool.type != 'disk') || (currentPool.type != 'disk' && prevPool.type == 'disk'))
this.onValueChanged('format', this.getDefaultVolumeFormat(value));

if (this.state.mode === USE_EXISTING) { // user changed pool
this.onValueChanged('existingVolumeName', this.getDefaultVolumeName(value));
}
Expand All @@ -440,7 +449,6 @@ class AddDiskModalBody extends React.Component {
case 'existingVolumeName': {
stateDelta.existingVolumeName = value;
this.setState(prevState => { // to prevent asynchronous for recursive call with existingVolumeName as a key
const { storagePools, vm } = this.props;
const pool = storagePools.find(pool => pool.name === prevState.storagePoolName && pool.connectionName === vm.connectionName);
stateDelta.diskFileFormat = this.getDefaultVolumeFormat(pool);
if (['dir', 'fs', 'netfs', 'gluster', 'vstorage'].indexOf(pool.type) > -1) {
Expand Down
12 changes: 7 additions & 5 deletions test/verify/machineslib.py
Original file line number Diff line number Diff line change
Expand Up @@ -664,8 +664,11 @@ def verify_disk_added(self):
# Guess by the name of the pool it's format to avoid passing more parameters
if self.pool_type == 'iscsi':
expected_format = 'unknown'
elif self.pool_type == 'disk':
expected_format = 'none'
else:
expected_format = 'qcow2'

self.test_obj.assertEqual(
m.execute(detect_format_cmd).rstrip(),
'<format type="{0}"/>'.format(self.volume_format or expected_format)
Expand Down Expand Up @@ -843,23 +846,22 @@ def verify_disk_added(self):

prepareDisk(self.machine)
cmds = [
"virsh pool-define-as disk-pool disk - - /dev/disk/by-id/scsi-0QEMU_QEMU_HARDDISK_DISK1 - /tmp/poolDiskImages",
"virsh pool-build disk-pool --overwrite",
"virsh pool-start disk-pool",
"virsh pool-define-as pool-disk disk - - /dev/disk/by-id/scsi-0QEMU_QEMU_HARDDISK_DISK1 - /tmp/poolDiskImages",
"virsh pool-build pool-disk --overwrite",
"virsh pool-start pool-disk",
]
self.machine.execute(" && ".join(cmds))

partition = str(self.machine.execute("readlink -f /dev/disk/by-id/scsi-0QEMU_QEMU_HARDDISK_DISK1 | cut -d '/' -f 3").strip()) + "1"

VMAddDiskDialog(
self,
pool_name='disk-pool',
pool_name='pool-disk',
pool_type='disk',
volume_name=partition,
volume_size=10,
volume_size_unit='MiB',
expected_target='vdc',
volume_format='none',
).execute()

def testNetworks(self):
Expand Down

0 comments on commit 93ab723

Please sign in to comment.