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