--- src/qemu/qemu_domain.c | 30 ++----------- src/qemu/qemu_migration.c | 2 - src/qemu/qemu_monitor.c | 109 ++++++++++++++++++++++++-------------------- src/qemu/qemu_monitor.h | 4 +- src/qemu/qemu_process.c | 32 +++++++------- 5 files changed, 81 insertions(+), 96 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3a3c953..d11dc5f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -560,9 +560,8 @@ void qemuDomainObjEnterMonitor(virDomainObjPtr obj) { qemuDomainObjPrivatePtr priv = obj->privateData; - qemuMonitorLock(priv->mon); - qemuMonitorRef(priv->mon); virDomainObjUnlock(obj); + qemuMonitorLock(priv->mon); } @@ -573,18 +572,9 @@ void qemuDomainObjEnterMonitor(virDomainObjPtr obj) void qemuDomainObjExitMonitor(virDomainObjPtr obj) { qemuDomainObjPrivatePtr priv = obj->privateData; - int refs; - - refs = qemuMonitorUnref(priv->mon); - - if (refs > 0) - qemuMonitorUnlock(priv->mon); + qemuMonitorUnlock(priv->mon); virDomainObjLock(obj); - - if (refs == 0) { - priv->mon = NULL; - } } @@ -601,10 +591,8 @@ void qemuDomainObjEnterMonitorWithDriver(struct qemud_driver *driver, { qemuDomainObjPrivatePtr priv = obj->privateData; - qemuMonitorLock(priv->mon); - qemuMonitorRef(priv->mon); virDomainObjUnlock(obj); - qemuDriverUnlock(driver); + qemuMonitorLock(priv->mon); } @@ -617,19 +605,9 @@ void qemuDomainObjExitMonitorWithDriver(struct qemud_driver *driver, virDomainObjPtr obj) { qemuDomainObjPrivatePtr priv = obj->privateData; - int refs; - - refs = qemuMonitorUnref(priv->mon); - - if (refs > 0) - qemuMonitorUnlock(priv->mon); - qemuDriverLock(driver); + qemuMonitorUnlock(priv->mon); virDomainObjLock(obj); - - if (refs == 0) { - priv->mon = NULL; - } } void qemuDomainObjEnterRemoteWithDriver(struct qemud_driver *driver, diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 462e6be..6af2e24 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -224,11 +224,9 @@ qemuMigrationWaitForCompletion(struct qemud_driver *driver, virDomainObjPtr vm) } virDomainObjUnlock(vm); - qemuDriverUnlock(driver); nanosleep(&ts, NULL); - qemuDriverLock(driver); virDomainObjLock(vm); } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 2d28f8d..98c55e7 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -37,6 +37,7 @@ #include "memory.h" #include "logging.h" #include "files.h" +#include "virobject.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -44,11 +45,10 @@ #define DEBUG_RAW_IO 0 struct _qemuMonitor { + virObject obj; virMutex lock; /* also used to protect fd */ virCond notify; - int refs; - int fd; int watch; int hasSendFD; @@ -203,35 +203,61 @@ void qemuMonitorUnlock(qemuMonitorPtr mon) } -static void qemuMonitorFree(qemuMonitorPtr mon) +static void doMonitorFree(virObjectPtr obj) { + qemuMonitorPtr mon = (qemuMonitorPtr)obj; + VIR_DEBUG("mon=%p", mon); if (mon->cb && mon->cb->destroy) (mon->cb->destroy)(mon, mon->vm); - if (virCondDestroy(&mon->notify) < 0) - {} + ignore_value(virCondDestroy(&mon->notify)); virMutexDestroy(&mon->lock); VIR_FREE(mon->buffer); VIR_FREE(mon); } -int qemuMonitorRef(qemuMonitorPtr mon) +static qemuMonitorPtr qemuMonitorAlloc(void) { - mon->refs++; - return mon->refs; -} + qemuMonitorPtr mon; -int qemuMonitorUnref(qemuMonitorPtr mon) -{ - mon->refs--; + if (VIR_ALLOC(mon) < 0) { + virReportOOMError(); + return NULL; + } - if (mon->refs == 0) { - qemuMonitorUnlock(mon); - qemuMonitorFree(mon); - return 0; + if (virObjectInit(&mon->obj, doMonitorFree)) { + VIR_FREE(mon); + return NULL; + } + + if (virMutexInit(&mon->lock) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot initialize monitor mutex")); + VIR_FREE(mon); + return NULL; } - return mon->refs; + if (virCondInit(&mon->notify) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot initialize monitor condition")); + virMutexDestroy(&mon->lock); + VIR_FREE(mon); + return NULL; + } + + mon->fd = -1; + + return mon; +} + +void qemuMonitorRef(qemuMonitorPtr mon) +{ + virObjectRef(&mon->obj); +} + +void qemuMonitorUnref(qemuMonitorPtr mon) +{ + virObjectUnref(&mon->obj); } static void @@ -239,9 +265,7 @@ qemuMonitorUnwatch(void *monitor) { qemuMonitorPtr mon = monitor; - qemuMonitorLock(mon); - if (qemuMonitorUnref(mon) > 0) - qemuMonitorUnlock(mon); + qemuMonitorUnref(mon); } static int @@ -521,7 +545,6 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) { /* lock access to the monitor and protect fd */ qemuMonitorLock(mon); - qemuMonitorRef(mon); #if DEBUG_IO VIR_DEBUG("Monitor %p I/O on watch %d fd %d events %d", mon, watch, fd, events); #endif @@ -598,13 +621,11 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) { /* Make sure anyone waiting wakes up now */ virCondSignal(&mon->notify); - if (qemuMonitorUnref(mon) > 0) - qemuMonitorUnlock(mon); + qemuMonitorUnlock(mon); VIR_DEBUG("Triggering EOF callback error? %d", failed); (eofNotify)(mon, vm, failed); } else { - if (qemuMonitorUnref(mon) > 0) - qemuMonitorUnlock(mon); + qemuMonitorUnlock(mon); } } @@ -623,30 +644,13 @@ qemuMonitorOpen(virDomainObjPtr vm, return NULL; } - if (VIR_ALLOC(mon) < 0) { - virReportOOMError(); + mon = qemuMonitorAlloc(); + if (!mon) return NULL; - } - if (virMutexInit(&mon->lock) < 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot initialize monitor mutex")); - VIR_FREE(mon); - return NULL; - } - if (virCondInit(&mon->notify) < 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot initialize monitor condition")); - virMutexDestroy(&mon->lock); - VIR_FREE(mon); - return NULL; - } - mon->fd = -1; - mon->refs = 1; mon->vm = vm; mon->json = json; mon->cb = cb; - qemuMonitorLock(mon); switch (config->type) { case VIR_DOMAIN_CHR_TYPE_UNIX: @@ -679,20 +683,25 @@ qemuMonitorOpen(virDomainObjPtr vm, } + /* mon will be accessed by qemuMonitorIO which is called in + * event thread, so ref it before passing it to the thread. + * + * Note: mon is unrefed in qemuMonitorUnwatch + */ + qemuMonitorRef(mon); if ((mon->watch = virEventAddHandle(mon->fd, VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR | VIR_EVENT_HANDLE_READABLE, qemuMonitorIO, mon, qemuMonitorUnwatch)) < 0) { + qemuMonitorUnref(mon); qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("unable to register monitor events")); goto cleanup; } - qemuMonitorRef(mon); VIR_DEBUG("New mon %p fd =%d watch=%d", mon, mon->fd, mon->watch); - qemuMonitorUnlock(mon); return mon; @@ -703,8 +712,8 @@ cleanup: * so kill the callbacks now. */ mon->cb = NULL; - qemuMonitorUnlock(mon); qemuMonitorClose(mon); + qemuMonitorUnref(mon); return NULL; } @@ -724,8 +733,8 @@ void qemuMonitorClose(qemuMonitorPtr mon) VIR_FORCE_CLOSE(mon->fd); } - if (qemuMonitorUnref(mon) > 0) - qemuMonitorUnlock(mon); + qemuMonitorUnlock(mon); + qemuMonitorUnref(mon); } @@ -778,7 +787,7 @@ int qemuMonitorHMPCommandWithFd(qemuMonitorPtr mon, if ((mon)->cb && (mon)->cb->callback) \ (ret) = ((mon)->cb->callback)(mon, __VA_ARGS__); \ qemuMonitorLock(mon); \ - ignore_value(qemuMonitorUnref(mon)); \ + qemuMonitorUnref(mon); \ } while (0) int qemuMonitorGetDiskSecret(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index c90219b..4498c49 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -132,8 +132,8 @@ int qemuMonitorCheckHMP(qemuMonitorPtr mon, const char *cmd); void qemuMonitorLock(qemuMonitorPtr mon); void qemuMonitorUnlock(qemuMonitorPtr mon); -int qemuMonitorRef(qemuMonitorPtr mon); -int qemuMonitorUnref(qemuMonitorPtr mon) ATTRIBUTE_RETURN_CHECK; +void qemuMonitorRef(qemuMonitorPtr mon); +void qemuMonitorUnref(qemuMonitorPtr mon); /* These APIs are for use by the internal Text/JSON monitor impl code only */ int qemuMonitorSend(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 244b22a..4b9087f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -107,7 +107,6 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED, VIR_DEBUG("Received EOF on %p '%s'", vm, vm->def->name); - qemuDriverLock(driver); virDomainObjLock(vm); if (!virDomainObjIsActive(vm)) { @@ -133,15 +132,17 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED, qemuProcessStop(driver, vm, 0); qemuAuditDomainStop(vm, hasError ? "failed" : "shutdown"); - if (!vm->persistent) + if (!vm->persistent) { + qemuDriverLock(driver); virDomainRemoveInactive(&driver->domains, vm); - else - virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + } + + virDomainObjUnlock(vm); if (event) { qemuDomainEventQueue(driver, event); } - qemuDriverUnlock(driver); } @@ -602,8 +603,10 @@ static void qemuProcessHandleMonitorDestroy(qemuMonitorPtr mon, virDomainObjLock(vm); priv = vm->privateData; - if (priv->mon == mon) + if (priv->mon == mon) { priv->mon = NULL; + qemuMonitorUnref(mon); + } virDomainObjUnlock(vm); virDomainObjUnref(vm); } @@ -633,19 +636,11 @@ qemuConnectMonitor(struct qemud_driver *driver, virDomainObjPtr vm) goto error; } - /* Hold an extra reference because we can't allow 'vm' to be - * deleted while the monitor is active */ - virDomainObjRef(vm); - priv->mon = qemuMonitorOpen(vm, priv->monConfig, priv->monJSON, &monitorCallbacks); - /* Safe to ignore value since ref count was incremented above */ - if (priv->mon == NULL) - virDomainObjUnref(vm); - if (virSecurityManagerClearSocketLabel(driver->securityManager, vm) < 0) { VIR_ERROR(_("Failed to clear security context for monitor for %s"), vm->def->name); @@ -657,6 +652,7 @@ qemuConnectMonitor(struct qemud_driver *driver, virDomainObjPtr vm) goto error; } + qemuMonitorRef(priv->mon); qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorSetCapabilities(priv->mon); @@ -2471,8 +2467,12 @@ void qemuProcessStop(struct qemud_driver *driver, _("Failed to send SIGTERM to %s (%d)"), vm->def->name, vm->pid); - if (priv->mon) - qemuMonitorClose(priv->mon); + if (priv->mon) { + qemuMonitorPtr mon = priv->mon; + priv->mon = NULL; + qemuMonitorClose(mon); + qemuMonitorUnref(mon); + } if (priv->monConfig) { if (priv->monConfig->type == VIR_DOMAIN_CHR_TYPE_UNIX) -- 1.7.3.1 -- Thanks, Hu Tao -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list