7:20 a.m.
Some cleanups to the command line generator so that it's easier to
splice in the new blockdev-add stuff.
Peter Krempa (2):
qemu: command: Separate validation from command line building for
-drive
qemu: command: Move PPC fdc check to domain validation
src/qemu/qemu_command.c | 295 ++++++++++++++++++++++++++----------------------
src/qemu/qemu_process.c | 11 +-
2 files changed, 171 insertions(+), 135 deletions(-)
--
2.14.1
7:20 a.m.
New subject: [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
7:20 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: command: Move PPC fdc check to domain validation
Apart from not littering the command line generator, the added benefit
is that new configs with a FDC will be rejected at define stage.
---
src/qemu/qemu_command.c | 8 --------
src/qemu/qemu_process.c | 11 ++++++++++-
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 698fef58d..95f99d4d9 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2247,14 +2247,6 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo;
qemuDomainSecretInfoPtr encinfo = diskPriv->encinfo;
- /* PowerPC pseries based VMs do not support floppy device */
- if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY &&
- qemuDomainIsPSeries(def)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("PowerPC pseries machines do not support floppy
device"));
- return -1;
- }
-
if (disk->info.bootIndex) {
bootindex = disk->info.bootIndex;
} else {
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index bde3ba462..d8b721282 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4586,7 +4586,8 @@ qemuProcessStartValidateDisks(virDomainObjPtr vm,
size_t i;
for (i = 0; i < vm->def->ndisks; i++) {
- virStorageSourcePtr src = vm->def->disks[i]->src;
+ virDomainDiskDefPtr disk = vm->def->disks[i];
+ virStorageSourcePtr src = disk->src;
/* This is a best effort check as we can only check if the command
* option exists, but we cannot determine whether the running QEMU
@@ -4599,6 +4600,14 @@ qemuProcessStartValidateDisks(virDomainObjPtr vm,
"QEMU binary"));
return -1;
}
+
+ /* PowerPC pseries based VMs do not support floppy device */
+ if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY &&
+ qemuDomainIsPSeries(vm->def)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("PowerPC pseries machines do not support floppy
device"));
+ return -1;
+ }
}
return 0;
--
2.14.1
10:17 a.m.
On Wed, Oct 04, 2017 at 02:20:01PM +0200, Peter Krempa wrote:
Some cleanups to the command line generator so that it's easier
to
splice in the new blockdev-add stuff.
Peter Krempa (2):
qemu: command: Separate validation from command line building for
-drive
qemu: command: Move PPC fdc check to domain validation
src/qemu/qemu_command.c | 295 ++++++++++++++++++++++++++----------------------
src/qemu/qemu_process.c | 11 +-
2 files changed, 171 insertions(+), 135 deletions(-)
ACK series
Can't wait for the sequel
Jan
2609
days inactive
2609
days old
3 comments
2 participants
participants (2)
-
Ján Tomko -
Peter Krempa