On Mon, Apr 13, 2015 at 02:03:57PM -0400, John Ferlan wrote:
On 04/04/2015 01:16 PM, Ján Tomko wrote:
Only for devices that have an alias. --- src/qemu/qemu_driver.c | 17 ++++++++++++++++- src/qemu/qemu_hotplug.c | 5 +++++ 2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6132674..c13f22b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7586,17 +7586,20 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, { virQEMUDriverPtr driver = dom->conn->privateData; int ret = -1; + const char *alias = NULL;
switch ((virDomainDeviceType) dev->type) { case VIR_DOMAIN_DEVICE_DISK: qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, -1); ret = qemuDomainAttachDeviceDiskLive(dom->conn, driver, vm, dev); + alias = dev->data.disk->info.alias; if (!ret) dev->data.disk = NULL; break;
case VIR_DOMAIN_DEVICE_CONTROLLER: ret = qemuDomainAttachDeviceControllerLive(driver, vm, dev); + alias = dev->data.controller->info.alias;
The one concern I'd have for all of these is - if (ret != 0) - is there any path that free's anything along the way that you're using a pointer in the alias fetching?
All the functions except AttachMemory should only add the device to the domain definition right after setting ret to 0, so if ret == -1, it should still be owned by the caller.
Additionally of course, since the only way to print the alias is if (ret == 0) later, one could point out that setting it when ret != 0 is pointless; however, at least if ret == 0, you should be able to assume no one as deleted the alias!
Actually, this is wrong. When ret == 0, the device is already a part of the domain definition. And the domain object is unlocked when we enter the monitor in qemuDomainUpdateDeviceList. So the Event should be queued before the call to UpdateDeviceList, or a local copy of the alias is needed. Either way, another version of this patch is needed.
Perhaps it's best to only get the alias if (!ret)
Your call if you want to add a "note" for case VIR_DOMAIN_DEVICE_MEMORY that the event is elicited inside the call since the call consumes dev->data.memory and hence the alias.
Good idea. Jan
I think with the alias setting inside !ret I'd feel comfortable giving an ACK - although I suspect in the other case it's not deleted until after this call exits
John