1:51 p.m.
Assumptions were made that if someone provided an address type ccw or
s390 that it would occur only if using an enabled emulator. Turns out
that premise isn't necessarily true and it leads to libvirtd crashing
for hotplugs and qemu start errors for config paths.
These patches will make the checks prior to crashes or qemu process
starts in order to avoid the situation.
v1:
http://www.redhat.com/archives/libvir-list/2015-August/msg01043.html
Changes since v1...
... Implement a function to handle the s390-ccw check using STRPREFIX
instead of a mix of STRPREFIX and STREQLEN
... Create qemuCheckCCWS390AddressSupport to handle checking address
type if defined on entry to disk, controller, and rng device additions
whether through hotplug or config options.
NB: It doesn't seem network devices are afflicted, although perhaps
I read the code wrong. It seems for a network device there is/was
none of the set the default address if undefined code added.
John Ferlan (2):
qemu: Introduce qemuDomainMachineIsS390CCW
qemu: Need to check for machine.os when using ADDRESS_TYPE_CCW
src/qemu/qemu_command.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++--
src/qemu/qemu_command.h | 5 +++++
src/qemu/qemu_domain.c | 6 ++++++
src/qemu/qemu_domain.h | 1 +
src/qemu/qemu_hotplug.c | 24 +++++++++++++++------
5 files changed, 83 insertions(+), 8 deletions(-)
--
2.1.0
1:51 p.m.
New subject: [libvirt] [PATCH v2 1/2] qemu: Introduce qemuDomainMachineIsS390CCW
Rather than have different usages of STR function in order to determine
whether the domain is s390-ccw or s390-ccw-virtio, make a single API
which will check the machine.os prefix. Then use the function.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 4 ++--
src/qemu/qemu_domain.c | 6 ++++++
src/qemu/qemu_domain.h | 1 +
src/qemu/qemu_hotplug.c | 12 ++++++------
4 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2a820af..9528b4a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1342,7 +1342,7 @@ qemuDomainAssignS390Addresses(virDomainDefPtr def,
virDomainCCWAddressSetPtr addrs = NULL;
qemuDomainObjPrivatePtr priv = NULL;
- if (STREQLEN(def->os.machine, "s390-ccw", 8) &&
+ if (qemuDomainMachineIsS390CCW(def) &&
virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_CCW)) {
qemuDomainPrimeVirtioDeviceAddresses(
def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW);
@@ -1864,7 +1864,7 @@ qemuDomainReleaseDeviceAddress(virDomainObjPtr vm,
devstr = info->alias;
if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW &&
- STREQLEN(vm->def->os.machine, "s390-ccw", 8) &&
+ qemuDomainMachineIsS390CCW(vm->def) &&
virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW) &&
virDomainCCWAddressReleaseAddr(priv->ccwaddrs, info) < 0)
VIR_WARN("Unable to release CCW address on %s",
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0a9ed6b..91c632c 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3323,6 +3323,12 @@ qemuDomainMachineNeedsFDC(const virDomainDef *def)
}
+bool
+qemuDomainMachineIsS390CCW(const virDomainDef *def)
+{
+ return STRPREFIX(def->os.machine, "s390-ccw");
+}
+
/**
* qemuDomainUpdateCurrentMemorySize:
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 2af7c59..91eaea1 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -469,6 +469,7 @@ virDomainChrSourceDefPtr qemuFindAgentConfig(virDomainDefPtr def);
bool qemuDomainMachineIsQ35(const virDomainDef *def);
bool qemuDomainMachineIsI440FX(const virDomainDef *def);
bool qemuDomainMachineNeedsFDC(const virDomainDef *def);
+bool qemuDomainMachineIsS390CCW(const virDomainDef *def);
int qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver,
virDomainObjPtr vm);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index ac683ad..f46ec5e 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -326,7 +326,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
const char *src = virDomainDiskGetSource(disk);
if (!disk->info.type) {
- if (STREQLEN(vm->def->os.machine, "s390-ccw", 8) &&
+ if (qemuDomainMachineIsS390CCW(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))
@@ -443,7 +443,7 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver,
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
- if (STRPREFIX(vm->def->os.machine, "s390-ccw") &&
+ if (qemuDomainMachineIsS390CCW(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))
@@ -991,7 +991,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
goto cleanup;
}
- if (STREQLEN(vm->def->os.machine, "s390-ccw", 8) &&
+ if (qemuDomainMachineIsS390CCW(vm->def) &&
virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) {
net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW;
if (virDomainCCWAddressAssign(&net->info, priv->ccwaddrs,
@@ -1657,7 +1657,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
return -1;
if (rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
- if (STRPREFIX(vm->def->os.machine, "s390-ccw") &&
+ if (qemuDomainMachineIsS390CCW(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)) {
@@ -3412,7 +3412,7 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver,
goto cleanup;
}
- if (STREQLEN(vm->def->os.machine, "s390-ccw", 8) &&
+ if (qemuDomainMachineIsS390CCW(vm->def) &&
virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) {
if (!virDomainDeviceAddressIsValid(&detach->info,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) {
@@ -3942,7 +3942,7 @@ qemuDomainDetachNetDevice(virQEMUDriverPtr driver,
virDomainNetGetActualHostdev(detach));
goto cleanup;
}
- if (STREQLEN(vm->def->os.machine, "s390-ccw", 8) &&
+ if (qemuDomainMachineIsS390CCW(vm->def) &&
virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) {
if (!virDomainDeviceAddressIsValid(&detach->info,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) {
--
2.1.0
1:51 p.m.
New subject: [libvirt] [PATCH v2 2/2] qemu: Need to check for machine.os when using ADDRESS_TYPE_CCW
https://bugzilla.redhat.com/show_bug.cgi?id=1258361
When attaching a disk, controller, or rng using an address type ccw
or s390, we need to ensure the support is provided by both the machine.os
and the emulator capabilities (corollary to unconditional setting when
address was not provided for the correct machine.os and emulator.
For an inactive guest, an addition followed by a start would cause the
startup to fail after qemu_command builds the command line and attempts
to start the guest. For an active guest, libvirtd would crash.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_command.h | 5 +++++
src/qemu/qemu_hotplug.c | 12 ++++++++++++
3 files changed, 68 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9528b4a..ec5e3d4 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3585,6 +3585,46 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk)
}
+/* Check whether the device address is using either 'ccw' or default s390
+ * address format and whether that's "legal" for the current qemu and/or
+ * guest os.machine type. This is the corollary to the code which doesn't
+ * find the address type set using an emulator that supports either 'ccw'
+ * or s390 and sets the address type based on the capabilities.
+ *
+ * If the address is using 'ccw' or s390 and it's not supported, generate
+ * an error and return false; otherwise, return true.
+ */
+bool
+qemuCheckCCWS390AddressSupport(virDomainDefPtr def,
+ virDomainDeviceInfo info,
+ virQEMUCapsPtr qemuCaps,
+ const char *devicename)
+{
+ if (info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
+ if (!qemuDomainMachineIsS390CCW(def)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("cannot use CCW address type for device "
+ "'%s' using machine type '%s'"),
+ devicename, def->os.machine);
+ return false;
+ } else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_CCW)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("CCW address type is not supported by "
+ "this QEMU"));
+ return false;
+ }
+ } else if (info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) {
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_S390)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("virtio S390 address type is not supported by "
+ "this QEMU"));
+ return false;
+ }
+ }
+ return true;
+}
+
+
/* Qemu 1.2 and later have a binary flag -enable-fips that must be
* used for VNC auth to obey FIPS settings; but the flag only
* exists on Linux, and with no way to probe for it via QMP. Our
@@ -4138,6 +4178,9 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
}
}
+ if (!qemuCheckCCWS390AddressSupport(def, disk->info, qemuCaps, disk->dst))
+ goto error;
+
if (disk->iothread && !qemuCheckIOThreads(def, qemuCaps, disk))
goto error;
@@ -4592,6 +4635,10 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
int model = def->model;
const char *modelName = NULL;
+ if (!qemuCheckCCWS390AddressSupport(domainDef, def->info, qemuCaps,
+ "controller"))
+ return NULL;
+
if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
if ((qemuSetSCSIControllerModel(domainDef, qemuCaps, &model)) < 0)
return NULL;
@@ -6884,6 +6931,10 @@ qemuBuildRNGDevStr(virDomainDefPtr def,
goto error;
}
+ if (!qemuCheckCCWS390AddressSupport(def, dev->info, qemuCaps,
+ dev->source.file))
+ goto error;
+
if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)
virBufferAsprintf(&buf, "virtio-rng-ccw,rng=obj%s,id=%s",
dev->info.alias, dev->info.alias);
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index e356f5b..767d31f 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -318,4 +318,9 @@ int qemuCheckDiskConfig(virDomainDiskDefPtr disk);
bool
qemuCheckFips(void);
+
+bool qemuCheckCCWS390AddressSupport(virDomainDefPtr def,
+ virDomainDeviceInfo info,
+ virQEMUCapsPtr qemuCaps,
+ const char *devicename);
#endif /* __QEMU_COMMAND_H__*/
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index f46ec5e..63fafa6 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -331,6 +331,10 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW;
else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390))
disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390;
+ } else {
+ if (!qemuCheckCCWS390AddressSupport(vm->def, disk->info,
priv->qemuCaps,
+ disk->dst))
+ goto cleanup;
}
for (i = 0; i < vm->def->ndisks; i++) {
@@ -448,6 +452,10 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver,
controller->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW;
else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390))
controller->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390;
+ } else {
+ if (!qemuCheckCCWS390AddressSupport(vm->def, controller->info,
+ priv->qemuCaps,
"controller"))
+ goto cleanup;
}
if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
@@ -1663,6 +1671,10 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
} else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) {
rng->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390;
}
+ } else {
+ if (!qemuCheckCCWS390AddressSupport(vm->def, rng->info, priv->qemuCaps,
+ rng->source.file))
+ return -1;
}
if (rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
--
2.1.0
6:30 a.m.
On Thu, Sep 03, 2015 at 02:51:54PM -0400, John Ferlan wrote:
Assumptions were made that if someone provided an address type ccw
or
s390 that it would occur only if using an enabled emulator. Turns out
that premise isn't necessarily true and it leads to libvirtd crashing
for hotplugs and qemu start errors for config paths.
These patches will make the checks prior to crashes or qemu process
starts in order to avoid the situation.
v1:
http://www.redhat.com/archives/libvir-list/2015-August/msg01043.html
Changes since v1...
... Implement a function to handle the s390-ccw check using STRPREFIX
instead of a mix of STRPREFIX and STREQLEN
... Create qemuCheckCCWS390AddressSupport to handle checking address
type if defined on entry to disk, controller, and rng device additions
whether through hotplug or config options.
NB: It doesn't seem network devices are afflicted, although perhaps
I read the code wrong. It seems for a network device there is/was
none of the set the default address if undefined code added.
John Ferlan (2):
qemu: Introduce qemuDomainMachineIsS390CCW
qemu: Need to check for machine.os when using ADDRESS_TYPE_CCW
src/qemu/qemu_command.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++--
src/qemu/qemu_command.h | 5 +++++
src/qemu/qemu_domain.c | 6 ++++++
src/qemu/qemu_domain.h | 1 +
src/qemu/qemu_hotplug.c | 24 +++++++++++++++------
5 files changed, 83 insertions(+), 8 deletions(-)
ACK series.
Jan
3401
days inactive
3402
days old
3 comments
2 participants
participants (2)
-
John Ferlan -
Ján Tomko