Re: [libvirt] [PATCH 2/2] tests: qemuhotplug: Don't free the monitor object as part of @vm

On Thu, Feb 02, 2017 at 04:46 PM +0100, Peter Krempa <pkrempa(a)redhat.com&gt; wrote: > The test monitor should be freed separately so we need to remove the > pointer from the @vm object. This fixes a race condition crash in the > test introduced in commit a245abce43. > --- > tests/qemuhotplugtest.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c > index 8cceb883e..8a58d5468 100644 > --- a/tests/qemuhotplugtest.c > +++ b/tests/qemuhotplugtest.c > @@ -365,6 +365,8 @@ struct testQemuHotplugCpuData { > static void > testQemuHotplugCpuDataFree(struct testQemuHotplugCpuData *data) > { > + qemuDomainObjPrivatePtr priv; > + > if (!data) > return; > > @@ -375,7 +377,13 @@ testQemuHotplugCpuDataFree(struct testQemuHotplugCpuData *data) > > VIR_FREE(data->xml_dom); > > - virObjectUnref(data->vm); > + if (data->vm) { > + priv = data->vm->privateData; > + priv->mon = NULL; > + > + virObjectUnref(data->vm); > + } > + > qemuMonitorTestFree(data->mon); > VIR_FREE(data); > } > -- > 2.11.0 > > -- > libvir-list mailing list > libvir-list(a)redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list > Just a question to this. Currently we get the reference to the monitor with 'priv->mon = qemuMonitorTestGetMonitor(data->mon);' (testQemuHotplugCpuPrepare in tests/qemuhotplugtest.c). Shouldn't we use 'priv->mon = virObjectRef(qemuMonitorTestGetMonitor(data->mon));' for getting the reference (and later on 'virObjectUnref(priv->mon); priv->mon = NULL;')? I know your fix works but I'm still thinking about that.

Anyway, Reviewed-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com&gt;