[libvirt] [PATCH] qemu: Check address when attaching a virtio disk with an invalid address.

[Ruifeng Bian]

----- Original Message -----
> From: "Ján Tomko" <jtomko at redhat.com>
> To: "rbian" <rbian at redhat.com>
> Cc: libvir-list at 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 at 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