[PATCH 05/20] qemu: Move disk config validation to qemuValidateDomainDeviceDefDiskFrontend
Peter Krempa
pkrempa at redhat.com
Wed May 6 12:08:20 UTC 2020
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 at redhat.com>
---
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
More information about the libvir-list
mailing list