[PATCH 05/20] qemu: Move disk config validation to qemuValidateDomainDeviceDefDiskFrontend

Previously we've validated it in qemuCheckDiskConfig which was directly called from the command line generator. Move the checks to the validator where they belong. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_command.c | 202 +------------------------------------- src/qemu/qemu_validate.c | 204 ++++++++++++++++++++++++++++++++++++++- tests/qemuxml2argvtest.c | 2 +- 3 files changed, 205 insertions(+), 203 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 87cb78e350..2dfd17ad40 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -651,23 +651,6 @@ qemuBuildIoEventFdStr(virBufferPtr buf, return 0; } -#define QEMU_SERIAL_PARAM_ACCEPTED_CHARS \ - "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_ .+" - -static int -qemuSafeSerialParamValue(const char *value) -{ - if (strspn(value, QEMU_SERIAL_PARAM_ACCEPTED_CHARS) != strlen(value)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("driver serial '%s' contains unsafe characters"), - value); - return -1; - } - - return 0; -} - - /** * qemuBuildSecretInfoProps: * @secinfo: pointer to the secret info object @@ -1140,191 +1123,10 @@ qemuDiskConfigBlkdeviotuneEnabled(virDomainDiskDefPtr disk) * error reported. */ int -qemuCheckDiskConfig(virDomainDiskDefPtr disk, +qemuCheckDiskConfig(virDomainDiskDefPtr disk G_GNUC_UNUSED, const virDomainDef *def G_GNUC_UNUSED, - virQEMUCapsPtr qemuCaps) + virQEMUCapsPtr qemuCaps G_GNUC_UNUSED) { - if (disk->wwn) { - if ((disk->bus != VIR_DOMAIN_DISK_BUS_IDE) && - (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Only ide and scsi disk support wwn")); - return -1; - } - } - - if ((disk->vendor || disk->product) && - disk->bus != VIR_DOMAIN_DISK_BUS_SCSI) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Only scsi disk supports vendor and product")); - return -1; - } - - if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { - /* make sure that both the bus supports type='lun' (SG_IO). */ - if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO && - disk->bus != VIR_DOMAIN_DISK_BUS_SCSI) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("disk device='lun' is not supported for bus='%s'"), - virDomainDiskBusTypeToString(disk->bus)); - return -1; - } - - if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI && - disk->src->format != VIR_STORAGE_FILE_RAW) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("disk device 'lun' using target 'scsi' must use " - "'raw' format")); - 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")); - return -1; - } - if (disk->vendor || disk->product) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Setting vendor or product is not supported " - "for lun device")); - return -1; - } - } - - switch (disk->bus) { - case VIR_DOMAIN_DISK_BUS_SCSI: - if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("unexpected address type for scsi disk")); - return -1; - } - - /* Setting bus= attr for SCSI drives, causes a controller - * to be created. Yes this is slightly odd. It is not possible - * to have > 1 bus on a SCSI controller (yet). */ - if (disk->info.addr.drive.bus != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("SCSI controller only supports 1 bus")); - return -1; - } - break; - - case VIR_DOMAIN_DISK_BUS_IDE: - if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("unexpected address type for ide disk")); - return -1; - } - /* We can only have 1 IDE controller (currently) */ - if (disk->info.addr.drive.controller != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Only 1 IDE controller is supported")); - return -1; - } - break; - - case VIR_DOMAIN_DISK_BUS_FDC: - if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("unexpected address type for fdc disk")); - return -1; - } - /* We can only have 1 FDC controller (currently) */ - if (disk->info.addr.drive.controller != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Only 1 fdc controller is supported")); - return -1; - } - /* We can only have 1 FDC bus (currently) */ - if (disk->info.addr.drive.bus != 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Only 1 fdc bus is supported")); - return -1; - } - if (disk->info.addr.drive.target != 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("target must be 0 for controller fdc")); - return -1; - } - break; - - case VIR_DOMAIN_DISK_BUS_VIRTIO: - case VIR_DOMAIN_DISK_BUS_XEN: - case VIR_DOMAIN_DISK_BUS_SD: - break; - } - - if (disk->src->readonly && - disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { - if (disk->bus == VIR_DOMAIN_DISK_BUS_IDE) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("readonly ide disks are not supported")); - return -1; - } - - if (disk->bus == VIR_DOMAIN_DISK_BUS_SATA) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("readonly sata disks are not supported")); - return -1; - } - } - - if (disk->transient) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("transient disks not supported yet")); - return -1; - } - - if (disk->iomode == VIR_DOMAIN_DISK_IO_NATIVE && - disk->cachemode != VIR_DOMAIN_DISK_CACHE_DIRECTSYNC && - disk->cachemode != VIR_DOMAIN_DISK_CACHE_DISABLE) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("native I/O needs either no disk cache " - "or directsync cache mode, QEMU will fallback " - "to aio=threads")); - return -1; - } - - if (qemuCaps) { - if (disk->serial && - disk->bus == VIR_DOMAIN_DISK_BUS_SCSI && - disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("scsi-block 'lun' devices do not support the " - "serial property")); - return -1; - } - - if (disk->discard && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_DISCARD)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("discard is not supported by this QEMU binary")); - return -1; - } - - if (disk->detect_zeroes && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_DETECT_ZEROES)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("detect_zeroes is not supported by this QEMU binary")); - return -1; - } - - if (disk->iomode == VIR_DOMAIN_DISK_IO_URING) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_AIO_IO_URING)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("io uring is not supported by this QEMU binary")); - return -1; - } - } - } - - if (disk->serial && - qemuSafeSerialParamValue(disk->serial) < 0) - return -1; - return 0; } diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 17cfcddd30..63cde01762 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1897,8 +1897,26 @@ qemuValidateDomainDeviceDefVideo(const virDomainVideoDef *video, } +#define QEMU_SERIAL_PARAM_ACCEPTED_CHARS \ + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_ .+" + +static int +qemuValidateDomainDeviceDefDiskSerial(const char *value) +{ + if (strspn(value, QEMU_SERIAL_PARAM_ACCEPTED_CHARS) != strlen(value)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("driver serial '%s' contains unsafe characters"), + value); + return -1; + } + + return 0; +} + + static int -qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk) +qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk, + virQEMUCapsPtr qemuCaps) { if (disk->geometry.cylinders > 0 && disk->geometry.heads > 0 && @@ -1952,6 +1970,188 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk) } } + if (disk->wwn) { + if ((disk->bus != VIR_DOMAIN_DISK_BUS_IDE) && + (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Only ide and scsi disk support wwn")); + return -1; + } + } + + if ((disk->vendor || disk->product) && + disk->bus != VIR_DOMAIN_DISK_BUS_SCSI) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Only scsi disk supports vendor and product")); + return -1; + } + + if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { + /* make sure that both the bus supports type='lun' (SG_IO). */ + if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO && + disk->bus != VIR_DOMAIN_DISK_BUS_SCSI) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk device='lun' is not supported for bus='%s'"), + virDomainDiskBusTypeToString(disk->bus)); + return -1; + } + + if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI && + disk->src->format != VIR_STORAGE_FILE_RAW) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("disk device 'lun' using target 'scsi' must use " + "'raw' format")); + 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")); + return -1; + } + if (disk->vendor || disk->product) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Setting vendor or product is not supported " + "for lun device")); + return -1; + } + } + + switch (disk->bus) { + case VIR_DOMAIN_DISK_BUS_SCSI: + if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("unexpected address type for scsi disk")); + return -1; + } + + /* Setting bus= attr for SCSI drives, causes a controller + * to be created. Yes this is slightly odd. It is not possible + * to have > 1 bus on a SCSI controller (yet). */ + if (disk->info.addr.drive.bus != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("SCSI controller only supports 1 bus")); + return -1; + } + break; + + case VIR_DOMAIN_DISK_BUS_IDE: + if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("unexpected address type for ide disk")); + return -1; + } + /* We can only have 1 IDE controller (currently) */ + if (disk->info.addr.drive.controller != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Only 1 IDE controller is supported")); + return -1; + } + break; + + case VIR_DOMAIN_DISK_BUS_FDC: + if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("unexpected address type for fdc disk")); + return -1; + } + /* We can only have 1 FDC controller (currently) */ + if (disk->info.addr.drive.controller != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Only 1 fdc controller is supported")); + return -1; + } + /* We can only have 1 FDC bus (currently) */ + if (disk->info.addr.drive.bus != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Only 1 fdc bus is supported")); + return -1; + } + if (disk->info.addr.drive.target != 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("target must be 0 for controller fdc")); + return -1; + } + break; + + case VIR_DOMAIN_DISK_BUS_VIRTIO: + case VIR_DOMAIN_DISK_BUS_XEN: + case VIR_DOMAIN_DISK_BUS_SD: + break; + } + + if (disk->src->readonly && + disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { + if (disk->bus == VIR_DOMAIN_DISK_BUS_IDE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("readonly ide disks are not supported")); + return -1; + } + + if (disk->bus == VIR_DOMAIN_DISK_BUS_SATA) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("readonly sata disks are not supported")); + return -1; + } + } + + if (disk->transient) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("transient disks not supported yet")); + return -1; + } + + if (disk->iomode == VIR_DOMAIN_DISK_IO_NATIVE && + disk->cachemode != VIR_DOMAIN_DISK_CACHE_DIRECTSYNC && + disk->cachemode != VIR_DOMAIN_DISK_CACHE_DISABLE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("native I/O needs either no disk cache " + "or directsync cache mode, QEMU will fallback " + "to aio=threads")); + return -1; + } + + if (qemuCaps) { + if (disk->serial && + disk->bus == VIR_DOMAIN_DISK_BUS_SCSI && + disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("scsi-block 'lun' devices do not support the " + "serial property")); + return -1; + } + + if (disk->discard && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_DISCARD)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("discard is not supported by this QEMU binary")); + return -1; + } + + if (disk->detect_zeroes && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_DETECT_ZEROES)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("detect_zeroes is not supported by this QEMU binary")); + return -1; + } + + if (disk->iomode == VIR_DOMAIN_DISK_IO_URING) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_AIO_IO_URING)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("io uring is not supported by this QEMU binary")); + return -1; + } + } + } + + if (disk->serial && + qemuValidateDomainDeviceDefDiskSerial(disk->serial) < 0) + return -1; + + return 0; } @@ -2062,7 +2262,7 @@ qemuValidateDomainDeviceDefDisk(const virDomainDiskDef *disk, int idx; int partition; - if (qemuValidateDomainDeviceDefDiskFrontend(disk) < 0) + if (qemuValidateDomainDeviceDefDiskFrontend(disk, qemuCaps) < 0) return -1; if (qemuValidateDomainDeviceDefDiskBlkdeviotune(disk, def, qemuCaps) < 0) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index f45f04548f..ad89353910 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1115,7 +1115,7 @@ mymain(void) QEMU_CAPS_SCSI_LSI, QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_SCSI_DISK_WWN); DO_TEST("disk-scsi-disk-vpd", QEMU_CAPS_SCSI_LSI, QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_SCSI_DISK_WWN); - DO_TEST_FAILURE("disk-scsi-disk-vpd-build-error", + DO_TEST_PARSE_ERROR("disk-scsi-disk-vpd-build-error", QEMU_CAPS_SCSI_LSI, QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_SCSI_DISK_WWN); DO_TEST_CAPS_LATEST("controller-virtio-scsi"); DO_TEST("disk-sata-device", -- 2.26.2