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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list