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

[Michal Privoznik]

On 18.10.2013 06:06, Wangyufei (A) wrote:
> Thanks at first, this patch some kinda solve my problem until now. But I still have a doubt about this patch.
> 
>> -----Original Message-----
>> From: libvir-list-bounces at redhat.com
>> [mailto:libvir-list-bounces at redhat.com] On Behalf Of Michal Privoznik
>> Sent: Friday, October 11, 2013 8:15 PM
>> To: libvir-list at redhat.com
>> Subject: [libvirt] [PATCH v2] qemu_migration: Avoid crashing if domain dies
>> too quickly

>> @@ -2673,6 +2677,8 @@ cleanup:
>>      virCommandFree(cmd);
>>      VIR_FREE(monarg);
>>      VIR_FREE(monpath);
>> +    virObjectUnref(vm);
> 
> Is this virObjectUnref(vm) corresponding to mon->vm = virObjectRef(vm) added in qemuMonitorOpenInternal?
> If it is, how can we confirm virObjectRef(vm) has been done in qemuMonitorOpenInternal? If an error (anyone follows)happened in qemuMonitorOpenInternal is before mon->vm = virObjectRef(vm),
> then we still goto cleanup and do virObjectUnref(vm), the refs will be wrong. Am I right?
> 

Unfortunately, you've cut off the chunk above that allocates @mon.
Anyway, on initialization, @mon is filled with zeros. So until somebody
sets mon->vm [1] mon->vm is effectively NULL. And virObjectUnref() acts
like NOP on NULL.

>     if (!cb->eofNotify) {
>         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                        _("EOF notify callback must be supplied"));
>         return NULL;
>     }
>     if (!cb->errorNotify) {
>         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                        _("Error notify callback must be supplied"));
>         return NULL;
>     }
> 
>     if (qemuMonitorInitialize() < 0)
>         return NULL;
> 
>     if (!(mon = virObjectLockableNew(qemuMonitorClass)))
>         return NULL;
> 
>     mon->fd = -1;
>     mon->logfd = -1;
>     if (virCondInit(&mon->notify) < 0) {
>         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                        _("cannot initialize monitor condition"));
>         goto cleanup;
>     }
>     mon->fd = fd;
>     mon->hasSendFD = hasSendFD;
>     mon->vm = virObjectRef(vm);

1: ^^ until after this line
> 
>> +    virObjectUnref(xmlopt);
>>
>>      if (pid != 0) {
>>          char ebuf[1024];

I hope it makes things a bit clearer.

Michal