3:40 a.m.
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
3:40 a.m.
New subject: [libvirt] [PATCH 1/4] qemu: Remove redundant arguments to qemuBuildSerialChrDeviceStr()
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
1:03 p.m.
New subject: [libvirt] [PATCH 1/4] qemu: Remove redundant arguments to qemuBuildSerialChrDeviceStr()
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.
3:40 a.m.
New subject: [libvirt] [PATCH 2/4] qemu: Add architecture checks to qemuDomainMachineIsVirt()
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
1:08 p.m.
New subject: [libvirt] [PATCH 2/4] qemu: Add architecture checks to qemuDomainMachineIsVirt()
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;
}
4:29 a.m.
New subject: [libvirt] [PATCH 2/4] qemu: Add architecture checks to qemuDomainMachineIsVirt()
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
3:40 a.m.
New subject: [libvirt] [PATCH 3/4] qemu: Introduce qemuDomainMachineIsPSeries()
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
1:13 p.m.
New subject: [libvirt] [PATCH 3/4] qemu: Introduce qemuDomainMachineIsPSeries()
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.
3:40 a.m.
New subject: [libvirt] [PATCH 4/4] qemu: Use stricter checks in virQEMUCapsFillDomainDeviceDiskCaps()
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
1:17 p.m.
New subject: [libvirt] [PATCH 4/4] qemu: Use stricter checks in virQEMUCapsFillDomainDeviceDiskCaps()
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))
4:36 a.m.
New subject: [libvirt] [PATCH 4/4] qemu: Use stricter checks in virQEMUCapsFillDomainDeviceDiskCaps()
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
3073
days inactive
3074
days old
10 comments
2 participants
participants (2)
-
Andrea Bolognani -
Laine Stump