Re: [libvirt] [PATCH 1/2] Always free the device in AttachChrDevice

On 27.01.2015 14:45, Ján Tomko wrote: > Do not free it in the caller (qemuDomainAttachDeviceFlags), > because it has no way of knowing if it hasn't been cleaned > up by qemuProcessStop on monitor EOF. > > https://bugzilla.redhat.com/show_bug.cgi?id=1161024 > --- > src/qemu/qemu_driver.c | 3 +-- > src/qemu/qemu_hotplug.c | 14 ++++++++++---- > tests/qemuhotplugtest.c | 1 + > 3 files changed, 12 insertions(+), 6 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index ee5e4f5..333b2bf 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -6997,8 +6997,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, > case VIR_DOMAIN_DEVICE_CHR: > ret = qemuDomainAttachChrDevice(driver, vm, > dev->data.chr); > - if (!ret) > - dev->data.chr = NULL; > + dev->data.chr = NULL; I think it would be clearer if we make qemuDomainAttachChrDevice to add the device into vm->def only on success like the rest of attach function do.

There's nothing wrong with the code though. Michal