LUN disks are supported only by VMX and QEMU drivers and the VMX implementation is a subset of qemu's implementation, thus we can move the qemu-specific validator to the global validation code providing that we allow the format to be 'none' (qemu driver always sets 'raw' if it's not set) and allow disk type 'volume' as a source (qemu always translates the source, and VMX doesn't implement 'volume' at all). Moving the code to the global validation allows us to stop calling it from the qemu specific validation and also deduplicates the checks. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/conf/domain_validate.c | 61 ++++++++++++++++++++++++++++++++------ src/conf/domain_validate.h | 2 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_domain.c | 48 ------------------------------ src/qemu/qemu_domain.h | 3 -- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_validate.c | 3 -- 7 files changed, 56 insertions(+), 64 deletions(-) diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 60f7ccdddd..dbabb953af 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -540,6 +540,57 @@ virDomainDiskDefValidateSource(const virStorageSource *src) #define VENDOR_LEN 8 #define PRODUCT_LEN 16 + +/** + * virDomainDiskDefSourceLUNValidate: + * @src: disk source struct + * + * Validate whether the disk source is valid for disk device='lun'. + * + * Returns 0 if the configuration is valid -1 and a libvirt error if the source + * is invalid. + */ +int +virDomainDiskDefSourceLUNValidate(const virStorageSource *src) +{ + if (virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_NETWORK) { + if (src->protocol != VIR_STORAGE_NET_PROTOCOL_ISCSI) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk device='lun' is not supported for protocol='%s'"), + virStorageNetProtocolTypeToString(src->protocol)); + return -1; + } + } else if (!virStorageSourceIsBlockLocal(src) && + src->type != VIR_STORAGE_TYPE_VOLUME) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("disk device='lun' is only valid for block type disk source")); + return -1; + } + + if (src->format != VIR_STORAGE_FILE_RAW && + src->format != VIR_STORAGE_FILE_NONE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("disk device 'lun' must use 'raw' format")); + return -1; + } + + if (src->sliceStorage) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("disk device 'lun' doesn't support storage slice")); + return -1; + } + + if (src->encryption && + src->encryption->format != VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("disk device 'lun' doesn't support encryption")); + return -1; + } + + return 0; +} + + static int virDomainDiskDefValidate(const virDomainDef *def, const virDomainDiskDef *disk) @@ -551,16 +602,8 @@ virDomainDiskDefValidate(const virDomainDef *def, /* Validate LUN configuration */ if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { - /* volumes haven't been translated at this point, so accept them */ - if (!(disk->src->type == VIR_STORAGE_TYPE_BLOCK || - disk->src->type == VIR_STORAGE_TYPE_VOLUME || - (disk->src->type == VIR_STORAGE_TYPE_NETWORK && - disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI))) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("disk '%s' improperly configured for a " - "device='lun'"), disk->dst); + if (virDomainDiskDefSourceLUNValidate(disk->src) < 0) return -1; - } } else { if (disk->src->pr) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", diff --git a/src/conf/domain_validate.h b/src/conf/domain_validate.h index 38a1abcc8f..430d61fd3c 100644 --- a/src/conf/domain_validate.h +++ b/src/conf/domain_validate.h @@ -40,3 +40,5 @@ int virDomainDeviceDefValidate(const virDomainDeviceDef *dev, void *parseOpaque); int virDomainDiskDefValidateSource(const virStorageSource *src); + +int virDomainDiskDefSourceLUNValidate(const virStorageSource *src); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ab8a6c00c3..2778fe7f8f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -767,6 +767,7 @@ virDomainConfVMNWFilterTeardown; virDomainActualNetDefValidate; virDomainDefValidate; virDomainDeviceValidateAliasForHotplug; +virDomainDiskDefSourceLUNValidate; # conf/interface_conf.h diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 7f7d6dcfcc..5de7461fb3 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9825,54 +9825,6 @@ qemuDomainDiskByName(virDomainDef *def, } -/** - * qemuDomainDefValidateDiskLunSource: - * @src: disk source struct - * - * Validate whether the disk source is valid for disk device='lun'. - * - * Returns 0 if the configuration is valid -1 and a libvirt error if the source - * is invalid. - */ -int -qemuDomainDefValidateDiskLunSource(const virStorageSource *src) -{ - if (virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_NETWORK) { - if (src->protocol != VIR_STORAGE_NET_PROTOCOL_ISCSI) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("disk device='lun' is not supported for protocol='%s'"), - virStorageNetProtocolTypeToString(src->protocol)); - return -1; - } - } else if (!virStorageSourceIsBlockLocal(src)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("disk device='lun' is only valid for block type disk source")); - return -1; - } - - if (src->format != VIR_STORAGE_FILE_RAW) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("disk device 'lun' must use 'raw' format")); - return -1; - } - - if (src->sliceStorage) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("disk device 'lun' doesn't support storage slice")); - return -1; - } - - if (src->encryption && - src->encryption->format != VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("disk device 'lun' doesn't support encryption")); - return -1; - } - - return 0; -} - - int qemuDomainPrepareChannel(virDomainChrDef *channel, const char *domainChannelTargetDir) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index cb1cd968d5..ae9d76ec4a 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -893,9 +893,6 @@ int qemuDomainSecretPrepare(virQEMUDriver *driver, virDomainObj *vm) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -int qemuDomainDefValidateDiskLunSource(const virStorageSource *src) - ATTRIBUTE_NONNULL(1); - int qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk, virQEMUCaps *qemuCaps); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2ea67b941e..302d2b4647 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14861,7 +14861,7 @@ qemuDomainBlockCopyCommon(virDomainObj *vm, goto endjob; if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN && - qemuDomainDefValidateDiskLunSource(mirror) < 0) + virDomainDiskDefSourceLUNValidate(mirror) < 0) goto endjob; if (!(flags & VIR_DOMAIN_BLOCK_COPY_TRANSIENT_JOB) && diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 1a470f1ff5..d8f39b6bdd 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2663,9 +2663,6 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk, return -1; } - if (qemuDomainDefValidateDiskLunSource(disk->src) < 0) - return -1; - if (disk->wwn) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Setting wwn is not supported for lun device")); -- 2.31.1

