Re: [libvirt] [PATCH 1/3] conf: Reposition adding SCSI controller for SCSI hostdev hotplug

On 11/30/2015 06:05 AM, Boris Fiuczynski wrote: [...] After some more experimentation and making sure my debug environment was "up to date" (e.g. had virtlogd started)... Using top of tree I can hotplug a hostdev as long as it has a drive address defined. Not sure why it was failing before, but at least things are making more sense now. The reason why that path works with top of tree is because virDomainDeviceDefPostParseInternal won't call virDomainHostdevAssignAddress since hdev->info->type is not VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE. That means when the qemuDomainFindOrCreateSCSIDiskController is called it will do the magic add. The path for this patch handles the inactive domain definition. While patch 2 handles the backend of the active domain path. When you have a running domain, then qemuDomainAttachDeviceFlags (e.g. what virsh attach-device would use) calls virDomainDeviceDefParse which is where the controller is added during virDomainDeviceDefPostParse. So patch 2 where you're removing that PostParse callback is why the hotplug will then work because no longer does post parse add the controller to the domain def. The call to qemuDomainFindOrCreateSCSIDiskController called by qemuDomainAttachSCSIDisk during qemuDomainAttachDeviceDiskLive in the live path of qemuDomainAttachDeviceFlags would then automagically add the controller. Since the active path wouldn't have added it during post parse. While these patches would work for the active domain hostdev add, I think perhaps a different mechanism should be used. Given the failure from trying to add a disk in the same environment, I'm not sure either patch solves the root problem that during hotplug we're relying on *not* adding a controller device when a new one is deemed necessary (for both hostdev and disk). One thing that is of interest is that virDomainDefMaybeAddController has 3 return values -1 fail, 0 didn't add a controller, and 1 added a controller. I'm wondering if that can somehow be used to ensure that if we add a controller that we ensure it really gets plugged in. Instead of relying on qemuDomainFindOrCreateSCSIDiskController to backdoor it in. John