[libvirt] [PATCH] 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 or rng when providing an address type ccw, we need to ensure the machine.os is valid for the target guest. Checks are made when the address type is not provided, but if provided the os.machine check is bypassed. 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@redhat.com> --- NOTE: For 'rng' in qemu_command - rather than change each of the if..else if.. conditions to have the {}, I just made the check up front. Also while I could separate these into disk and rng - since it's the same fix for the same problem it just seem "proper" to include both in same patch src/qemu/qemu_command.c | 15 +++++++++++++++ src/qemu/qemu_hotplug.c | 21 ++++++++++++++++++--- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index abc57d7..ecdec2c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4296,6 +4296,13 @@ qemuBuildDriveDevStr(virDomainDefPtr def, case VIR_DOMAIN_DISK_BUS_VIRTIO: if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { + if (!STREQLEN(def->os.machine, "s390-ccw", 8)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot use CCW address type for disk target " + "'%s' using machine type '%s'"), + disk->dst, def->os.machine); + goto error; + } virBufferAddLit(&opt, "virtio-blk-ccw"); if (disk->iothread) virBufferAsprintf(&opt, ",iothread=iothread%u", disk->iothread); @@ -6874,6 +6881,14 @@ qemuBuildRNGDevStr(virDomainDefPtr def, goto error; } + if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && + !STREQLEN(def->os.machine, "s390-ccw", 8)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot use CCW address type for rng device " + "using machine type '%s'"), + def->os.machine); + 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_hotplug.c b/src/qemu/qemu_hotplug.c index e71a204..154539c 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -319,6 +319,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, int ret = -1; const char* type = virDomainDiskBusTypeToString(disk->bus); qemuDomainObjPrivatePtr priv = vm->privateData; + bool is_s390_ccw = STREQLEN(vm->def->os.machine, "s390-ccw", 8); char *devstr = NULL; char *drivestr = NULL; bool releaseaddr = false; @@ -326,8 +327,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, const char *src = virDomainDiskGetSource(disk); if (!disk->info.type) { - if (STREQLEN(vm->def->os.machine, "s390-ccw", 8) && - virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) + if (is_s390_ccw && 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)) disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390; @@ -346,6 +346,13 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { + if (!is_s390_ccw) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot use CCW address type for disk target " + "'%s' using machine type '%s'"), + disk->dst, vm->def->os.machine); + return -1; + } if (virDomainCCWAddressAssign(&disk->info, priv->ccwaddrs, !disk->info.addr.ccw.assigned) < 0) goto error; @@ -1642,6 +1649,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver, virDomainRNGDefPtr rng) { qemuDomainObjPrivatePtr priv = vm->privateData; + bool is_s390_ccw = STRPREFIX(vm->def->os.machine, "s390-ccw"); char *devstr = NULL; char *charAlias = NULL; char *objAlias = NULL; @@ -1657,7 +1665,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 (is_s390_ccw && 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)) { @@ -1670,6 +1678,13 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver, if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &rng->info) < 0) return -1; } else if (rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { + if (!is_s390_ccw) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot use CCW address type for rng device " + "using machine type '%s'"), + vm->def->os.machine); + return -1; + } if (virDomainCCWAddressAssign(&rng->info, priv->ccwaddrs, !rng->info.addr.ccw.assigned) < 0) return -1; -- 2.1.0

On Mon, Aug 31, 2015 at 01:00:25PM -0400, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1258361
When attaching a disk or rng when providing an address type ccw, we need to ensure the machine.os is valid for the target guest. Checks are made when the address type is not provided, but if provided the os.machine check is bypassed.
Incomplete, attaching a scsi controller with a ccw address on an x86_64 still crashes the daemon after this patch.
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@redhat.com> ---
NOTE: For 'rng' in qemu_command - rather than change each of the if..else if.. conditions to have the {}, I just made the check up front.
Also while I could separate these into disk and rng - since it's the same fix for the same problem it just seem "proper" to include both in same patch
src/qemu/qemu_command.c | 15 +++++++++++++++ src/qemu/qemu_hotplug.c | 21 ++++++++++++++++++--- 2 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index abc57d7..ecdec2c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4296,6 +4296,13 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
case VIR_DOMAIN_DISK_BUS_VIRTIO: if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { + if (!STREQLEN(def->os.machine, "s390-ccw", 8)) {
Maybe we need a 'qemuDomainMachineIsS390CCW' helper? Jan

On Wed, 2015-09-02 at 15:47 +0200, Ján Tomko wrote:
case VIR_DOMAIN_DISK_BUS_VIRTIO: if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { + if (!STREQLEN(def->os.machine, "s390-ccw", 8)) {
Maybe we need a 'qemuDomainMachineIsS390CCW' helper?
I'd also use STRPREFIX() instead of STREQLEN(). Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team
participants (3)
-
Andrea Bolognani
-
John Ferlan
-
Ján Tomko