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 >
signature.asc
Description: Digital signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
