Re: [libvirt] [PATCH v2] qemu_migration: Avoid crashing if domain dies too quickly
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(a)redhat.com
> [mailto:libvir-list-bounces@redhat.com] On Behalf Of Michal Privoznik
> Sent: Friday, October 11, 2013 8:15 PM
> To: libvir-list(a)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