The code rejecting a XML when the disk target is missing was moved to the validation code which goes after post parse. One of the cases in the disk post parse code didn't check whether 'disk->dst' is set which at that point isn't guaranteed. Fixes: 61fd7174c2afbe128ef1896198919429bcaca3d7 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2001627 Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/conf/domain_conf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index dbefc98ee8..cb9e7218ff 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5413,6 +5413,7 @@ virDomainDiskDefPostParse(virDomainDiskDef *disk, } if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + disk->dst && virDomainDiskDefAssignAddress(xmlopt, disk, def) < 0) { return -1; } -- 2.31.1

Cover the case of missing disk target to cover the case fixed by previous commit. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- .../disk-missing-target-invalid.err | 1 + .../disk-missing-target-invalid.xml | 22 +++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 3 files changed, 24 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-missing-target-invalid.err create mode 100644 tests/qemuxml2argvdata/disk-missing-target-invalid.xml diff --git a/tests/qemuxml2argvdata/disk-missing-target-invalid.err b/tests/qemuxml2argvdata/disk-missing-target-invalid.err new file mode 100644 index 0000000000..6a33f1e8e8 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-missing-target-invalid.err @@ -0,0 +1 @@ +missing target information for device /dev/HostVG/QEMUGuest1 diff --git a/tests/qemuxml2argvdata/disk-missing-target-invalid.xml b/tests/qemuxml2argvdata/disk-missing-target-invalid.xml new file mode 100644 index 0000000000..49cd73142a --- /dev/null +++ b/tests/qemuxml2argvdata/disk-missing-target-invalid.xml @@ -0,0 +1,22 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i386</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + </disk> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 3b331d5fd4..c67214d01e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1455,6 +1455,7 @@ mymain(void) DO_TEST_PARSE_ERROR("disk-same-targets", QEMU_CAPS_SCSI_LSI, QEMU_CAPS_DEVICE_USB_STORAGE); + DO_TEST_PARSE_ERROR("disk-missing-target-invalid", QEMU_CAPS_SCSI_LSI); DO_TEST_PARSE_ERROR("disk-address-conflict", QEMU_CAPS_ICH9_AHCI); DO_TEST_PARSE_ERROR("disk-hostdev-scsi-address-conflict", -- 2.31.1