[libvirt] [PATCH 0/4] domain capabilities fix and test improvement

Pavel Hrdina (4): qemu: refactor qemuDomainMachineIs* and qemuDomainMachineNeedsFDC qemu: use qemuDomainMachineIsPSeries qemu: report IDE bus in domain capabilities only if it's supported tests: domaincapstest: add test for Q35 machine type src/qemu/qemu_alias.c | 4 +- src/qemu/qemu_capabilities.c | 20 ++-- src/qemu/qemu_command.c | 42 +++---- src/qemu/qemu_domain.c | 79 ++++++------- src/qemu/qemu_domain.h | 14 +-- src/qemu/qemu_domain_address.c | 16 +-- src/qemu/qemu_hotplug.c | 14 +-- src/qemu/qemu_parse_command.c | 8 +- .../qemu_2.6.0-gicv2-virt.aarch64.xml | 1 - .../qemu_2.6.0-gicv3-virt.aarch64.xml | 1 - tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml | 1 - tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml | 1 - tests/domaincapsschemadata/qemu_2.7.0.s390x.xml | 1 - tests/domaincapsschemadata/qemu_2.8.0.s390x.xml | 1 - .../domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml | 125 +++++++++++++++++++++ tests/domaincapstest.c | 4 + 16 files changed, 229 insertions(+), 103 deletions(-) create mode 100644 tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml -- 2.12.2

