Re: [libvirt] [PATCH 2/2] qemu_driver: remove duplicate code

On 01/07/15 12:05, Pavel Hrdina wrote: > The copy of persistent definition is already done in > virDomainLiveConfigHelperMethod few lines above. > > Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; > --- > src/qemu/qemu_driver.c | 6 ------ > 1 file changed, 6 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 4cfae03..ca93a1a 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -10336,12 +10336,6 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom, > &vmdef) < 0) > goto endjob; > > - if (flags & VIR_DOMAIN_AFFECT_CONFIG) { > - /* Make a copy for updated domain. */ > - if (!(vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt))) > - goto endjob; > - } > - > if (flags & VIR_DOMAIN_AFFECT_LIVE) { > if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) { > virReportError(VIR_ERR_OPERATION_INVALID, > NACK. It's not true, that a copy is made in virDomainLiveConfigHelperMethod, only a reference to domain definition is returned. The problem is that we free vmdef at the end of the API which might not result in a desired behavior, let's consider an inactive persistent domain and you try to set one of scheduler params, instead of modifying an freeing a copy, you manipulate and free the original instance, what happens then? Yep, daemon crashes in the next API (OK, not every time, it requires a bit of luck but after a couple of minutes I managed to do that as well).

Erik -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list