Re: [libvirt] [PATCH v2] qemu_migration: Avoid crashing if domain dies too quickly

18 Oct 2013


      On 18.10.2013 09:22, Wangyufei (A) wrote:
...
...
-----Original Message-----
From: Michal Privoznik [mailto:mprivozn@redhat.com]
Sent: Friday, October 18, 2013 2:37 PM
To: Wangyufei (A)
Cc: libvir-list@redhat.com; Wangrui (K)
Subject: Re: [libvirt] [PATCH v2] qemu_migration: Avoid crashing if domain
dies too quickly
On 18.10.2013 08:22, Wangyufei (A) wrote:
...
I'm sorry. I didn't get what you mean.
In virQEMUCapsInitQMP
if (!(xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL)) ||
        !(vm = virDomainObjNew(xmlopt)))
        goto cleanup;
vm->pid = pid;   //Apparently vm is not NULL here.
if (!(mon = qemuMonitorOpen(vm, &config, true, &callbacks, NULL)))
{  //If qemuMonitorOpen returns NULL here, but not do mon->vm =
virObjectRef(vm); in qemuMonitorOpenInternal.
        ret = 0;
        goto cleanup;  // We go to cleanup here.
    }
virObjectLock(mon);
if (virQEMUCapsInitQMPMonitor(qemuCaps, mon) < 0)
        goto cleanup;
ret = 0;
cleanup:
    if (mon)
        virObjectUnlock(mon);
    qemuMonitorClose(mon);
    virCommandAbort(cmd);
    virCommandFree(cmd);
    VIR_FREE(monarg);
    VIR_FREE(monpath);
    virObjectUnref(vm);    //vm is not NULL here, and we'll do
something about vm->refs, right?
Yes. In fact we dispose @vm as we are the only one holding reference to
it and we don't longer need it. If qemuMonitorOpenInternal would do
virObjectRef(vm), then vm->refs = 2 before executing this line. After
If qemuMonitorOpenInternal did not do virObjectRef(vm) and return NULL before it, then vm->refs = 1 before executing this line. Right?
Now we do virObjectUnref(vm), vm will be disposed here, and that's we expected. Fine, I've got you. Thanks a lot.
Exactly. We don't want to leave @vm allocated if there was an error.
However, we do want to leave @vm allocated if monitor is successfully
opened. Then, the qemuMonitorDispose will be the one to dispose the @vm.

Michal