[libvirt] [PATCH 0/4] Add architecture checks to qemuDomainMachineIs*()
The pseries machine type is only available on ppc64/ppc64le, and in the same way the virt machine type is only available on armv7l/aarch64. Until either machine type is introduced on another architecture, we can perform the architecture checks in a single location instead of repeating them all over the place. Andrea Bolognani (4): qemu: Remove redundant arguments to qemuBuildSerialChrDeviceStr() qemu: Add architecture checks to qemuDomainMachineIsVirt() qemu: Introduce qemuDomainMachineIsPSeries() qemu: Use stricter checks in virQEMUCapsFillDomainDeviceDiskCaps() src/qemu/qemu_capabilities.c | 18 +++++++++--------- src/qemu/qemu_command.c | 25 ++++++++----------------- src/qemu/qemu_domain.c | 31 +++++++++++++++++++++++++------ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_domain_address.c | 9 +++------ src/qemu/qemu_parse_command.c | 12 ++++-------- 6 files changed, 50 insertions(+), 46 deletions(-) -- 2.7.4
Since we're already passing the full virDomainDef, it doesn't make sense to also pass def->os.arch and def->os.machine as separate arguments. --- src/qemu/qemu_command.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6944129..030d84b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9388,13 +9388,11 @@ static int qemuBuildSerialChrDeviceStr(char **deviceStr, const virDomainDef *def, virDomainChrDefPtr serial, - virQEMUCapsPtr qemuCaps, - virArch arch, - char *machine) + virQEMUCapsPtr qemuCaps) { virBuffer cmd = VIR_BUFFER_INITIALIZER; - if (ARCH_IS_PPC64(arch) && STRPREFIX(machine, "pseries")) { + if (ARCH_IS_PPC64(def->os.arch) && STRPREFIX(def->os.machine, "pseries")) { 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", @@ -9569,9 +9567,7 @@ qemuBuildChrDeviceStr(char **deviceStr, switch ((virDomainChrDeviceType) chr->deviceType) { case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL: - ret = qemuBuildSerialChrDeviceStr(deviceStr, vmdef, chr, qemuCaps, - vmdef->os.arch, - vmdef->os.machine); + ret = qemuBuildSerialChrDeviceStr(deviceStr, vmdef, chr, qemuCaps); break; case VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL: -- 2.7.4
On 06/23/2016 04:40 AM, Andrea Bolognani wrote:
Since we're already passing the full virDomainDef, it doesn't make sense to also pass def->os.arch and def->os.machine as separate arguments. --- src/qemu/qemu_command.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)
ACK.
Remove all external architecture checks that have been made redundant by this change. --- src/qemu/qemu_capabilities.c | 12 +++++------- src/qemu/qemu_command.c | 4 +--- src/qemu/qemu_domain.c | 12 +++++++++--- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 4ed5b71..5fcd744 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2174,13 +2174,11 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps, return false; } - if (ARCH_IS_ARM(def->os.arch)) { - /* If 'virt' supports PCI, it supports multibus. - * No extra conditions here for simplicity. - */ - if (qemuDomainMachineIsVirt(def)) - return true; - } + /* If 'virt' supports PCI, it supports multibus. + * No extra conditions here for simplicity. + */ + if (qemuDomainMachineIsVirt(def)) + return true; return false; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 030d84b..10bcb1c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6831,9 +6831,7 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, if (def->features[VIR_DOMAIN_FEATURE_GIC] == VIR_TRISTATE_SWITCH_ON) { if (def->gic_version != VIR_GIC_VERSION_NONE) { - if ((def->os.arch != VIR_ARCH_ARMV7L && - def->os.arch != VIR_ARCH_AARCH64) || - !qemuDomainMachineIsVirt(def)) { + if (!qemuDomainMachineIsVirt(def)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("gic-version option is available " "only for ARM virt machine")); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1f99baa..3e906b3 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2040,7 +2040,6 @@ 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 && - (def->os.arch == VIR_ARCH_ARMV7L || def->os.arch == VIR_ARCH_AARCH64) && qemuDomainMachineIsVirt(def)) { VIR_DEBUG("Looking for usable GIC version in domain capabilities"); @@ -4919,8 +4918,15 @@ qemuDomainMachineIsS390CCW(const virDomainDef *def) bool qemuDomainMachineIsVirt(const virDomainDef *def) { - return STREQ(def->os.machine, "virt") || - STRPREFIX(def->os.machine, "virt-"); + if (def->os.arch != VIR_ARCH_ARMV7L && + def->os.arch != VIR_ARCH_AARCH64) + return false; + + if (STRNEQ(def->os.machine, "virt") && + !STRPREFIX(def->os.machine, "virt-")) + return false; + + return true; } -- 2.7.4
On 06/23/2016 04:40 AM, Andrea Bolognani wrote:
Remove all external architecture checks that have been made redundant by this change. --- src/qemu/qemu_capabilities.c | 12 +++++------- src/qemu/qemu_command.c | 4 +--- src/qemu/qemu_domain.c | 12 +++++++++--- 3 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 4ed5b71..5fcd744 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2174,13 +2174,11 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps, return false; }
- if (ARCH_IS_ARM(def->os.arch)) { - /* If 'virt' supports PCI, it supports multibus. - * No extra conditions here for simplicity. - */
Just to be pedantic - here you're removing a check for ARMV6L or ARMV7L or ARMV7B or AARCH64, and replacing it with a simpler check for ARMV7L or AARCH64. But I guess the virt machinetype isn't available/possible on those other two types? As long as that's okay, ACK.
- if (qemuDomainMachineIsVirt(def)) - return true; - } + /* If 'virt' supports PCI, it supports multibus. + * No extra conditions here for simplicity. + */ + if (qemuDomainMachineIsVirt(def)) + return true;
return false; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 030d84b..10bcb1c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6831,9 +6831,7 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
if (def->features[VIR_DOMAIN_FEATURE_GIC] == VIR_TRISTATE_SWITCH_ON) { if (def->gic_version != VIR_GIC_VERSION_NONE) { - if ((def->os.arch != VIR_ARCH_ARMV7L && - def->os.arch != VIR_ARCH_AARCH64) || - !qemuDomainMachineIsVirt(def)) { + if (!qemuDomainMachineIsVirt(def)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("gic-version option is available " "only for ARM virt machine")); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1f99baa..3e906b3 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2040,7 +2040,6 @@ 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 && - (def->os.arch == VIR_ARCH_ARMV7L || def->os.arch == VIR_ARCH_AARCH64) && qemuDomainMachineIsVirt(def)) {
VIR_DEBUG("Looking for usable GIC version in domain capabilities"); @@ -4919,8 +4918,15 @@ qemuDomainMachineIsS390CCW(const virDomainDef *def) bool qemuDomainMachineIsVirt(const virDomainDef *def) { - return STREQ(def->os.machine, "virt") || - STRPREFIX(def->os.machine, "virt-"); + if (def->os.arch != VIR_ARCH_ARMV7L && + def->os.arch != VIR_ARCH_AARCH64) + return false; + + if (STRNEQ(def->os.machine, "virt") && + !STRPREFIX(def->os.machine, "virt-")) + return false; + + return true; }
On Thu, 2016-06-23 at 14:08 -0400, Laine Stump wrote:
On 06/23/2016 04:40 AM, Andrea Bolognani wrote:
Remove all external architecture checks that have been made redundant by this change. --- src/qemu/qemu_capabilities.c | 12 +++++------- src/qemu/qemu_command.c | 4 +--- src/qemu/qemu_domain.c | 12 +++++++++--- 3 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 4ed5b71..5fcd744 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2174,13 +2174,11 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps, return false; }
- if (ARCH_IS_ARM(def->os.arch)) { - /* If 'virt' supports PCI, it supports multibus. - * No extra conditions here for simplicity. - */
Just to be pedantic - here you're removing a check for ARMV6L or ARMV7L or ARMV7B or AARCH64, and replacing it with a simpler check for ARMV7L or AARCH64. But I guess the virt machinetype isn't available/possible on those other two types?
As long as that's okay, ACK.
Yes, the virt machine type only supports ARMv7 and aarch64. ARMv7, aarch64 and ppc64 can actually be used in either LE or BE mode depending on the guest OS, so having separate PPC64LE/PPC64 and ARMV7L/ARMV7B guest architectures is a questionable design decision: the guest hardware is going to be exactly the same no matter what OS you install on it. Note how, funnily enough, we don't have AARCH64BE. Anyway, BE is basically unused on ARM: I haven't been able to find a single BE guest OS for the architecture. And all other architecture checks were already ignoring ARMV6L and ARMV7B, so it makes sense to be consistent to avoid nasty surprises down the road. -- Andrea Bolognani / Red Hat / Virtualization
This new function checks for both the architecture and the machine type, so we can use it instead of writing the same checks over and over again. --- src/qemu/qemu_command.c | 13 +++++-------- src/qemu/qemu_domain.c | 19 ++++++++++++++++--- src/qemu/qemu_domain.h | 1 + src/qemu/qemu_domain_address.c | 9 +++------ src/qemu/qemu_parse_command.c | 12 ++++-------- 5 files changed, 29 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 10bcb1c..e2201ff 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1946,9 +1946,8 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo; /* PowerPC pseries based VMs do not support floppy device */ - if ((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) && - ARCH_IS_PPC64(def->os.arch) && - STRPREFIX(def->os.machine, "pseries")) { + if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY && + qemuDomainMachineIsPSeries(def)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("PowerPC pseries machines do not support floppy device")); return -1; @@ -3701,8 +3700,7 @@ qemuBuildNVRAMCommandLine(virCommandPtr cmd, if (!def->nvram) return 0; - if (ARCH_IS_PPC64(def->os.arch) && - STRPREFIX(def->os.machine, "pseries")) { + if (qemuDomainMachineIsPSeries(def)) { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVRAM)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("nvram device is not supported by " @@ -8968,8 +8966,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 (!ARCH_IS_PPC64(def->os.arch) || - !STRPREFIX(def->os.machine, "pseries")) { + if (!qemuDomainMachineIsPSeries(def)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("only pSeries guests support panic device " "of model 'pseries'")); @@ -9390,7 +9387,7 @@ qemuBuildSerialChrDeviceStr(char **deviceStr, { virBuffer cmd = VIR_BUFFER_INITIALIZER; - if (ARCH_IS_PPC64(def->os.arch) && STRPREFIX(def->os.machine, "pseries")) { + if (qemuDomainMachineIsPSeries(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 3e906b3..6f70ca7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1878,7 +1878,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 (STRPREFIX(def->os.machine, "pseries")) + if (qemuDomainMachineIsPSeries(def)) addPanicDevice = true; break; @@ -2366,8 +2366,7 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, if (dev->type == VIR_DOMAIN_DEVICE_PANIC && dev->data.panic->model == VIR_DOMAIN_PANIC_MODEL_DEFAULT) { - if (ARCH_IS_PPC64(def->os.arch) && - STRPREFIX(def->os.machine, "pseries")) + if (qemuDomainMachineIsPSeries(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; @@ -4930,6 +4929,20 @@ qemuDomainMachineIsVirt(const virDomainDef *def) } +bool +qemuDomainMachineIsPSeries(const virDomainDef *def) +{ + if (!ARCH_IS_PPC64(def->os.arch)) + return false; + + if (STRNEQ(def->os.machine, "pseries") && + !STRPREFIX(def->os.machine, "pseries-")) + return false; + + return true; +} + + static bool qemuCheckMemoryDimmConflict(const virDomainDef *def, const virDomainMemoryDef *mem) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 2443e97..9b50b81 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -616,6 +616,7 @@ bool qemuDomainMachineIsI440FX(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); int qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver, diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index d9d71e7..ee44d45 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -88,8 +88,7 @@ qemuDomainSetSCSIControllerModel(const virDomainDef *def, return -1; } } else { - if (ARCH_IS_PPC64(def->os.arch) && - STRPREFIX(def->os.machine, "pseries")) { + if (qemuDomainMachineIsPSeries(def)) { *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI; } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI)) { *model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC; @@ -253,8 +252,7 @@ qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, for (i = 0; i < def->nserials; i++) { if (def->serials[i]->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && - ARCH_IS_PPC64(def->os.arch) && - STRPREFIX(def->os.machine, "pseries")) + qemuDomainMachineIsPSeries(def)) def->serials[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; if (qemuDomainAssignSpaprVIOAddress(def, &def->serials[i]->info, VIO_ADDR_SERIAL) < 0) @@ -262,8 +260,7 @@ qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def, } if (def->nvram) { - if (ARCH_IS_PPC64(def->os.arch) && - STRPREFIX(def->os.machine, "pseries")) + if (qemuDomainMachineIsPSeries(def)) def->nvram->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO; if (qemuDomainAssignSpaprVIOAddress(def, &def->nvram->info, VIO_ADDR_NVRAM) < 0) diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c index 1d54a68..9ee262b 100644 --- a/src/qemu/qemu_parse_command.c +++ b/src/qemu/qemu_parse_command.c @@ -654,8 +654,7 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, if (VIR_ALLOC(def->src) < 0) goto error; - if ((ARCH_IS_PPC64(dom->os.arch) && - dom->os.machine && STRPREFIX(dom->os.machine, "pseries"))) + if (qemuDomainMachineIsPSeries(dom)) def->bus = VIR_DOMAIN_DISK_BUS_SCSI; else def->bus = VIR_DOMAIN_DISK_BUS_IDE; @@ -747,8 +746,7 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, } else if (STREQ(keywords[i], "if")) { if (STREQ(values[i], "ide")) { def->bus = VIR_DOMAIN_DISK_BUS_IDE; - if ((ARCH_IS_PPC64(dom->os.arch) && - dom->os.machine && STRPREFIX(dom->os.machine, "pseries"))) { + if (qemuDomainMachineIsPSeries(dom)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("pseries systems do not support ide devices '%s'"), val); goto error; @@ -1939,8 +1937,7 @@ qemuParseCommandLine(virCapsPtr caps, } if (STREQ(arg, "-cdrom")) { disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM; - if ((ARCH_IS_PPC64(def->os.arch) && - def->os.machine && STRPREFIX(def->os.machine, "pseries"))) + if (qemuDomainMachineIsPSeries(def)) disk->bus = VIR_DOMAIN_DISK_BUS_SCSI; if (VIR_STRDUP(disk->dst, "hdc") < 0) goto error; @@ -1955,8 +1952,7 @@ qemuParseCommandLine(virCapsPtr caps, disk->bus = VIR_DOMAIN_DISK_BUS_IDE; else disk->bus = VIR_DOMAIN_DISK_BUS_SCSI; - if ((ARCH_IS_PPC64(def->os.arch) && - def->os.machine && STRPREFIX(def->os.machine, "pseries"))) + if (qemuDomainMachineIsPSeries(def)) disk->bus = VIR_DOMAIN_DISK_BUS_SCSI; } if (VIR_STRDUP(disk->dst, arg + 1) < 0) -- 2.7.4
On 06/23/2016 04:40 AM, Andrea Bolognani wrote:
This new function checks for both the architecture and the machine type, so we can use it instead of writing the same checks over and over again. --- src/qemu/qemu_command.c | 13 +++++-------- src/qemu/qemu_domain.c | 19 ++++++++++++++++--- src/qemu/qemu_domain.h | 1 + src/qemu/qemu_domain_address.c | 9 +++------ src/qemu/qemu_parse_command.c | 12 ++++-------- 5 files changed, 29 insertions(+), 25 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 10bcb1c..e2201ff 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c [...] @@ -4930,6 +4929,20 @@ qemuDomainMachineIsVirt(const virDomainDef *def) }
+bool +qemuDomainMachineIsPSeries(const virDomainDef *def) +{ + if (!ARCH_IS_PPC64(def->os.arch)) + return false; + + if (STRNEQ(def->os.machine, "pseries") && + !STRPREFIX(def->os.machine, "pseries-")) + return false;
...and you've also made sure that it will continue to work if the pseries machinetype is ever versioned (as all machinetypes should be). ACK.
Unfortunately, we can't just call qemuDomainMachineIsPSeries() here, because we don't have a virDomainDef instance; that said, the open-coded check should match said function as closely as possible. --- src/qemu/qemu_capabilities.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 5fcd744..01466fc 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4139,7 +4139,8 @@ virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr qemuCaps, VIR_DOMAIN_DISK_DEVICE_LUN); /* PowerPC pseries based VMs do not support floppy device */ - if (!(ARCH_IS_PPC64(qemuCaps->arch) && STRPREFIX(machine, "pseries"))) + if (!ARCH_IS_PPC64(qemuCaps->arch) || + (STRNEQ(machine, "pseries") && !STRPREFIX(machine, "pseries-"))) VIR_DOMAIN_CAPS_ENUM_SET(disk->diskDevice, VIR_DOMAIN_DISK_DEVICE_FLOPPY); VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, @@ -4149,7 +4150,8 @@ virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr qemuCaps, /* VIR_DOMAIN_DISK_BUS_SD */); /* PowerPC pseries based VMs do not support floppy device */ - if (!(ARCH_IS_PPC64(qemuCaps->arch) && STRPREFIX(machine, "pseries"))) + if (!ARCH_IS_PPC64(qemuCaps->arch) || + (STRNEQ(machine, "pseries") && !STRPREFIX(machine, "pseries-"))) VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, VIR_DOMAIN_DISK_BUS_FDC); if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE)) -- 2.7.4
On 06/23/2016 04:40 AM, Andrea Bolognani wrote:
Unfortunately, we can't just call qemuDomainMachineIsPSeries() here, because we don't have a virDomainDef instance; that said, the open-coded check should match said function as closely as possible.
Maybe you could make a separate function called something like "qemuMachineIsPSeries() that took the arch and machinetype args separately, then call that new function from qemuDomainMachineIsPSeries() and from the place you've patched below. That way there would be a single location for the logic, and no need to worry about whether or not it matched in the future. Either way is fine with me though, since I doubt this will change. ACK.
--- src/qemu/qemu_capabilities.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 5fcd744..01466fc 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4139,7 +4139,8 @@ virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr qemuCaps, VIR_DOMAIN_DISK_DEVICE_LUN);
/* PowerPC pseries based VMs do not support floppy device */ - if (!(ARCH_IS_PPC64(qemuCaps->arch) && STRPREFIX(machine, "pseries"))) + if (!ARCH_IS_PPC64(qemuCaps->arch) || + (STRNEQ(machine, "pseries") && !STRPREFIX(machine, "pseries-"))) VIR_DOMAIN_CAPS_ENUM_SET(disk->diskDevice, VIR_DOMAIN_DISK_DEVICE_FLOPPY);
VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, @@ -4149,7 +4150,8 @@ virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr qemuCaps, /* VIR_DOMAIN_DISK_BUS_SD */);
/* PowerPC pseries based VMs do not support floppy device */ - if (!(ARCH_IS_PPC64(qemuCaps->arch) && STRPREFIX(machine, "pseries"))) + if (!ARCH_IS_PPC64(qemuCaps->arch) || + (STRNEQ(machine, "pseries") && !STRPREFIX(machine, "pseries-"))) VIR_DOMAIN_CAPS_ENUM_SET(disk->bus, VIR_DOMAIN_DISK_BUS_FDC);
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE))
On Thu, 2016-06-23 at 14:17 -0400, Laine Stump wrote:
On 06/23/2016 04:40 AM, Andrea Bolognani wrote:
Unfortunately, we can't just call qemuDomainMachineIsPSeries() here, because we don't have a virDomainDef instance; that said, the open-coded check should match said function as closely as possible.
Maybe you could make a separate function called something like "qemuMachineIsPSeries() that took the arch and machinetype args separately, then call that new function from qemuDomainMachineIsPSeries() and from the place you've patched below. That way there would be a single location for the logic, and no need to worry about whether or not it matched in the future.
Either way is fine with me though, since I doubt this will change. ACK.
That could be a nice improvement. I'll look into it :) -- Andrea Bolognani / Red Hat / Virtualization
participants (2)
-
Andrea Bolognani -
Laine Stump