[libvirt] [PATCH 02/17] qemu: Introduce qemuDomainDeviceDefSkipController
Ján Tomko
jtomko at redhat.com
Tue Dec 5 14:18:54 UTC 2017
On Mon, Dec 04, 2017 at 08:38:52PM -0500, John Ferlan wrote:
>In order to be able to reuse some code for both controller def
>validation and command line building, create a convenience helper
>that will perform the "skip" avoidance checks. It will also set
>some flags to allow the caller to specifically skip (or fail)
>depending on the result of the flag (as opposed to building up
>some ever growing list of variables).
>
>Signed-off-by: John Ferlan <jferlan at redhat.com>
>---
> src/qemu/qemu_command.c | 61 +++++------------------------------
> src/qemu/qemu_domain.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++-
> src/qemu/qemu_domain.h | 12 +++++++
> 3 files changed, 102 insertions(+), 55 deletions(-)
>
[...]
>diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>index 569bbd29e..d4c7674c0 100644
>--- a/src/qemu/qemu_domain.c
>+++ b/src/qemu/qemu_domain.c
>@@ -3884,10 +3884,92 @@ qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk)
> }
>
>
>+/**
>+ * qemuDomainDeviceDefSkipController:
>+ * @controller: Controller to check
>+ * @def: Domain definition
>+ * @flags: qemuDomainDeviceSkipFlags to set if specific condition found
>+ *
>+ * Returns true if this controller can be skipped for command line generation
>+ * or device validation
We should not skip validation for implicit devices. Some of the checks
added later are for implicit devices (PCI root, SATA controller).
It would be enough to split out the logic of figuring out whether the
controller is implicit out of command line generation.
>+ */
>+bool
>+qemuDomainDeviceDefSkipController(const virDomainControllerDef *controller,
>+ const virDomainDef *def,
>+ unsigned int *flags)
>+{
>+ /* skip USB controllers with type none.*/
>+ if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
>+ controller->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) {
>+ *flags |= QEMU_DOMAIN_DEVICE_SKIP_USB_CONTROLLER_FLAG;
>+ return true;
>+ }
Simply returning true here (no USB controller is implicit) should
suffice. The caller can figure out whether USB_NONE is present
by going through the controllers array again (via virDomainDefHasUSB).
>+
>+ /* skip pcie-root */
>+ if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
>+ controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)
>+ return true;
>+
>+ /* Skip pci-root, except for pSeries guests (which actually
>+ * support more than one PCI Host Bridge per guest) */
>+ if (!qemuDomainIsPSeries(def) &&
>+ controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
>+ controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT)
>+ return true;
>+
>+ /* first SATA controller on Q35 machines is implicit */
>+ if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA &&
>+ controller->idx == 0 && qemuDomainIsQ35(def))
>+ return true;
>+
>+ /* first IDE controller is implicit on various machines */
>+ if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE &&
>+ controller->idx == 0 && qemuDomainHasBuiltinIDE(def))
>+ return true;
>+
>+ if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
>+ controller->model == -1 &&
>+ !qemuDomainIsQ35(def) &&
>+ !qemuDomainIsVirt(def)) {
>+
>+ /* An appropriate default USB controller model should already
>+ * have been selected in qemuDomainDeviceDefPostParse(); if
>+ * we still have no model by now, we have to fall back to the
>+ * legacy USB controller.
>+ *
>+ * Note that we *don't* want to end up with the legacy USB
>+ * controller for q35 and virt machines, so we go ahead and
>+ * fail in qemuBuildControllerDevStr(); on the other hand,
>+ * for s390 machines we want to ignore any USB controller
>+ * (see 548ba43028 for the full story), so we skip
>+ * qemuBuildControllerDevStr() but we don't ultimately end
>+ * up adding the legacy USB controller */
>+ if (*flags & QEMU_DOMAIN_DEVICE_SKIP_USB_LEGACY_FLAG) {
>+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>+ _("Multiple legacy USB controllers are "
>+ "not supported"));
A function returning bool where both options are valid should not be
setting errors.
For this error, validate can go through all the controllers. Command
line generator should not care and we can get rid of the remaining two
flags as their only purpose is to save us multiple passes through
the controllers field.
>+ *flags |= QEMU_DOMAIN_DEVICE_SKIP_USB_LEGACY_FAIL_FLAG;
>+ }
>+ *flags |= QEMU_DOMAIN_DEVICE_SKIP_USB_LEGACY_FLAG;
>+ return true;
>+ }
>+
>+ return false;
>+}
>+
>+
> static int
> qemuDomainDeviceDefValidateController(const virDomainControllerDef *controller,
>- const virDomainDef *def ATTRIBUTE_UNUSED)
>+ const virDomainDef *def)
> {
>+ unsigned int flags = 0;
>+
>+ if (qemuDomainDeviceDefSkipController(controller, def, &flags))
>+ return 0;
>+
>+ if (flags & QEMU_DOMAIN_DEVICE_SKIP_USB_LEGACY_FAIL_FLAG)
>+ return -1;
To possibly set this flag, SkipController must be called with non-zero
flags, so this would be dead code.
I'd rather go through def->controllers again to look for another legacy
controller, just for the purpose of validation.
Jan
>+
> switch ((virDomainControllerType) controller->type) {
> case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
> case VIR_DOMAIN_CONTROLLER_TYPE_FDC:
>diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
>index c33af3671..5c9c55d38 100644
>--- a/src/qemu/qemu_domain.h
>+++ b/src/qemu/qemu_domain.h
>@@ -999,4 +999,16 @@ qemuDomainPrepareDiskSource(virConnectPtr conn,
> qemuDomainObjPrivatePtr priv,
> virQEMUDriverConfigPtr cfg);
>
>+typedef enum {
>+ QEMU_DOMAIN_DEVICE_SKIP_USB_CONTROLLER_FLAG = (1 << 0),
>+ QEMU_DOMAIN_DEVICE_SKIP_USB_LEGACY_FLAG = (1 << 1),
>+ QEMU_DOMAIN_DEVICE_SKIP_USB_LEGACY_FAIL_FLAG = (1 << 2),
>+} qemuDomainDeviceSkipFlags;
>+
>+bool
>+qemuDomainDeviceDefSkipController(const virDomainControllerDef *controller,
>+ const virDomainDef *def,
>+ unsigned int *flags);
>+
>+
> #endif /* __QEMU_DOMAIN_H__ */
>--
>2.13.6
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20171205/bddef87c/attachment-0001.sig>
More information about the libvir-list
mailing list