These functions don't require the whole virDomainDef structure, they only need *machine* and *arch*. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_alias.c | 4 +-- src/qemu/qemu_capabilities.c | 10 +++--- src/qemu/qemu_command.c | 42 +++++++++++----------- src/qemu/qemu_domain.c | 79 ++++++++++++++++++++++-------------------- src/qemu/qemu_domain.h | 14 ++++---- src/qemu/qemu_domain_address.c | 16 ++++----- src/qemu/qemu_hotplug.c | 14 ++++---- src/qemu/qemu_parse_command.c | 8 ++--- 8 files changed, 95 insertions(+), 92 deletions(-) diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index 05e1293ef0..2169744245 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -152,14 +152,14 @@ qemuAssignDeviceControllerAlias(virDomainDefPtr domainDef, * first (and currently only) IDE controller is an integrated * controller hardcoded with id "ide" */ - if (qemuDomainMachineHasBuiltinIDE(domainDef) && + if (qemuDomainMachineHasBuiltinIDE(domainDef->os.machine) && controller->idx == 0) return VIR_STRDUP(controller->info.alias, "ide"); } else if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA) { /* for any Q35 machine, the first SATA controller is the * integrated one, and it too is hardcoded with id "ide" */ - if (qemuDomainMachineIsQ35(domainDef) && controller->idx == 0) + if (qemuDomainMachineIsQ35(domainDef->os.machine) && controller->idx == 0) return VIR_STRDUP(controller->info.alias, "ide"); } else if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_USB) { /* first USB device is "usb", others are normal "usb%d" */ diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 3ab99f450e..06f122ed0d 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2315,7 +2315,7 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps, /* If 'virt' supports PCI, it supports multibus. * No extra conditions here for simplicity. */ - if (qemuDomainMachineIsVirt(def)) + if (qemuDomainMachineIsVirt(def->os.machine, def->os.arch)) return true; return false; @@ -5369,7 +5369,7 @@ virQEMUCapsSupportsChardev(const virDomainDef *def, return false; if ((def->os.arch == VIR_ARCH_PPC) || ARCH_IS_PPC64(def->os.arch)) { - if (!qemuDomainMachineIsPSeries(def)) + if (!qemuDomainMachineIsPSeries(def->os.machine, def->os.arch)) return false; /* only pseries need -device spapr-vty with -chardev */ if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && @@ -5396,8 +5396,8 @@ virQEMUCapsSupportsVmport(virQEMUCapsPtr qemuCaps, if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_VMPORT_OPT)) return false; - return qemuDomainMachineIsI440FX(def) || - qemuDomainMachineIsQ35(def) || + return qemuDomainMachineIsI440FX(def->os.machine) || + qemuDomainMachineIsQ35(def->os.machine) || STREQ(def->os.machine, "isapc"); } @@ -5409,7 +5409,7 @@ virQEMUCapsSupportsSMM(virQEMUCapsPtr qemuCaps, if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_SMM_OPT)) return false; - return qemuDomainMachineIsQ35(def); + return qemuDomainMachineIsQ35(def->os.machine); } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 57654246c0..599bc292db 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1390,7 +1390,7 @@ qemuCheckCCWS390AddressSupport(const virDomainDef *def, const char *devicename) { if (info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { - if (!qemuDomainMachineIsS390CCW(def)) { + if (!qemuDomainMachineIsS390CCW(def->os.machine)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("cannot use CCW address type for device " "'%s' using machine type '%s'"), @@ -2294,7 +2294,7 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, /* PowerPC pseries based VMs do not support floppy device */ if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY && - qemuDomainMachineIsPSeries(def)) { + qemuDomainMachineIsPSeries(def->os.machine, def->os.arch)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("PowerPC pseries machines do not support floppy device")); return -1; @@ -2345,7 +2345,7 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, disk->info.alias) < 0) return -1; - if (!qemuDomainMachineNeedsFDC(def)) { + if (!qemuDomainMachineNeedsFDC(def->os.machine)) { virCommandAddArg(cmd, "-global"); virCommandAddArgFormat(cmd, "isa-fdc.%s", optstr); } else { @@ -2360,7 +2360,7 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, bootindex) < 0) return -1; - if (!qemuDomainMachineNeedsFDC(def)) { + if (!qemuDomainMachineNeedsFDC(def->os.machine)) { virCommandAddArg(cmd, "-global"); virCommandAddArgFormat(cmd, "isa-fdc.%s", optstr); } else { @@ -3076,7 +3076,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, * specified, or one with a single IDE contraller had multiple * ide controllers specified. */ - if (qemuDomainMachineHasBuiltinIDE(domainDef)) + if (qemuDomainMachineHasBuiltinIDE(domainDef->os.machine)) virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Only a single IDE controller is supported " "for this machine type")); @@ -3174,18 +3174,18 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd, /* first SATA controller on Q35 machines is implicit */ if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA && - cont->idx == 0 && qemuDomainMachineIsQ35(def)) + cont->idx == 0 && qemuDomainMachineIsQ35(def->os.machine)) continue; /* first IDE controller is implicit on various machines */ if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE && - cont->idx == 0 && qemuDomainMachineHasBuiltinIDE(def)) + cont->idx == 0 && qemuDomainMachineHasBuiltinIDE(def->os.machine)) continue; if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB && cont->model == -1 && - !qemuDomainMachineIsQ35(def) && - !qemuDomainMachineIsVirt(def)) { + !qemuDomainMachineIsQ35(def->os.machine) && + !qemuDomainMachineIsVirt(def->os.machine, def->os.arch)) { /* An appropriate default USB controller model should already * have been selected in qemuDomainDeviceDefPostParse(); if @@ -3222,8 +3222,8 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd, * not to add one either. Add a legacy USB controller, unless we're * creating a kind of guest we want to keep legacy-free */ if (usbcontroller == 0 && - !qemuDomainMachineIsQ35(def) && - !qemuDomainMachineIsVirt(def) && + !qemuDomainMachineIsQ35(def->os.machine) && + !qemuDomainMachineIsVirt(def->os.machine, def->os.arch) && !ARCH_IS_S390(def->os.arch)) virCommandAddArg(cmd, "-usb"); @@ -4104,7 +4104,7 @@ qemuBuildNVRAMCommandLine(virCommandPtr cmd, if (!def->nvram) return 0; - if (qemuDomainMachineIsPSeries(def)) { + if (qemuDomainMachineIsPSeries(def->os.machine, def->os.arch)) { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVRAM)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("nvram device is not supported by " @@ -6487,7 +6487,7 @@ qemuBuildPMCommandLine(virCommandPtr cmd, if (def->pm.s3) { const char *pm_object = "PIIX4_PM"; - if (qemuDomainMachineIsQ35(def) && + if (qemuDomainMachineIsQ35(def->os.machine) && virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_DISABLE_S3)) { pm_object = "ICH9-LPC"; } else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX_DISABLE_S3)) { @@ -6504,7 +6504,7 @@ qemuBuildPMCommandLine(virCommandPtr cmd, if (def->pm.s4) { const char *pm_object = "PIIX4_PM"; - if (qemuDomainMachineIsQ35(def) && + if (qemuDomainMachineIsQ35(def->os.machine) && virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_DISABLE_S4)) { pm_object = "ICH9-LPC"; } else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX_DISABLE_S4)) { @@ -6693,7 +6693,7 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd, virDomainIOMMUModelTypeToString(iommu->model)); return -1; } - if (!qemuDomainMachineIsQ35(def)) { + if (!qemuDomainMachineIsQ35(def->os.machine)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("IOMMU device: '%s' is only supported with " "Q35 machines"), @@ -6732,13 +6732,13 @@ qemuBuildGlobalControllerCommandLine(virCommandPtr cmd, case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: hoststr = "i440FX-pcihost"; cap = virQEMUCapsGet(qemuCaps, QEMU_CAPS_I440FX_PCI_HOLE64_SIZE); - machine = qemuDomainMachineIsI440FX(def); + machine = qemuDomainMachineIsI440FX(def->os.machine); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: hoststr = "q35-pcihost"; cap = virQEMUCapsGet(qemuCaps, QEMU_CAPS_Q35_PCI_HOLE64_SIZE); - machine = qemuDomainMachineIsQ35(def); + machine = qemuDomainMachineIsQ35(def->os.machine); break; default: @@ -7323,7 +7323,7 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, if (def->features[VIR_DOMAIN_FEATURE_GIC] == VIR_TRISTATE_SWITCH_ON) { if (def->gic_version != VIR_GIC_VERSION_NONE) { - if (!qemuDomainMachineIsVirt(def)) { + if (!qemuDomainMachineIsVirt(def->os.machine, def->os.arch)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("gic-version option is available " "only for ARM virt machine")); @@ -7352,7 +7352,7 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_IOMMU)) { switch (def->iommu->model) { case VIR_DOMAIN_IOMMU_MODEL_INTEL: - if (!qemuDomainMachineIsQ35(def)) { + if (!qemuDomainMachineIsQ35(def->os.machine)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("IOMMU device: '%s' is only supported with " "Q35 machines"), @@ -9615,7 +9615,7 @@ qemuBuildPanicCommandLine(virCommandPtr cmd, /* For pSeries guests, the firmware provides the same * functionality as the pvpanic device. The address * cannot be configured by the user */ - if (!qemuDomainMachineIsPSeries(def)) { + if (!qemuDomainMachineIsPSeries(def->os.machine, def->os.arch)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("only pSeries guests support panic device " "of model 'pseries'")); @@ -10041,7 +10041,7 @@ qemuBuildSerialChrDeviceStr(char **deviceStr, { virBuffer cmd = VIR_BUFFER_INITIALIZER; - if (qemuDomainMachineIsPSeries(def)) { + if (qemuDomainMachineIsPSeries(def->os.machine, def->os.arch)) { if (serial->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && serial->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO) { virBufferAsprintf(&cmd, "spapr-vty,chardev=char%s", diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 39bc8c7483..e1c46848b0 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2365,7 +2365,7 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, addDefaultUSB = false; break; } - if (qemuDomainMachineIsQ35(def)) { + if (qemuDomainMachineIsQ35(def->os.machine)) { addPCIeRoot = true; addImplicitSATA = true; @@ -2382,7 +2382,7 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, addDefaultUSB = false; break; } - if (qemuDomainMachineIsI440FX(def)) + if (qemuDomainMachineIsI440FX(def->os.machine)) addPCIRoot = true; break; @@ -2390,7 +2390,7 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, case VIR_ARCH_AARCH64: addDefaultUSB = false; addDefaultMemballoon = false; - if (qemuDomainMachineIsVirt(def)) + if (qemuDomainMachineIsVirt(def->os.machine, def->os.arch)) addPCIeRoot = virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_GPEX); break; @@ -2402,7 +2402,7 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, /* For pSeries guests, the firmware provides the same * functionality as the pvpanic device, so automatically * add the definition if not already present */ - if (qemuDomainMachineIsPSeries(def)) + if (qemuDomainMachineIsPSeries(def->os.machine, def->os.arch)) addPanicDevice = true; break; @@ -2552,7 +2552,7 @@ qemuDomainDefEnableDefaultFeatures(virDomainDefPtr def, * was not included in the domain XML, we need to choose a suitable * GIC version ourselves */ if (def->features[VIR_DOMAIN_FEATURE_GIC] == VIR_TRISTATE_SWITCH_ABSENT && - qemuDomainMachineIsVirt(def)) { + qemuDomainMachineIsVirt(def->os.machine, def->os.arch)) { VIR_DEBUG("Looking for usable GIC version in domain capabilities"); for (version = VIR_GIC_VERSION_LAST - 1; @@ -2940,7 +2940,7 @@ qemuDomainDefValidate(const virDomainDef *def, /* These are the QEMU implementation limitations. But we * have to live with them for now. */ - if (!qemuDomainMachineIsQ35(def)) { + if (!qemuDomainMachineIsQ35(def->os.machine)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Secure boot is supported with q35 machine types only")); goto cleanup; @@ -3039,7 +3039,7 @@ qemuDomainDefaultNetModel(const virDomainDef *def, if (STREQ(def->os.machine, "versatilepb")) return "smc91c111"; - if (qemuDomainMachineIsVirt(def)) + if (qemuDomainMachineIsVirt(def->os.machine, def->os.arch)) return "virtio"; /* Incomplete. vexpress (and a few others) use this, but not all @@ -3198,14 +3198,14 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont, case VIR_DOMAIN_CONTROLLER_TYPE_PCI: if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS && - !qemuDomainMachineIsI440FX(def)) { + !qemuDomainMachineIsI440FX(def->os.machine)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("pci-expander-bus controllers are only supported " "on 440fx-based machinetypes")); return -1; } if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS && - !qemuDomainMachineIsQ35(def)) { + !qemuDomainMachineIsQ35(def->os.machine)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("pcie-expander-bus controllers are only supported " "on q35-based machinetypes")); @@ -3360,7 +3360,7 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, if (dev->type == VIR_DOMAIN_DEVICE_PANIC && dev->data.panic->model == VIR_DOMAIN_PANIC_MODEL_DEFAULT) { - if (qemuDomainMachineIsPSeries(def)) + if (qemuDomainMachineIsPSeries(def->os.machine, def->os.arch)) dev->data.panic->model = VIR_DOMAIN_PANIC_MODEL_PSERIES; else if (ARCH_IS_S390(def->os.arch)) dev->data.panic->model = VIR_DOMAIN_PANIC_MODEL_S390; @@ -4047,7 +4047,8 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver, * because other architectures and machine types were introduced * when libvirt already supported <controller type='usb'/>. */ - if (ARCH_IS_X86(def->os.arch) && qemuDomainMachineIsI440FX(def) && + if (ARCH_IS_X86(def->os.arch) && + qemuDomainMachineIsI440FX(def->os.machine) && usb && usb->idx == 0 && (usb->model == -1 || usb->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI)) { @@ -5855,21 +5856,21 @@ qemuFindAgentConfig(virDomainDefPtr def) bool -qemuDomainMachineIsQ35(const virDomainDef *def) +qemuDomainMachineIsQ35(const char *machine) { - return (STRPREFIX(def->os.machine, "pc-q35") || - STREQ(def->os.machine, "q35")); + return (STRPREFIX(machine, "pc-q35") || + STREQ(machine, "q35")); } bool -qemuDomainMachineIsI440FX(const virDomainDef *def) +qemuDomainMachineIsI440FX(const char *machine) { - return (STREQ(def->os.machine, "pc") || - STRPREFIX(def->os.machine, "pc-0.") || - STRPREFIX(def->os.machine, "pc-1.") || - STRPREFIX(def->os.machine, "pc-i440") || - STRPREFIX(def->os.machine, "rhel")); + return (STREQ(machine, "pc") || + STRPREFIX(machine, "pc-0.") || + STRPREFIX(machine, "pc-1.") || + STRPREFIX(machine, "pc-i440") || + STRPREFIX(machine, "rhel")); } @@ -5904,9 +5905,9 @@ qemuDomainMachineHasPCIeRoot(const virDomainDef *def) bool -qemuDomainMachineNeedsFDC(const virDomainDef *def) +qemuDomainMachineNeedsFDC(const char *machine) { - char *p = STRSKIP(def->os.machine, "pc-q35-"); + const char *p = STRSKIP(machine, "pc-q35-"); if (p) { if (STRPREFIX(p, "1.") || @@ -5922,21 +5923,22 @@ qemuDomainMachineNeedsFDC(const virDomainDef *def) bool -qemuDomainMachineIsS390CCW(const virDomainDef *def) +qemuDomainMachineIsS390CCW(const char *machine) { - return STRPREFIX(def->os.machine, "s390-ccw"); + return STRPREFIX(machine, "s390-ccw"); } bool -qemuDomainMachineIsVirt(const virDomainDef *def) +qemuDomainMachineIsVirt(const char *machine, + const virArch arch) { - if (def->os.arch != VIR_ARCH_ARMV7L && - def->os.arch != VIR_ARCH_AARCH64) + if (arch != VIR_ARCH_ARMV7L && + arch != VIR_ARCH_AARCH64) return false; - if (STRNEQ(def->os.machine, "virt") && - !STRPREFIX(def->os.machine, "virt-")) + if (STRNEQ(machine, "virt") && + !STRPREFIX(machine, "virt-")) return false; return true; @@ -5944,13 +5946,14 @@ qemuDomainMachineIsVirt(const virDomainDef *def) bool -qemuDomainMachineIsPSeries(const virDomainDef *def) +qemuDomainMachineIsPSeries(const char *machine, + const virArch arch) { - if (!ARCH_IS_PPC64(def->os.arch)) + if (!ARCH_IS_PPC64(arch)) return false; - if (STRNEQ(def->os.machine, "pseries") && - !STRPREFIX(def->os.machine, "pseries-")) + if (STRNEQ(machine, "pseries") && + !STRPREFIX(machine, "pseries-")) return false; return true; @@ -6152,12 +6155,12 @@ qemuDomainDefValidateMemoryHotplug(const virDomainDef *def, bool -qemuDomainMachineHasBuiltinIDE(const virDomainDef *def) +qemuDomainMachineHasBuiltinIDE(const char *machine) { - return qemuDomainMachineIsI440FX(def) || - STREQ(def->os.machine, "malta") || - STREQ(def->os.machine, "sun4u") || - STREQ(def->os.machine, "g3beige"); + return qemuDomainMachineIsI440FX(machine) || + STREQ(machine, "malta") || + STREQ(machine, "sun4u") || + STREQ(machine, "g3beige"); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index caac5d5503..f66510da70 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -675,15 +675,15 @@ void qemuDomainMemoryDeviceAlignSize(virDomainDefPtr def, virDomainChrDefPtr qemuFindAgentConfig(virDomainDefPtr def); -bool qemuDomainMachineIsQ35(const virDomainDef *def); -bool qemuDomainMachineIsI440FX(const virDomainDef *def); +bool qemuDomainMachineIsQ35(const char *machine); +bool qemuDomainMachineIsI440FX(const char *machine); bool qemuDomainMachineHasPCIRoot(const virDomainDef *def); bool qemuDomainMachineHasPCIeRoot(const virDomainDef *def); -bool qemuDomainMachineNeedsFDC(const virDomainDef *def); -bool qemuDomainMachineIsS390CCW(const virDomainDef *def); -bool qemuDomainMachineIsVirt(const virDomainDef *def); -bool qemuDomainMachineIsPSeries(const virDomainDef *def); -bool qemuDomainMachineHasBuiltinIDE(const virDomainDef *def); +bool qemuDomainMachineNeedsFDC(const char *machine); +bool qemuDomainMachineIsS390CCW(const char *machine); +bool qemuDomainMachineIsVirt(const char *machine, const virArch arch); +bool qemuDomainMachineIsPSeries(const char *machine, const virArch arch); +bool qemuDomainMachineHasBuiltinIDE(const char *machine); int qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver, virDomainObjPtr vm); diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 22d8bf67d9..2df5d05d03 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -88,7 +88,7 @@ qemuDomainSetSCSIControllerModel(const virDomainDef *def, return -1; } } else { - if (qemuDomainMachineIsPSeries(def)) { + if (qemuDomainMachineIsPSeries(def->os.machine, def->os.arch)) { *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI; } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) { *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC; @@ -245,7 +245,7 @@ qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, for (i = 0; i < def->nserials; i++) { if (def->serials[i]->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && - qemuDomainMachineIsPSeries(def)) + qemuDomainMachineIsPSeries(def->os.machine, def->os.arch)) def->serials[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; if (qemuDomainAssignSpaprVIOAddress(def, &def->serials[i]->info, VIO_ADDR_SERIAL) < 0) @@ -253,7 +253,7 @@ qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, } if (def->nvram) { - if (qemuDomainMachineIsPSeries(def)) + if (qemuDomainMachineIsPSeries(def->os.machine, def->os.arch)) def->nvram->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; if (qemuDomainAssignSpaprVIOAddress(def, &def->nvram->info, VIO_ADDR_NVRAM) < 0) @@ -375,7 +375,7 @@ qemuDomainAssignS390Addresses(virDomainDefPtr def, int ret = -1; virDomainCCWAddressSetPtr addrs = NULL; - if (qemuDomainMachineIsS390CCW(def) && + if (qemuDomainMachineIsS390CCW(def->os.machine) && virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { qemuDomainPrimeVirtioDeviceAddresses( def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW); @@ -445,7 +445,7 @@ qemuDomainAssignARMVirtioMMIOAddresses(virDomainDefPtr def, return; if (!(STRPREFIX(def->os.machine, "vexpress-") || - qemuDomainMachineIsVirt(def))) + qemuDomainMachineIsVirt(def->os.machine, def->os.arch))) return; /* We use virtio-mmio by default on mach-virt guests only if they already @@ -1479,12 +1479,12 @@ qemuDomainValidateDevicePCISlotsChipsets(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, virDomainPCIAddressSetPtr addrs) { - if (qemuDomainMachineIsI440FX(def) && + if (qemuDomainMachineIsI440FX(def->os.machine) && qemuDomainValidateDevicePCISlotsPIIX3(def, qemuCaps, addrs) < 0) { return -1; } - if (qemuDomainMachineIsQ35(def) && + if (qemuDomainMachineIsQ35(def->os.machine) && qemuDomainValidateDevicePCISlotsQ35(def, qemuCaps, addrs) < 0) { return -1; } @@ -1845,7 +1845,7 @@ qemuDomainSupportsPCI(virDomainDefPtr def, if (STREQ(def->os.machine, "versatilepb")) return true; - if (qemuDomainMachineIsVirt(def) && + if (qemuDomainMachineIsVirt(def->os.machine, def->os.arch) && virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_GPEX)) return true; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index a6dac6f095..36cfe675c2 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -329,7 +329,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, qemuDomainSecretInfoPtr encinfo; if (!disk->info.type) { - if (qemuDomainMachineIsS390CCW(vm->def) && + if (qemuDomainMachineIsS390CCW(vm->def->os.machine) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW; else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) @@ -497,7 +497,7 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver, } if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { - if (qemuDomainMachineIsS390CCW(vm->def) && + if (qemuDomainMachineIsS390CCW(vm->def->os.machine) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) controller->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW; else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) @@ -1141,7 +1141,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, goto cleanup; } - if (qemuDomainMachineIsS390CCW(vm->def) && + if (qemuDomainMachineIsS390CCW(vm->def->os.machine) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW; if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def))) @@ -2079,7 +2079,7 @@ qemuDomainAttachRNGDevice(virConnectPtr conn, goto cleanup; if (rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { - if (qemuDomainMachineIsS390CCW(vm->def) && + if (qemuDomainMachineIsS390CCW(vm->def->os.machine) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { rng->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW; } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) { @@ -2620,7 +2620,7 @@ qemuDomainAttachSCSIVHostDevice(virQEMUDriverPtr driver, goto cleanup; if (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { - if (qemuDomainMachineIsS390CCW(vm->def) && + if (qemuDomainMachineIsS390CCW(vm->def->os.machine) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) hostdev->info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW; } @@ -4447,7 +4447,7 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, goto cleanup; } - if (qemuDomainMachineIsS390CCW(vm->def) && + if (qemuDomainMachineIsS390CCW(vm->def->os.machine) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { if (!virDomainDeviceAddressIsValid(&detach->info, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) { @@ -5007,7 +5007,7 @@ qemuDomainDetachNetDevice(virQEMUDriverPtr driver, virDomainNetGetActualHostdev(detach)); goto cleanup; } - if (qemuDomainMachineIsS390CCW(vm->def) && + if (qemuDomainMachineIsS390CCW(vm->def->os.machine) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { if (!virDomainDeviceAddressIsValid(&detach->info, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) { diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c index fc176c1681..4627e85751 100644 --- a/src/qemu/qemu_parse_command.c +++ b/src/qemu/qemu_parse_command.c @@ -654,7 +654,7 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, if (VIR_ALLOC(def->src) < 0) goto error; - if (qemuDomainMachineIsPSeries(dom)) + if (qemuDomainMachineIsPSeries(dom->os.machine, dom->os.arch)) def->bus = VIR_DOMAIN_DISK_BUS_SCSI; else def->bus = VIR_DOMAIN_DISK_BUS_IDE; @@ -746,7 +746,7 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, } else if (STREQ(keywords[i], "if")) { if (STREQ(values[i], "ide")) { def->bus = VIR_DOMAIN_DISK_BUS_IDE; - if (qemuDomainMachineIsPSeries(dom)) { + if (qemuDomainMachineIsPSeries(dom->os.machine, dom->os.arch)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("pseries systems do not support ide devices '%s'"), val); goto error; @@ -1950,7 +1950,7 @@ qemuParseCommandLine(virCapsPtr caps, } if (STREQ(arg, "-cdrom")) { disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM; - if (qemuDomainMachineIsPSeries(def)) + if (qemuDomainMachineIsPSeries(def->os.machine, def->os.arch)) disk->bus = VIR_DOMAIN_DISK_BUS_SCSI; if (VIR_STRDUP(disk->dst, "hdc") < 0) goto error; @@ -1965,7 +1965,7 @@ qemuParseCommandLine(virCapsPtr caps, disk->bus = VIR_DOMAIN_DISK_BUS_IDE; else disk->bus = VIR_DOMAIN_DISK_BUS_SCSI; - if (qemuDomainMachineIsPSeries(def)) + if (qemuDomainMachineIsPSeries(def->os.machine, def->os.arch)) disk->bus = VIR_DOMAIN_DISK_BUS_SCSI; } if (VIR_STRDUP(disk->dst, arg + 1) < 0) -- 2.12.2

On Thu, Apr 13, 2017 at 16:57:13 +0200, Pavel Hrdina wrote:
These functions don't require the whole virDomainDef structure, they only need *machine* and *arch*.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_alias.c | 4 +-- src/qemu/qemu_capabilities.c | 10 +++--- src/qemu/qemu_command.c | 42 +++++++++++----------- src/qemu/qemu_domain.c | 79 ++++++++++++++++++++++-------------------- src/qemu/qemu_domain.h | 14 ++++---- src/qemu/qemu_domain_address.c | 16 ++++----- src/qemu/qemu_hotplug.c | 14 ++++---- src/qemu/qemu_parse_command.c | 8 ++--- 8 files changed, 95 insertions(+), 92 deletions(-)
In most cases where this is used you have the domain object so it's more straightfowrward to extract it from that. I'm suggesting that you modify qemuDomainMachineHasBuiltinIDE and friends to be a wrapper which only extracts the os.machine object from def and calls a new helper which will not take the domain (and not contain the word "Domain"). That way you won't have to make all other callers uglier.

On Tue, Apr 18, 2017 at 11:38:09AM +0200, Peter Krempa wrote:
On Thu, Apr 13, 2017 at 16:57:13 +0200, Pavel Hrdina wrote:
These functions don't require the whole virDomainDef structure, they only need *machine* and *arch*.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_alias.c | 4 +-- src/qemu/qemu_capabilities.c | 10 +++--- src/qemu/qemu_command.c | 42 +++++++++++----------- src/qemu/qemu_domain.c | 79 ++++++++++++++++++++++-------------------- src/qemu/qemu_domain.h | 14 ++++---- src/qemu/qemu_domain_address.c | 16 ++++----- src/qemu/qemu_hotplug.c | 14 ++++---- src/qemu/qemu_parse_command.c | 8 ++--- 8 files changed, 95 insertions(+), 92 deletions(-)
In most cases where this is used you have the domain object so it's more straightfowrward to extract it from that.
I'm suggesting that you modify qemuDomainMachineHasBuiltinIDE and friends to be a wrapper which only extracts the os.machine object from def and calls a new helper which will not take the domain (and not contain the word "Domain"). That way you won't have to make all other callers uglier.
I had the same idea to create wrapper just for this case and don't mess with the existing functions. I agree that having nice helper that will extract required variables form virDomainDef is convenient. I'll rework this patch. Thanks, Pavel
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

Introduce new wrapper functions without *Machine* in the function name that take the whole virDomainDef structure as argument and call the existing functions with *Machine* in the function name. Change the arguments of existing functions to *machine* and *arch* because they don't need the whole virDomainDef structure and they could be used in places where we don't have virDomainDef. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- New in v2: - add a wrappers that extract required data from virDomainDef structure src/qemu/qemu_alias.c | 4 +- src/qemu/qemu_capabilities.c | 10 ++-- src/qemu/qemu_command.c | 42 ++++++------- src/qemu/qemu_domain.c | 131 ++++++++++++++++++++++++++++------------- src/qemu/qemu_domain.h | 28 ++++++--- src/qemu/qemu_domain_address.c | 24 ++++---- src/qemu/qemu_hotplug.c | 14 ++--- src/qemu/qemu_parse_command.c | 8 +-- 8 files changed, 161 insertions(+), 100 deletions(-) diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index 05e1293ef0..914b2b94d8 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -152,14 +152,14 @@ qemuAssignDeviceControllerAlias(virDomainDefPtr domainDef, * first (and currently only) IDE controller is an integrated * controller hardcoded with id "ide" */ - if (qemuDomainMachineHasBuiltinIDE(domainDef) && + if (qemuDomainHasBuiltinIDE(domainDef) && controller->idx == 0) return VIR_STRDUP(controller->info.alias, "ide"); } else if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA) { /* for any Q35 machine, the first SATA controller is the * integrated one, and it too is hardcoded with id "ide" */ - if (qemuDomainMachineIsQ35(domainDef) && controller->idx == 0) + if (qemuDomainIsQ35(domainDef) && controller->idx == 0) return VIR_STRDUP(controller->info.alias, "ide"); } else if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_USB) { /* first USB device is "usb", others are normal "usb%d" */ diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 3ab99f450e..6d6bbf2467 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2315,7 +2315,7 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps, /* If 'virt' supports PCI, it supports multibus. * No extra conditions here for simplicity. */ - if (qemuDomainMachineIsVirt(def)) + if (qemuDomainIsVirt(def)) return true; return false; @@ -5369,7 +5369,7 @@ virQEMUCapsSupportsChardev(const virDomainDef *def, return false; if ((def->os.arch == VIR_ARCH_PPC) || ARCH_IS_PPC64(def->os.arch)) { - if (!qemuDomainMachineIsPSeries(def)) + if (!qemuDomainIsPSeries(def)) return false; /* only pseries need -device spapr-vty with -chardev */ if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && @@ -5396,8 +5396,8 @@ virQEMUCapsSupportsVmport(virQEMUCapsPtr qemuCaps, if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_VMPORT_OPT)) return false; - return qemuDomainMachineIsI440FX(def) || - qemuDomainMachineIsQ35(def) || + return qemuDomainIsI440FX(def) || + qemuDomainIsQ35(def) || STREQ(def->os.machine, "isapc"); } @@ -5409,7 +5409,7 @@ virQEMUCapsSupportsSMM(virQEMUCapsPtr qemuCaps, if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_SMM_OPT)) return false; - return qemuDomainMachineIsQ35(def); + return qemuDomainIsQ35(def); } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 57654246c0..3d64e3cc4d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1390,7 +1390,7 @@ qemuCheckCCWS390AddressSupport(const virDomainDef *def, const char *devicename) { if (info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { - if (!qemuDomainMachineIsS390CCW(def)) { + if (!qemuDomainIsS390CCW(def)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("cannot use CCW address type for device " "'%s' using machine type '%s'"), @@ -2294,7 +2294,7 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, /* PowerPC pseries based VMs do not support floppy device */ if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY && - qemuDomainMachineIsPSeries(def)) { + qemuDomainIsPSeries(def)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("PowerPC pseries machines do not support floppy device")); return -1; @@ -2345,7 +2345,7 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, disk->info.alias) < 0) return -1; - if (!qemuDomainMachineNeedsFDC(def)) { + if (!qemuDomainNeedsFDC(def)) { virCommandAddArg(cmd, "-global"); virCommandAddArgFormat(cmd, "isa-fdc.%s", optstr); } else { @@ -2360,7 +2360,7 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, bootindex) < 0) return -1; - if (!qemuDomainMachineNeedsFDC(def)) { + if (!qemuDomainNeedsFDC(def)) { virCommandAddArg(cmd, "-global"); virCommandAddArgFormat(cmd, "isa-fdc.%s", optstr); } else { @@ -3076,7 +3076,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, * specified, or one with a single IDE contraller had multiple * ide controllers specified. */ - if (qemuDomainMachineHasBuiltinIDE(domainDef)) + if (qemuDomainHasBuiltinIDE(domainDef)) virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Only a single IDE controller is supported " "for this machine type")); @@ -3174,18 +3174,18 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd, /* first SATA controller on Q35 machines is implicit */ if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA && - cont->idx == 0 && qemuDomainMachineIsQ35(def)) + cont->idx == 0 && qemuDomainIsQ35(def)) continue; /* first IDE controller is implicit on various machines */ if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE && - cont->idx == 0 && qemuDomainMachineHasBuiltinIDE(def)) + cont->idx == 0 && qemuDomainHasBuiltinIDE(def)) continue; if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB && cont->model == -1 && - !qemuDomainMachineIsQ35(def) && - !qemuDomainMachineIsVirt(def)) { + !qemuDomainIsQ35(def) && + !qemuDomainIsVirt(def)) { /* An appropriate default USB controller model should already * have been selected in qemuDomainDeviceDefPostParse(); if @@ -3222,8 +3222,8 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd, * not to add one either. Add a legacy USB controller, unless we're * creating a kind of guest we want to keep legacy-free */ if (usbcontroller == 0 && - !qemuDomainMachineIsQ35(def) && - !qemuDomainMachineIsVirt(def) && + !qemuDomainIsQ35(def) && + !qemuDomainIsVirt(def) && !ARCH_IS_S390(def->os.arch)) virCommandAddArg(cmd, "-usb"); @@ -4104,7 +4104,7 @@ qemuBuildNVRAMCommandLine(virCommandPtr cmd, if (!def->nvram) return 0; - if (qemuDomainMachineIsPSeries(def)) { + if (qemuDomainIsPSeries(def)) { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVRAM)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("nvram device is not supported by " @@ -6487,7 +6487,7 @@ qemuBuildPMCommandLine(virCommandPtr cmd, if (def->pm.s3) { const char *pm_object = "PIIX4_PM"; - if (qemuDomainMachineIsQ35(def) && + if (qemuDomainIsQ35(def) && virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_DISABLE_S3)) { pm_object = "ICH9-LPC"; } else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX_DISABLE_S3)) { @@ -6504,7 +6504,7 @@ qemuBuildPMCommandLine(virCommandPtr cmd, if (def->pm.s4) { const char *pm_object = "PIIX4_PM"; - if (qemuDomainMachineIsQ35(def) && + if (qemuDomainIsQ35(def) && virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_DISABLE_S4)) { pm_object = "ICH9-LPC"; } else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX_DISABLE_S4)) { @@ -6693,7 +6693,7 @@ qemuBuildIOMMUCommandLine(virCommandPtr cmd, virDomainIOMMUModelTypeToString(iommu->model)); return -1; } - if (!qemuDomainMachineIsQ35(def)) { + if (!qemuDomainIsQ35(def)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("IOMMU device: '%s' is only supported with " "Q35 machines"), @@ -6732,13 +6732,13 @@ qemuBuildGlobalControllerCommandLine(virCommandPtr cmd, case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: hoststr = "i440FX-pcihost"; cap = virQEMUCapsGet(qemuCaps, QEMU_CAPS_I440FX_PCI_HOLE64_SIZE); - machine = qemuDomainMachineIsI440FX(def); + machine = qemuDomainIsI440FX(def); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: hoststr = "q35-pcihost"; cap = virQEMUCapsGet(qemuCaps, QEMU_CAPS_Q35_PCI_HOLE64_SIZE); - machine = qemuDomainMachineIsQ35(def); + machine = qemuDomainIsQ35(def); break; default: @@ -7323,7 +7323,7 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, if (def->features[VIR_DOMAIN_FEATURE_GIC] == VIR_TRISTATE_SWITCH_ON) { if (def->gic_version != VIR_GIC_VERSION_NONE) { - if (!qemuDomainMachineIsVirt(def)) { + if (!qemuDomainIsVirt(def)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("gic-version option is available " "only for ARM virt machine")); @@ -7352,7 +7352,7 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_IOMMU)) { switch (def->iommu->model) { case VIR_DOMAIN_IOMMU_MODEL_INTEL: - if (!qemuDomainMachineIsQ35(def)) { + if (!qemuDomainIsQ35(def)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("IOMMU device: '%s' is only supported with " "Q35 machines"), @@ -9615,7 +9615,7 @@ qemuBuildPanicCommandLine(virCommandPtr cmd, /* For pSeries guests, the firmware provides the same * functionality as the pvpanic device. The address * cannot be configured by the user */ - if (!qemuDomainMachineIsPSeries(def)) { + if (!qemuDomainIsPSeries(def)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("only pSeries guests support panic device " "of model 'pseries'")); @@ -10041,7 +10041,7 @@ qemuBuildSerialChrDeviceStr(char **deviceStr, { virBuffer cmd = VIR_BUFFER_INITIALIZER; - if (qemuDomainMachineIsPSeries(def)) { + if (qemuDomainIsPSeries(def)) { if (serial->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && serial->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO) { virBufferAsprintf(&cmd, "spapr-vty,chardev=char%s", diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 39bc8c7483..62f56c227d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2365,7 +2365,7 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, addDefaultUSB = false; break; } - if (qemuDomainMachineIsQ35(def)) { + if (qemuDomainIsQ35(def)) { addPCIeRoot = true; addImplicitSATA = true; @@ -2382,7 +2382,7 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, addDefaultUSB = false; break; } - if (qemuDomainMachineIsI440FX(def)) + if (qemuDomainIsI440FX(def)) addPCIRoot = true; break; @@ -2390,7 +2390,7 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, case VIR_ARCH_AARCH64: addDefaultUSB = false; addDefaultMemballoon = false; - if (qemuDomainMachineIsVirt(def)) + if (qemuDomainIsVirt(def)) addPCIeRoot = virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_GPEX); break; @@ -2402,7 +2402,7 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, /* For pSeries guests, the firmware provides the same * functionality as the pvpanic device, so automatically * add the definition if not already present */ - if (qemuDomainMachineIsPSeries(def)) + if (qemuDomainIsPSeries(def)) addPanicDevice = true; break; @@ -2552,7 +2552,7 @@ qemuDomainDefEnableDefaultFeatures(virDomainDefPtr def, * was not included in the domain XML, we need to choose a suitable * GIC version ourselves */ if (def->features[VIR_DOMAIN_FEATURE_GIC] == VIR_TRISTATE_SWITCH_ABSENT && - qemuDomainMachineIsVirt(def)) { + qemuDomainIsVirt(def)) { VIR_DEBUG("Looking for usable GIC version in domain capabilities"); for (version = VIR_GIC_VERSION_LAST - 1; @@ -2940,7 +2940,7 @@ qemuDomainDefValidate(const virDomainDef *def, /* These are the QEMU implementation limitations. But we * have to live with them for now. */ - if (!qemuDomainMachineIsQ35(def)) { + if (!qemuDomainIsQ35(def)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Secure boot is supported with q35 machine types only")); goto cleanup; @@ -3039,7 +3039,7 @@ qemuDomainDefaultNetModel(const virDomainDef *def, if (STREQ(def->os.machine, "versatilepb")) return "smc91c111"; - if (qemuDomainMachineIsVirt(def)) + if (qemuDomainIsVirt(def)) return "virtio"; /* Incomplete. vexpress (and a few others) use this, but not all @@ -3198,14 +3198,14 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont, case VIR_DOMAIN_CONTROLLER_TYPE_PCI: if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS && - !qemuDomainMachineIsI440FX(def)) { + !qemuDomainIsI440FX(def)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("pci-expander-bus controllers are only supported " "on 440fx-based machinetypes")); return -1; } if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS && - !qemuDomainMachineIsQ35(def)) { + !qemuDomainIsQ35(def)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("pcie-expander-bus controllers are only supported " "on q35-based machinetypes")); @@ -3360,7 +3360,7 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, if (dev->type == VIR_DOMAIN_DEVICE_PANIC && dev->data.panic->model == VIR_DOMAIN_PANIC_MODEL_DEFAULT) { - if (qemuDomainMachineIsPSeries(def)) + if (qemuDomainIsPSeries(def)) dev->data.panic->model = VIR_DOMAIN_PANIC_MODEL_PSERIES; else if (ARCH_IS_S390(def->os.arch)) dev->data.panic->model = VIR_DOMAIN_PANIC_MODEL_S390; @@ -4047,7 +4047,7 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver, * because other architectures and machine types were introduced * when libvirt already supported <controller type='usb'/>. */ - if (ARCH_IS_X86(def->os.arch) && qemuDomainMachineIsI440FX(def) && + if (ARCH_IS_X86(def->os.arch) && qemuDomainIsI440FX(def) && usb && usb->idx == 0 && (usb->model == -1 || usb->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI)) { @@ -5855,26 +5855,40 @@ qemuFindAgentConfig(virDomainDefPtr def) bool -qemuDomainMachineIsQ35(const virDomainDef *def) +qemuDomainIsQ35(const virDomainDef *def) { - return (STRPREFIX(def->os.machine, "pc-q35") || - STREQ(def->os.machine, "q35")); + return qemuDomainMachineIsQ35(def->os.machine); } bool -qemuDomainMachineIsI440FX(const virDomainDef *def) +qemuDomainMachineIsQ35(const char *machine) { - return (STREQ(def->os.machine, "pc") || - STRPREFIX(def->os.machine, "pc-0.") || - STRPREFIX(def->os.machine, "pc-1.") || - STRPREFIX(def->os.machine, "pc-i440") || - STRPREFIX(def->os.machine, "rhel")); + return (STRPREFIX(machine, "pc-q35") || + STREQ(machine, "q35")); } bool -qemuDomainMachineHasPCIRoot(const virDomainDef *def) +qemuDomainIsI440FX(const virDomainDef *def) +{ + return qemuDomainMachineIsI440FX(def->os.machine); +} + + +bool +qemuDomainMachineIsI440FX(const char *machine) +{ + return (STREQ(machine, "pc") || + STRPREFIX(machine, "pc-0.") || + STRPREFIX(machine, "pc-1.") || + STRPREFIX(machine, "pc-i440") || + STRPREFIX(machine, "rhel")); +} + + +bool +qemuDomainHasPCIRoot(const virDomainDef *def) { int root = virDomainControllerFind(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0); @@ -5889,7 +5903,7 @@ qemuDomainMachineHasPCIRoot(const virDomainDef *def) bool -qemuDomainMachineHasPCIeRoot(const virDomainDef *def) +qemuDomainHasPCIeRoot(const virDomainDef *def) { int root = virDomainControllerFind(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0); @@ -5904,9 +5918,16 @@ qemuDomainMachineHasPCIeRoot(const virDomainDef *def) bool -qemuDomainMachineNeedsFDC(const virDomainDef *def) +qemuDomainNeedsFDC(const virDomainDef *def) { - char *p = STRSKIP(def->os.machine, "pc-q35-"); + return qemuDomainMachineNeedsFDC(def->os.machine); +} + + +bool +qemuDomainMachineNeedsFDC(const char *machine) +{ + const char *p = STRSKIP(machine, "pc-q35-"); if (p) { if (STRPREFIX(p, "1.") || @@ -5922,21 +5943,36 @@ qemuDomainMachineNeedsFDC(const virDomainDef *def) bool -qemuDomainMachineIsS390CCW(const virDomainDef *def) +qemuDomainIsS390CCW(const virDomainDef *def) { - return STRPREFIX(def->os.machine, "s390-ccw"); + return qemuDomainMachineIsS390CCW(def->os.machine); } bool -qemuDomainMachineIsVirt(const virDomainDef *def) +qemuDomainMachineIsS390CCW(const char *machine) { - if (def->os.arch != VIR_ARCH_ARMV7L && - def->os.arch != VIR_ARCH_AARCH64) + return STRPREFIX(machine, "s390-ccw"); +} + + +bool +qemuDomainIsVirt(const virDomainDef *def) +{ + return qemuDomainMachineIsVirt(def->os.machine, def->os.arch); +} + + +bool +qemuDomainMachineIsVirt(const char *machine, + const virArch arch) +{ + if (arch != VIR_ARCH_ARMV7L && + arch != VIR_ARCH_AARCH64) return false; - if (STRNEQ(def->os.machine, "virt") && - !STRPREFIX(def->os.machine, "virt-")) + if (STRNEQ(machine, "virt") && + !STRPREFIX(machine, "virt-")) return false; return true; @@ -5944,13 +5980,21 @@ qemuDomainMachineIsVirt(const virDomainDef *def) bool -qemuDomainMachineIsPSeries(const virDomainDef *def) +qemuDomainIsPSeries(const virDomainDef *def) { - if (!ARCH_IS_PPC64(def->os.arch)) + return qemuDomainMachineIsPSeries(def->os.machine, def->os.arch); +} + + +bool +qemuDomainMachineIsPSeries(const char *machine, + const virArch arch) +{ + if (!ARCH_IS_PPC64(arch)) return false; - if (STRNEQ(def->os.machine, "pseries") && - !STRPREFIX(def->os.machine, "pseries-")) + if (STRNEQ(machine, "pseries") && + !STRPREFIX(machine, "pseries-")) return false; return true; @@ -6152,12 +6196,19 @@ qemuDomainDefValidateMemoryHotplug(const virDomainDef *def, bool -qemuDomainMachineHasBuiltinIDE(const virDomainDef *def) +qemuDomainHasBuiltinIDE(const virDomainDef *def) { - return qemuDomainMachineIsI440FX(def) || - STREQ(def->os.machine, "malta") || - STREQ(def->os.machine, "sun4u") || - STREQ(def->os.machine, "g3beige"); + return qemuDomainMachineHasBuiltinIDE(def->os.machine); +} + + +bool +qemuDomainMachineHasBuiltinIDE(const char *machine) +{ + return qemuDomainMachineIsI440FX(machine) || + STREQ(machine, "malta") || + STREQ(machine, "sun4u") || + STREQ(machine, "g3beige"); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index caac5d5503..036ae12933 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -675,15 +675,25 @@ void qemuDomainMemoryDeviceAlignSize(virDomainDefPtr def, virDomainChrDefPtr qemuFindAgentConfig(virDomainDefPtr def); -bool qemuDomainMachineIsQ35(const virDomainDef *def); -bool qemuDomainMachineIsI440FX(const virDomainDef *def); -bool qemuDomainMachineHasPCIRoot(const virDomainDef *def); -bool qemuDomainMachineHasPCIeRoot(const virDomainDef *def); -bool qemuDomainMachineNeedsFDC(const virDomainDef *def); -bool qemuDomainMachineIsS390CCW(const virDomainDef *def); -bool qemuDomainMachineIsVirt(const virDomainDef *def); -bool qemuDomainMachineIsPSeries(const virDomainDef *def); -bool qemuDomainMachineHasBuiltinIDE(const virDomainDef *def); +bool qemuDomainIsQ35(const virDomainDef *def); +bool qemuDomainIsI440FX(const virDomainDef *def); +bool qemuDomainHasPCIRoot(const virDomainDef *def); +bool qemuDomainHasPCIeRoot(const virDomainDef *def); +bool qemuDomainNeedsFDC(const virDomainDef *def); +bool qemuDomainIsS390CCW(const virDomainDef *def); +bool qemuDomainIsVirt(const virDomainDef *def); +bool qemuDomainIsPSeries(const virDomainDef *def); +bool qemuDomainHasBuiltinIDE(const virDomainDef *def); + +bool qemuDomainMachineIsQ35(const char *machine); +bool qemuDomainMachineIsI440FX(const char *machine); +bool qemuDomainMachineNeedsFDC(const char *machine); +bool qemuDomainMachineIsS390CCW(const char *machine); +bool qemuDomainMachineIsVirt(const char *machine, + const virArch arch); +bool qemuDomainMachineIsPSeries(const char *machine, + const virArch arch); +bool qemuDomainMachineHasBuiltinIDE(const char *machine); int qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver, virDomainObjPtr vm); diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 22d8bf67d9..064d05079c 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -88,7 +88,7 @@ qemuDomainSetSCSIControllerModel(const virDomainDef *def, return -1; } } else { - if (qemuDomainMachineIsPSeries(def)) { + if (qemuDomainIsPSeries(def)) { *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI; } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) { *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC; @@ -245,7 +245,7 @@ qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, for (i = 0; i < def->nserials; i++) { if (def->serials[i]->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && - qemuDomainMachineIsPSeries(def)) + qemuDomainIsPSeries(def)) def->serials[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; if (qemuDomainAssignSpaprVIOAddress(def, &def->serials[i]->info, VIO_ADDR_SERIAL) < 0) @@ -253,7 +253,7 @@ qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, } if (def->nvram) { - if (qemuDomainMachineIsPSeries(def)) + if (qemuDomainIsPSeries(def)) def->nvram->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; if (qemuDomainAssignSpaprVIOAddress(def, &def->nvram->info, VIO_ADDR_NVRAM) < 0) @@ -375,7 +375,7 @@ qemuDomainAssignS390Addresses(virDomainDefPtr def, int ret = -1; virDomainCCWAddressSetPtr addrs = NULL; - if (qemuDomainMachineIsS390CCW(def) && + if (qemuDomainIsS390CCW(def) && virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { qemuDomainPrimeVirtioDeviceAddresses( def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW); @@ -445,13 +445,13 @@ qemuDomainAssignARMVirtioMMIOAddresses(virDomainDefPtr def, return; if (!(STRPREFIX(def->os.machine, "vexpress-") || - qemuDomainMachineIsVirt(def))) + qemuDomainIsVirt(def))) return; /* We use virtio-mmio by default on mach-virt guests only if they already * have at least one virtio-mmio device: in all other cases, we prefer * virtio-pci */ - if (qemuDomainMachineHasPCIeRoot(def) && + if (qemuDomainHasPCIeRoot(def) && !qemuDomainHasVirtioMMIODevices(def)) { qemuDomainPrimeVirtioDeviceAddresses(def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI); @@ -826,7 +826,7 @@ qemuDomainFillDevicePCIConnectFlagsIterInit(virDomainDefPtr def, data->driver = driver; - if (qemuDomainMachineHasPCIeRoot(def)) { + if (qemuDomainHasPCIeRoot(def)) { data->pcieFlags = (VIR_PCI_CONNECT_TYPE_PCIE_DEVICE | VIR_PCI_CONNECT_HOTPLUGGABLE); } else { @@ -1479,12 +1479,12 @@ qemuDomainValidateDevicePCISlotsChipsets(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, virDomainPCIAddressSetPtr addrs) { - if (qemuDomainMachineIsI440FX(def) && + if (qemuDomainIsI440FX(def) && qemuDomainValidateDevicePCISlotsPIIX3(def, qemuCaps, addrs) < 0) { return -1; } - if (qemuDomainMachineIsQ35(def) && + if (qemuDomainIsQ35(def) && qemuDomainValidateDevicePCISlotsQ35(def, qemuCaps, addrs) < 0) { return -1; } @@ -1845,7 +1845,7 @@ qemuDomainSupportsPCI(virDomainDefPtr def, if (STREQ(def->os.machine, "versatilepb")) return true; - if (qemuDomainMachineIsVirt(def) && + if (qemuDomainIsVirt(def) && virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_GPEX)) return true; @@ -2024,7 +2024,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, * all *actual* devices. */ - if (qemuDomainMachineHasPCIRoot(def)) { + if (qemuDomainHasPCIRoot(def)) { /* This is a dummy info used to reserve a slot for a * legacy PCI device that doesn't exist, but may in the * future, e.g. if another device is hotplugged into the @@ -2066,7 +2066,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, if (max_idx <= 0 && addrs->nbuses > max_idx + 1 && - qemuDomainMachineHasPCIeRoot(def)) { + qemuDomainHasPCIeRoot(def)) { virDomainDeviceInfo info = { .pciConnectFlags = (VIR_PCI_CONNECT_HOTPLUGGABLE | VIR_PCI_CONNECT_TYPE_PCIE_DEVICE) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index a6dac6f095..37b8d455c7 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -329,7 +329,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, qemuDomainSecretInfoPtr encinfo; if (!disk->info.type) { - if (qemuDomainMachineIsS390CCW(vm->def) && + if (qemuDomainIsS390CCW(vm->def) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW; else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) @@ -497,7 +497,7 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver, } if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { - if (qemuDomainMachineIsS390CCW(vm->def) && + if (qemuDomainIsS390CCW(vm->def) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) controller->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW; else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) @@ -1141,7 +1141,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, goto cleanup; } - if (qemuDomainMachineIsS390CCW(vm->def) && + if (qemuDomainIsS390CCW(vm->def) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW; if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def))) @@ -2079,7 +2079,7 @@ qemuDomainAttachRNGDevice(virConnectPtr conn, goto cleanup; if (rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { - if (qemuDomainMachineIsS390CCW(vm->def) && + if (qemuDomainIsS390CCW(vm->def) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { rng->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW; } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) { @@ -2620,7 +2620,7 @@ qemuDomainAttachSCSIVHostDevice(virQEMUDriverPtr driver, goto cleanup; if (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { - if (qemuDomainMachineIsS390CCW(vm->def) && + if (qemuDomainIsS390CCW(vm->def) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) hostdev->info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW; } @@ -4447,7 +4447,7 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, goto cleanup; } - if (qemuDomainMachineIsS390CCW(vm->def) && + if (qemuDomainIsS390CCW(vm->def) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { if (!virDomainDeviceAddressIsValid(&detach->info, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) { @@ -5007,7 +5007,7 @@ qemuDomainDetachNetDevice(virQEMUDriverPtr driver, virDomainNetGetActualHostdev(detach)); goto cleanup; } - if (qemuDomainMachineIsS390CCW(vm->def) && + if (qemuDomainIsS390CCW(vm->def) && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { if (!virDomainDeviceAddressIsValid(&detach->info, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) { diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c index fc176c1681..af9063c026 100644 --- a/src/qemu/qemu_parse_command.c +++ b/src/qemu/qemu_parse_command.c @@ -654,7 +654,7 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, if (VIR_ALLOC(def->src) < 0) goto error; - if (qemuDomainMachineIsPSeries(dom)) + if (qemuDomainIsPSeries(dom)) def->bus = VIR_DOMAIN_DISK_BUS_SCSI; else def->bus = VIR_DOMAIN_DISK_BUS_IDE; @@ -746,7 +746,7 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, } else if (STREQ(keywords[i], "if")) { if (STREQ(values[i], "ide")) { def->bus = VIR_DOMAIN_DISK_BUS_IDE; - if (qemuDomainMachineIsPSeries(dom)) { + if (qemuDomainIsPSeries(dom)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("pseries systems do not support ide devices '%s'"), val); goto error; @@ -1950,7 +1950,7 @@ qemuParseCommandLine(virCapsPtr caps, } if (STREQ(arg, "-cdrom")) { disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM; - if (qemuDomainMachineIsPSeries(def)) + if (qemuDomainIsPSeries(def)) disk->bus = VIR_DOMAIN_DISK_BUS_SCSI; if (VIR_STRDUP(disk->dst, "hdc") < 0) goto error; @@ -1965,7 +1965,7 @@ qemuParseCommandLine(virCapsPtr caps, disk->bus = VIR_DOMAIN_DISK_BUS_IDE; else disk->bus = VIR_DOMAIN_DISK_BUS_SCSI; - if (qemuDomainMachineIsPSeries(def)) + if (qemuDomainIsPSeries(def)) disk->bus = VIR_DOMAIN_DISK_BUS_SCSI; } if (VIR_STRDUP(disk->dst, arg + 1) < 0) -- 2.12.2

On Tue, Apr 18, 2017 at 13:37:26 +0200, Pavel Hrdina wrote:
Introduce new wrapper functions without *Machine* in the function name that take the whole virDomainDef structure as argument and call the existing functions with *Machine* in the function name.
Change the arguments of existing functions to *machine* and *arch* because they don't need the whole virDomainDef structure and they could be used in places where we don't have virDomainDef.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> ---
New in v2: - add a wrappers that extract required data from virDomainDef structure
This should have been split into the rename patch and then into addition of the new wrappers. Calling this "refactor" is a convenient way to avoid doing that. ACK regardless.

On Tue, Apr 18, 2017 at 01:45:31PM +0200, Peter Krempa wrote:
On Tue, Apr 18, 2017 at 13:37:26 +0200, Pavel Hrdina wrote:
Introduce new wrapper functions without *Machine* in the function name that take the whole virDomainDef structure as argument and call the existing functions with *Machine* in the function name.
Change the arguments of existing functions to *machine* and *arch* because they don't need the whole virDomainDef structure and they could be used in places where we don't have virDomainDef.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> ---
New in v2: - add a wrappers that extract required data from virDomainDef structure
This should have been split into the rename patch and then into addition of the new wrappers. Calling this "refactor" is a convenient way to avoid doing that.
And I almost got away with it :).
ACK regardless.
Thanks

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_capabilities.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 06f122ed0d..ba5cb19427 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5544,8 +5544,7 @@ virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr qemuCaps, VIR_DOMAIN_DISK_DEVICE_LUN); /* PowerPC pseries based VMs do not support floppy device */ - if (!ARCH_IS_PPC64(qemuCaps->arch) || - (STRNEQ(machine, "pseries") && !STRPREFIX(machine, "pseries-"))) + if (!qemuDomainMachineIsPSeries(machine, qemuCaps->arch)) VIR_DOMAIN_CAPS_ENUM_SET(disk->diskDevice, VIR_DOMAIN_DISK_DEVICE_FLOPPY); VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, @@ -5555,8 +5554,7 @@ virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr qemuCaps, /* VIR_DOMAIN_DISK_BUS_SD */); /* PowerPC pseries based VMs do not support floppy device */ - if (!ARCH_IS_PPC64(qemuCaps->arch) || - (STRNEQ(machine, "pseries") && !STRPREFIX(machine, "pseries-"))) + if (!qemuDomainMachineIsPSeries(machine, qemuCaps->arch)) VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, VIR_DOMAIN_DISK_BUS_FDC); if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE)) -- 2.12.2

On Thu, Apr 13, 2017 at 16:57:14 +0200, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_capabilities.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
ACK

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1441964 Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_capabilities.c | 4 +++- tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml | 1 - tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml | 1 - tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml | 1 - tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml | 1 - tests/domaincapsschemadata/qemu_2.7.0.s390x.xml | 1 - tests/domaincapsschemadata/qemu_2.8.0.s390x.xml | 1 - 7 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ba5cb19427..5197090825 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5547,8 +5547,10 @@ virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr qemuCaps, if (!qemuDomainMachineIsPSeries(machine, qemuCaps->arch)) VIR_DOMAIN_CAPS_ENUM_SET(disk->diskDevice, VIR_DOMAIN_DISK_DEVICE_FLOPPY); + if (qemuDomainMachineHasBuiltinIDE(machine)) + VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, VIR_DOMAIN_DISK_BUS_IDE); + VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, - VIR_DOMAIN_DISK_BUS_IDE, VIR_DOMAIN_DISK_BUS_SCSI, VIR_DOMAIN_DISK_BUS_VIRTIO, /* VIR_DOMAIN_DISK_BUS_SD */); diff --git a/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml b/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml index 1fa7f6dff8..54b89dc72b 100644 --- a/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml +++ b/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml @@ -63,7 +63,6 @@ <value>lun</value> </enum> <enum name='bus'> - <value>ide</value> <value>fdc</value> <value>scsi</value> <value>virtio</value> diff --git a/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml b/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml index d60fc1df98..60bf2f54f7 100644 --- a/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml +++ b/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml @@ -63,7 +63,6 @@ <value>lun</value> </enum> <enum name='bus'> - <value>ide</value> <value>fdc</value> <value>scsi</value> <value>virtio</value> diff --git a/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml b/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml index fcc6f50e0e..1a980927cf 100644 --- a/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml +++ b/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml @@ -63,7 +63,6 @@ <value>lun</value> </enum> <enum name='bus'> - <value>ide</value> <value>fdc</value> <value>scsi</value> <value>virtio</value> diff --git a/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml b/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml index 755c4f4475..4ecf8651b4 100644 --- a/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml +++ b/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml @@ -37,7 +37,6 @@ <value>lun</value> </enum> <enum name='bus'> - <value>ide</value> <value>scsi</value> <value>virtio</value> <value>usb</value> diff --git a/tests/domaincapsschemadata/qemu_2.7.0.s390x.xml b/tests/domaincapsschemadata/qemu_2.7.0.s390x.xml index 999e2795d8..dc6d2d8f0c 100644 --- a/tests/domaincapsschemadata/qemu_2.7.0.s390x.xml +++ b/tests/domaincapsschemadata/qemu_2.7.0.s390x.xml @@ -32,7 +32,6 @@ <value>lun</value> </enum> <enum name='bus'> - <value>ide</value> <value>fdc</value> <value>scsi</value> <value>virtio</value> diff --git a/tests/domaincapsschemadata/qemu_2.8.0.s390x.xml b/tests/domaincapsschemadata/qemu_2.8.0.s390x.xml index 0b8135bc5c..53c3190f20 100644 --- a/tests/domaincapsschemadata/qemu_2.8.0.s390x.xml +++ b/tests/domaincapsschemadata/qemu_2.8.0.s390x.xml @@ -113,7 +113,6 @@ <value>lun</value> </enum> <enum name='bus'> - <value>ide</value> <value>fdc</value> <value>scsi</value> <value>virtio</value> -- 2.12.2

On Thu, Apr 13, 2017 at 16:57:15 +0200, Pavel Hrdina wrote: Was this the problem that virt-manager allowed to use the IDE bus on Q35?
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1441964
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_capabilities.c | 4 +++- tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml | 1 - tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml | 1 - tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml | 1 - tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml | 1 - tests/domaincapsschemadata/qemu_2.7.0.s390x.xml | 1 - tests/domaincapsschemadata/qemu_2.8.0.s390x.xml | 1 - 7 files changed, 3 insertions(+), 7 deletions(-)
ACK, but I think you should follow up and do the same for the floppy device which AFAIK does not exist on aarch64 and others. There was an attempt to do this but only for ppc in 020a178318 Peter

On Tue, Apr 18, 2017 at 11:31:20AM +0200, Peter Krempa wrote:
On Thu, Apr 13, 2017 at 16:57:15 +0200, Pavel Hrdina wrote:
Was this the problem that virt-manager allowed to use the IDE bus on Q35?
Yes :)
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1441964
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_capabilities.c | 4 +++- tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml | 1 - tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml | 1 - tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml | 1 - tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml | 1 - tests/domaincapsschemadata/qemu_2.7.0.s390x.xml | 1 - tests/domaincapsschemadata/qemu_2.8.0.s390x.xml | 1 - 7 files changed, 3 insertions(+), 7 deletions(-)
ACK,
but I think you should follow up and do the same for the floppy device which AFAIK does not exist on aarch64 and others. There was an attempt to do this but only for ppc in 020a178318
There is a lot of space for improvement, I'll add it to my todo list :) Pavel

On Tue, Apr 18, 2017 at 12:00:12 +0200, Pavel Hrdina wrote:
On Tue, Apr 18, 2017 at 11:31:20AM +0200, Peter Krempa wrote:
On Thu, Apr 13, 2017 at 16:57:15 +0200, Pavel Hrdina wrote:
Was this the problem that virt-manager allowed to use the IDE bus on Q35?
Yes :)
Then please add something like "Since virt-manager (virt-install?) uses this data it would allow to create configuration which would be rejected by libvirt." or something like that ...
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1441964
Since this BZ does not clearly state that fact.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_capabilities.c | 4 +++- tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml | 1 - tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml | 1 - tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml | 1 - tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml | 1 - tests/domaincapsschemadata/qemu_2.7.0.s390x.xml | 1 - tests/domaincapsschemadata/qemu_2.8.0.s390x.xml | 1 - 7 files changed, 3 insertions(+), 7 deletions(-)
ACK,
but I think you should follow up and do the same for the floppy device which AFAIK does not exist on aarch64 and others. There was an attempt to do this but only for ppc in 020a178318
There is a lot of space for improvement, I'll add it to my todo list :)
Yes. There is. As in every other part of libvirt :)

On Tue, Apr 18, 2017 at 12:08:30PM +0200, Peter Krempa wrote:
On Tue, Apr 18, 2017 at 12:00:12 +0200, Pavel Hrdina wrote:
On Tue, Apr 18, 2017 at 11:31:20AM +0200, Peter Krempa wrote:
On Thu, Apr 13, 2017 at 16:57:15 +0200, Pavel Hrdina wrote:
Was this the problem that virt-manager allowed to use the IDE bus on Q35?
Yes :)
Then please add something like "Since virt-manager (virt-install?) uses this data it would allow to create configuration which would be rejected by libvirt." or something like that ...
I don't see any value in mentioning virt-manager in commit message, the domain capability output is wrong regardless of who uses it.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1441964
Since this BZ does not clearly state that fact.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_capabilities.c | 4 +++- tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml | 1 - tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml | 1 - tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml | 1 - tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml | 1 - tests/domaincapsschemadata/qemu_2.7.0.s390x.xml | 1 - tests/domaincapsschemadata/qemu_2.8.0.s390x.xml | 1 - 7 files changed, 3 insertions(+), 7 deletions(-)
ACK,
but I think you should follow up and do the same for the floppy device which AFAIK does not exist on aarch64 and others. There was an attempt to do this but only for ppc in 020a178318
There is a lot of space for improvement, I'll add it to my todo list :)
Yes. There is. As in every other part of libvirt :)

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- .../domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml | 125 +++++++++++++++++++++ tests/domaincapstest.c | 4 + 2 files changed, 129 insertions(+) create mode 100644 tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml diff --git a/tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml b/tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml new file mode 100644 index 0000000000..f6d54d9a12 --- /dev/null +++ b/tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml @@ -0,0 +1,125 @@ +<domainCapabilities> + <path>/usr/bin/qemu-system-x86_64</path> + <domain>kvm</domain> + <machine>pc-q35-2.9</machine> + <arch>x86_64</arch> + <vcpu max='288'/> + <os supported='yes'> + <loader supported='yes'> + <value>/usr/share/AAVMF/AAVMF_CODE.fd</value> + <value>/usr/share/OVMF/OVMF_CODE.fd</value> + <enum name='type'> + <value>rom</value> + <value>pflash</value> + </enum> + <enum name='readonly'> + <value>yes</value> + <value>no</value> + </enum> + </loader> + </os> + <cpu> + <mode name='host-passthrough' supported='yes'/> + <mode name='host-model' supported='yes'> + <model fallback='forbid'>Skylake-Client</model> + <vendor>Intel</vendor> + <feature policy='require' name='ss'/> + <feature policy='require' name='vmx'/> + <feature policy='require' name='hypervisor'/> + <feature policy='require' name='tsc_adjust'/> + <feature policy='require' name='clflushopt'/> + <feature policy='require' name='xsaves'/> + <feature policy='require' name='pdpe1gb'/> + <feature policy='require' name='invtsc'/> + </mode> + <mode name='custom' supported='yes'> + <model usable='yes'>qemu64</model> + <model usable='yes'>qemu32</model> + <model usable='no'>phenom</model> + <model usable='yes'>pentium3</model> + <model usable='yes'>pentium2</model> + <model usable='yes'>pentium</model> + <model usable='yes'>n270</model> + <model usable='yes'>kvm64</model> + <model usable='yes'>kvm32</model> + <model usable='yes'>coreduo</model> + <model usable='yes'>core2duo</model> + <model usable='no'>athlon</model> + <model usable='yes'>Westmere</model> + <model usable='yes'>Skylake-Client</model> + <model usable='yes'>SandyBridge</model> + <model usable='yes'>Penryn</model> + <model usable='no'>Opteron_G5</model> + <model usable='no'>Opteron_G4</model> + <model usable='no'>Opteron_G3</model> + <model usable='yes'>Opteron_G2</model> + <model usable='yes'>Opteron_G1</model> + <model usable='yes'>Nehalem</model> + <model usable='yes'>IvyBridge</model> + <model usable='yes'>Haswell</model> + <model usable='yes'>Haswell-noTSX</model> + <model usable='yes'>Conroe</model> + <model usable='yes'>Broadwell</model> + <model usable='yes'>Broadwell-noTSX</model> + <model usable='yes'>486</model> + </mode> + </cpu> + <devices> + <disk supported='yes'> + <enum name='diskDevice'> + <value>disk</value> + <value>cdrom</value> + <value>floppy</value> + <value>lun</value> + </enum> + <enum name='bus'> + <value>fdc</value> + <value>scsi</value> + <value>virtio</value> + <value>usb</value> + <value>sata</value> + </enum> + </disk> + <graphics supported='yes'> + <enum name='type'> + <value>sdl</value> + <value>vnc</value> + <value>spice</value> + </enum> + </graphics> + <video supported='yes'> + <enum name='modelType'> + <value>vga</value> + <value>cirrus</value> + <value>vmvga</value> + <value>qxl</value> + <value>virtio</value> + </enum> + </video> + <hostdev supported='yes'> + <enum name='mode'> + <value>subsystem</value> + </enum> + <enum name='startupPolicy'> + <value>default</value> + <value>mandatory</value> + <value>requisite</value> + <value>optional</value> + </enum> + <enum name='subsysType'> + <value>usb</value> + <value>pci</value> + <value>scsi</value> + </enum> + <enum name='capsType'/> + <enum name='pciBackend'> + <value>default</value> + <value>kvm</value> + <value>vfio</value> + </enum> + </hostdev> + </devices> + <features> + <gic supported='no'/> + </features> +</domainCapabilities> diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index edf06c4e8e..5a36fcf29d 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -522,6 +522,10 @@ mymain(void) "/usr/bin/qemu-system-x86_64", NULL, "x86_64", VIR_DOMAIN_VIRT_KVM); + DO_TEST_QEMU("2.9.0", "caps_2.9.0", + "/usr/bin/qemu-system-x86_64", "q35", + "x86_64", VIR_DOMAIN_VIRT_KVM); + DO_TEST_QEMU("2.9.0-tcg", "caps_2.9.0", "/usr/bin/qemu-system-x86_64", NULL, "x86_64", VIR_DOMAIN_VIRT_QEMU); -- 2.12.2

On Thu, Apr 13, 2017 at 16:57:16 +0200, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- .../domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml | 125 +++++++++++++++++++++ tests/domaincapstest.c | 4 + 2 files changed, 129 insertions(+) create mode 100644 tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml
diff --git a/tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml b/tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml new file mode 100644 index 0000000000..f6d54d9a12 --- /dev/null +++ b/tests/domaincapsschemadata/qemu_2.9.0-q35.x86_64.xml @@ -0,0 +1,125 @@ +<domainCapabilities>
[...]
+ <cpu>
[...]
+ <mode name='custom' supported='yes'>
[...]
+ <model usable='yes'>Haswell</model> + <model usable='yes'>Haswell-noTSX</model> + <model usable='yes'>Conroe</model> + <model usable='yes'>Broadwell</model> + <model usable='yes'>Broadwell-noTSX</model> + <model usable='yes'>486</model>
Lol. Really? A Q35 machine with 486 CPU?
+ </mode>
ACK
participants (2)
-
Pavel Hrdina
-
Peter Krempa