[libvirt] [PATCH] qemu: Check address when attaching a virtio disk with an invalid address.
https://bugzilla.redhat.com/show_bug.cgi?id=1257844 Attach-device can hotplug a virtio disk device with any address now. It need to validate the address before the attachment. This patch fix the problem. Signed-off-by: rbian <rbian@redhat.com> --- src/qemu/qemu_hotplug.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e71a204..be24993 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -331,6 +331,23 @@ 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 (STREQLEN(vm->def->os.machine, "s390-ccw", 8) && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { + if (!virDomainDeviceAddressIsValid(&disk->info, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("device cannot be attached without a valid CCW address")); + goto error; + } + } else { + if (!virDomainDeviceAddressIsValid(&disk->info, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("device cannot be attached without a valid PCI address")); + goto error; + } + } } for (i = 0; i < vm->def->ndisks; i++) { -- 2.4.3
On 09/02/2015 04:50 AM, rbian wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1257844
Attach-device can hotplug a virtio disk device with any address now. It need to validate the address before the attachment. This patch fix the problem.
Signed-off-by: rbian <rbian@redhat.com> --- src/qemu/qemu_hotplug.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
Wow - this is eerily familiar... See: http://www.redhat.com/archives/libvir-list/2015-August/msg01043.html Your approach is slightly different, but does only cover the hotplug case it seems... John
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e71a204..be24993 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -331,6 +331,23 @@ 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 (STREQLEN(vm->def->os.machine, "s390-ccw", 8) && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { + if (!virDomainDeviceAddressIsValid(&disk->info, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("device cannot be attached without a valid CCW address")); + goto error; + } + } else { + if (!virDomainDeviceAddressIsValid(&disk->info, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("device cannot be attached without a valid PCI address")); + goto error; + } + } }
for (i = 0; i < vm->def->ndisks; i++) {
On Wed, Sep 02, 2015 at 04:50:50PM +0800, rbian wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1257844
Attach-device can hotplug a virtio disk device with any address now. It need to validate the address before the attachment. This patch fix the problem.
Signed-off-by: rbian <rbian@redhat.com>
We prefer using full names for authors, that is, at least two words with capital letters separated by a space.
--- src/qemu/qemu_hotplug.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e71a204..be24993 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -331,6 +331,23 @@ 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 (STREQLEN(vm->def->os.machine, "s390-ccw", 8) && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { + if (!virDomainDeviceAddressIsValid(&disk->info, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("device cannot be attached without a valid CCW address")); + goto error; + }
This part is covered by the patch posted by John.
+ } else { + if (!virDomainDeviceAddressIsValid(&disk->info, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("device cannot be attached without a valid PCI address")); + goto error; + } + }
It looks like this would break hotplugging devices with S390 address types. We should only check it when the type is VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI. Are there other devices where we don't check the validity of the user-supplied address? Jan
----- Original Message -----
From: "Ján Tomko" <jtomko@redhat.com> To: "rbian" <rbian@redhat.com> Cc: libvir-list@redhat.com Sent: Wednesday, September 2, 2015 9:54:49 PM Subject: Re: [libvirt] [PATCH] qemu: Check address when attaching a virtio disk with an invalid address.
On Wed, Sep 02, 2015 at 04:50:50PM +0800, rbian wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1257844
Attach-device can hotplug a virtio disk device with any address now. It need to validate the address before the attachment. This patch fix the problem.
Signed-off-by: rbian <rbian@redhat.com>
We prefer using full names for authors, that is, at least two words with capital letters separated by a space.
--- src/qemu/qemu_hotplug.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e71a204..be24993 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -331,6 +331,23 @@ 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 (STREQLEN(vm->def->os.machine, "s390-ccw", 8) && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) { + if (!virDomainDeviceAddressIsValid(&disk->info, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("device cannot be attached without a valid CCW address")); + goto error; + }
This part is covered by the patch posted by John.
+ } else { + if (!virDomainDeviceAddressIsValid(&disk->info, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("device cannot be attached without a valid PCI address")); + goto error; + } + }
It looks like this would break hotplugging devices with S390 address types.
Yes, I will submit another version.
We should only check it when the type is VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI. Are there other devices where we don't check the validity of the user-supplied address?
Jan
Attaching scsi disk with --config option also has problem, I will try to fix it in next version. Ruifeng
participants (4)
-
John Ferlan -
Ján Tomko -
rbian -
Ruifeng Bian