[libvirt] [PATCH 1/2] qemu: command: Separate validation from command line building for -drive

Remove validation code into a separate function so that it's not interleaved with actual building of the command line. --- src/qemu/qemu_command.c | 287 +++++++++++++++++++++++++++--------------------- 1 file changed, 161 insertions(+), 126 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4f141e0ac..698fef58d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1481,24 +1481,16 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk, } -char * -qemuBuildDriveStr(virDomainDiskDefPtr disk, - virQEMUDriverConfigPtr cfg, - bool bootable, - virQEMUCapsPtr qemuCaps) +static int +qemuBuildDriveStrValidate(virDomainDiskDefPtr disk, + virQEMUCapsPtr qemuCaps, + const char *bus, + int idx) { - virBuffer opt = VIR_BUFFER_INITIALIZER; - const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus); - const char *trans = - virDomainDiskGeometryTransTypeToString(disk->geometry.trans); - int idx = virDiskNameToIndex(disk->dst); - int busid = -1, unitid = -1; - bool emitDeviceSyntax = qemuDiskBusNeedsDeviceArg(disk->bus); - if (idx < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported disk type '%s'"), disk->dst); - goto error; + return -1; } switch (disk->bus) { @@ -1506,7 +1498,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, if (disk->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("unexpected address type for scsi disk")); - goto error; + return -1; } /* Setting bus= attr for SCSI drives, causes a controller @@ -1515,53 +1507,173 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, if (disk->info.addr.drive.bus != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("SCSI controller only supports 1 bus")); - goto error; + return -1; } - busid = disk->info.addr.drive.controller; - unitid = disk->info.addr.drive.unit; 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")); - goto error; + return -1; } /* We can only have 1 IDE controller (currently) */ if (disk->info.addr.drive.controller != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Only 1 %s controller is supported"), bus); - goto error; + return -1; } - busid = disk->info.addr.drive.bus; - unitid = disk->info.addr.drive.unit; 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")); - goto error; + return -1; } /* We can only have 1 FDC controller (currently) */ if (disk->info.addr.drive.controller != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Only 1 %s controller is supported"), bus); - goto error; + return -1; } /* We can only have 1 FDC bus (currently) */ if (disk->info.addr.drive.bus != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Only 1 %s bus is supported"), bus); - goto error; + return -1; } if (disk->info.addr.drive.target != 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("target must be 0 for controller fdc")); - goto error; + 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->serial && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_SERIAL)) { + if (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->cachemode == VIR_DOMAIN_DISK_CACHE_DIRECTSYNC && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_CACHE_DIRECTSYNC)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("disk cache mode 'directsync' is not supported by this QEMU")); + return -1; + } + + if (disk->cachemode == VIR_DOMAIN_DISK_CACHE_UNSAFE && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_CACHE_UNSAFE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("disk cache mode 'unsafe' is not supported by this QEMU")); + 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 (disk->copy_on_read && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_COPY_ON_READ)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("copy_on_read is not supported by this QEMU binary")); + 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 && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_AIO)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("disk aio mode not supported with this QEMU binary")); + return -1; + } + + return 0; +} + + +char * +qemuBuildDriveStr(virDomainDiskDefPtr disk, + virQEMUDriverConfigPtr cfg, + bool bootable, + virQEMUCapsPtr qemuCaps) +{ + virBuffer opt = VIR_BUFFER_INITIALIZER; + const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus); + const char *trans = + virDomainDiskGeometryTransTypeToString(disk->geometry.trans); + int idx = virDiskNameToIndex(disk->dst); + int busid = -1, unitid = -1; + bool emitDeviceSyntax = qemuDiskBusNeedsDeviceArg(disk->bus); + + if (qemuBuildDriveStrValidate(disk, qemuCaps, bus, idx) < 0) + goto error; + + switch (disk->bus) { + case VIR_DOMAIN_DISK_BUS_SCSI: + busid = disk->info.addr.drive.controller; unitid = disk->info.addr.drive.unit; + break; + case VIR_DOMAIN_DISK_BUS_IDE: + busid = disk->info.addr.drive.bus; + unitid = disk->info.addr.drive.unit; + break; + + case VIR_DOMAIN_DISK_BUS_FDC: + unitid = disk->info.addr.drive.unit; break; case VIR_DOMAIN_DISK_BUS_VIRTIO: @@ -1618,26 +1730,8 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) && disk->bus != VIR_DOMAIN_DISK_BUS_IDE) virBufferAddLit(&opt, ",boot=on"); - if (disk->src->readonly) { - if (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")); - goto error; - } - if (disk->bus == VIR_DOMAIN_DISK_BUS_SATA) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("readonly sata disks are not supported")); - goto error; - } - } + if (disk->src->readonly) virBufferAddLit(&opt, ",readonly=on"); - } - if (disk->transient) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("transient disks not supported yet")); - goto error; - } /* generate geometry command string */ if (disk->geometry.cylinders > 0 && @@ -1657,95 +1751,43 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_SERIAL)) { if (qemuSafeSerialParamValue(disk->serial) < 0) goto error; - if (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")); - goto error; - } virBufferAddLit(&opt, ",serial="); virBufferEscape(&opt, '\\', " ", "%s", disk->serial); } if (disk->cachemode) { - const char *mode = NULL; - - mode = qemuDiskCacheV2TypeToString(disk->cachemode); - - if (disk->cachemode == VIR_DOMAIN_DISK_CACHE_DIRECTSYNC && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_CACHE_DIRECTSYNC)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("disk cache mode 'directsync' is not " - "supported by this QEMU")); - goto error; - } else if (disk->cachemode == VIR_DOMAIN_DISK_CACHE_UNSAFE && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_CACHE_UNSAFE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("disk cache mode 'unsafe' is not " - "supported by this QEMU")); - goto error; - } - - 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")); - goto error; - } - - virBufferAsprintf(&opt, ",cache=%s", mode); + virBufferAsprintf(&opt, ",cache=%s", + qemuDiskCacheV2TypeToString(disk->cachemode)); } else if (disk->src->shared && !disk->src->readonly) { virBufferAddLit(&opt, ",cache=none"); } if (disk->copy_on_read) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_COPY_ON_READ)) { - virBufferAsprintf(&opt, ",copy-on-read=%s", - virTristateSwitchTypeToString(disk->copy_on_read)); - } else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("copy_on_read is not supported by this QEMU binary")); - goto error; - } + virBufferAsprintf(&opt, ",copy-on-read=%s", + virTristateSwitchTypeToString(disk->copy_on_read)); } if (disk->discard) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_DISCARD)) { - virBufferAsprintf(&opt, ",discard=%s", - virDomainDiskDiscardTypeToString(disk->discard)); - } else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("discard is not supported by this QEMU binary")); - goto error; - } + virBufferAsprintf(&opt, ",discard=%s", + virDomainDiskDiscardTypeToString(disk->discard)); } if (disk->detect_zeroes) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_DETECT_ZEROES)) { - int detect_zeroes = disk->detect_zeroes; - - /* - * As a convenience syntax, if discards are ignored and - * zero detection is set to 'unmap', then simply behave - * like zero detection is set to 'on'. But don't change - * it in the XML for easier adjustments. This behaviour - * is documented. - */ - if (disk->discard != VIR_DOMAIN_DISK_DISCARD_UNMAP && - detect_zeroes == VIR_DOMAIN_DISK_DETECT_ZEROES_UNMAP) - detect_zeroes = VIR_DOMAIN_DISK_DETECT_ZEROES_ON; + int detect_zeroes = disk->detect_zeroes; - virBufferAsprintf(&opt, ",detect-zeroes=%s", - virDomainDiskDetectZeroesTypeToString(detect_zeroes)); - } else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("detect_zeroes is not supported by this QEMU binary")); - goto error; - } + /* + * As a convenience syntax, if discards are ignored and + * zero detection is set to 'unmap', then simply behave + * like zero detection is set to 'on'. But don't change + * it in the XML for easier adjustments. This behaviour + * is documented. + */ + if (disk->discard != VIR_DOMAIN_DISK_DISCARD_UNMAP && + detect_zeroes == VIR_DOMAIN_DISK_DETECT_ZEROES_UNMAP) + detect_zeroes = VIR_DOMAIN_DISK_DETECT_ZEROES_ON; + + virBufferAsprintf(&opt, ",detect-zeroes=%s", + virDomainDiskDetectZeroesTypeToString(detect_zeroes)); } if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MONITOR_JSON)) { @@ -1774,15 +1816,8 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, } if (disk->iomode) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_AIO)) { - virBufferAsprintf(&opt, ",aio=%s", - virDomainDiskIoTypeToString(disk->iomode)); - } else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("disk aio mode not supported with this " - "QEMU binary")); - goto error; - } + virBufferAsprintf(&opt, ",aio=%s", + virDomainDiskIoTypeToString(disk->iomode)); } if (qemuCheckDiskConfigBlkdeviotune(disk, qemuCaps) < 0) -- 2.14.1