[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 4/4] qemu: Call virDomainDefPostParse via CONFIG hotplug



On Tue, 2016-05-17 at 14:24 -0400, Cole Robinson wrote:
> > 
> > > +                             virDomainXMLOptionPtr xmlopt)
> > >   {
> > >       virDomainDiskDefPtr disk;
> > >       virDomainNetDefPtr net;
> > > @@ -7803,11 +7805,6 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
> > >               return -1;
> > >           /* vmdef has the pointer. Generic codes for vmdef will do all jobs */
> > >           dev->data.disk = NULL;
> > > -        if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO)
> > > -            if (virDomainDefAddImplicitDevices(vmdef) < 0)
> > > -                return -1;
> > 
> > You removed the check on disk->bus here, and that concerns me a
> > little. Can you please spend a few words explaining why this is
> > safe?
> 
> I think the idea behind that check was 'adding a virtio disk doesn't need any
> implied controller, but bus=scsi might, so only call AddImplicit for non-virtio'
> 
> However AddImplicit _must_ do the right thing here anyways, since for example
> it is called every time we parse the XML, like reading it from disk on
> libvirtd startup. So the check here was overly paranoid (but maybe it made
> sense once)

Makes sense, thanks for the explanation.

ACK with the argument name changed.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]