Re: [libvirt] [RFC PATCH] check whether domain is active after qemuDomainObjExitMonitor* returns

qemu may quited unexpectedly when invoking a monitor command. And priv->mon will be NULL after qemuDomainObjExitMonitor* returns. So we must not use it. Unfortunately we still use it, and it will cause libvirtd crashed. As Eric suggested, qemuDomainObjExitMonitor* should be made to return the value of vm active after regaining lock, and marked ATTRIBUTE_RETURN_CHECK, to force all other callers to detect the case of a monitor command completing successfully but then the VM disappearing in the window between command completion and regaining the vm lock.

@@ -578,7 +578,7 @@ void qemuDomainObjEnterMonitor(virDomainObjPtr obj) * * Should be paired with an earlier qemuDomainObjEnterMonitor() call */ -void qemuDomainObjExitMonitor(virDomainObjPtr obj) +int qemuDomainObjExitMonitor(virDomainObjPtr obj) { qemuDomainObjPrivatePtr priv = obj->privateData; int refs; @@ -588,11 +588,20 @@ void qemuDomainObjExitMonitor(virDomainObjPtr obj) if (refs > 0) qemuMonitorUnlock(priv->mon); + /* Note: qemu may quited unexpectedly here, and the monitor will be freed. + * If it happened, priv->mon will be null. + */ + virDomainObjLock(obj); if (refs == 0) { priv->mon = NULL; } + + if (priv->mon == NULL) + return -1;

+ else + return 0; } @@ -621,8 +630,8 @@ void qemuDomainObjEnterMonitorWithDriver(struct qemud_driver *driver, * * Should be paired with an earlier qemuDomainObjEnterMonitorWithDriver() call */ -void qemuDomainObjExitMonitorWithDriver(struct qemud_driver *driver, - virDomainObjPtr obj) +int qemuDomainObjExitMonitorWithDriver(struct qemud_driver *driver, + virDomainObjPtr obj) { qemuDomainObjPrivatePtr priv = obj->privateData; int refs; @@ -632,12 +641,21 @@ void qemuDomainObjExitMonitorWithDriver(struct qemud_driver *driver, if (refs > 0) qemuMonitorUnlock(priv->mon); + /* Note: qemu may quited unexpectedly here, and the monitor will be freed. + * If it happened, priv->mon will be null. + */ + qemuDriverLock(driver); virDomainObjLock(obj); if (refs == 0) { priv->mon = NULL; } + + if (priv->mon == NULL) + return -1;

+ else + return 0; }

@@ -1452,7 +1452,11 @@ static int qemudDomainShutdown(virDomainPtr dom) { priv = vm->privateData; qemuDomainObjEnterMonitor(vm); ret = qemuMonitorSystemPowerdown(priv->mon); - qemuDomainObjExitMonitor(vm); + if (qemuDomainObjExitMonitor(vm) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + ret = -1; + }