We don't want to hold a lock on the driver object for the duration of each monitor call, and indeed we should not even need to use it. So this patch stops passing the driver object to the QEMU monitor API We're basically changing static int qemudMonitorCommand (const struct qemud_driver *driver, const virDomainObjPtr vm, const char *cmd, char **reply); To: static int qemudMonitorCommand (const virDomainObjPtr vm, const char *cmd, char **reply); This arg was already redundant and annotated ATTRIBUTE_UNUSED so no functional change here Regards, Daniel diff --git a/src/qemu_driver.c b/src/qemu_driver.c --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -118,8 +118,7 @@ static void qemudShutdownVMDaemon(virCon static int qemudDomainGetMaxVcpus(virDomainPtr dom); -static int qemudMonitorCommand (const struct qemud_driver *driver, - const virDomainObjPtr vm, +static int qemudMonitorCommand (const virDomainObjPtr vm, const char *cmd, char **reply); @@ -311,14 +310,12 @@ qemudShutdown(void) { /* Return -1 for error, 1 to continue reading and 0 for success */ typedef int qemudHandlerMonitorOutput(virConnectPtr conn, - struct qemud_driver *driver, virDomainObjPtr vm, const char *output, int fd); static int qemudReadMonitorOutput(virConnectPtr conn, - struct qemud_driver *driver, virDomainObjPtr vm, int fd, char *buf, @@ -377,7 +374,7 @@ qemudReadMonitorOutput(virConnectPtr con } else { got += ret; buf[got] = '\0'; - if ((ret = func(conn, driver, vm, buf, fd)) != 1) + if ((ret = func(conn, vm, buf, fd)) != 1) return ret; } } @@ -391,7 +388,6 @@ qemudReadMonitorOutput(virConnectPtr con static int qemudCheckMonitorPrompt(virConnectPtr conn ATTRIBUTE_UNUSED, - struct qemud_driver *driver ATTRIBUTE_UNUSED, virDomainObjPtr vm, const char *output, int fd) @@ -405,7 +401,6 @@ qemudCheckMonitorPrompt(virConnectPtr co } static int qemudOpenMonitor(virConnectPtr conn, - struct qemud_driver *driver, virDomainObjPtr vm, const char *monitor) { int monfd; @@ -429,7 +424,7 @@ static int qemudOpenMonitor(virConnectPt } ret = qemudReadMonitorOutput(conn, - driver, vm, monfd, + vm, monfd, buf, sizeof(buf), qemudCheckMonitorPrompt, "monitor"); @@ -489,7 +484,6 @@ static int qemudExtractMonitorPath(virCo static int qemudFindCharDevicePTYs(virConnectPtr conn, - struct qemud_driver *driver, virDomainObjPtr vm, const char *output, int fd ATTRIBUTE_UNUSED) @@ -528,7 +522,7 @@ qemudFindCharDevicePTYs(virConnectPtr co } /* Got them all, so now open the monitor console */ - ret = qemudOpenMonitor(conn, driver, vm, monitor); + ret = qemudOpenMonitor(conn, vm, monitor); cleanup: VIR_FREE(monitor); @@ -536,11 +530,10 @@ cleanup: } static int qemudWaitForMonitor(virConnectPtr conn, - struct qemud_driver *driver, virDomainObjPtr vm) { char buf[1024]; /* Plenty of space to get startup greeting */ int ret = qemudReadMonitorOutput(conn, - driver, vm, vm->stderr_fd, + vm, vm->stderr_fd, buf, sizeof(buf), qemudFindCharDevicePTYs, "console"); @@ -557,7 +550,6 @@ static int qemudWaitForMonitor(virConnec static int qemudDetectVcpuPIDs(virConnectPtr conn, - struct qemud_driver *driver, virDomainObjPtr vm) { char *qemucpus = NULL; char *line; @@ -581,7 +573,7 @@ qemudDetectVcpuPIDs(virConnectPtr conn, return 0; } - if (qemudMonitorCommand(driver, vm, "info cpus", &qemucpus) < 0) { + if (qemudMonitorCommand(vm, "info cpus", &qemucpus) < 0) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", _("cannot run monitor command to fetch CPU thread info")); VIR_FREE(vm->vcpupids); @@ -657,7 +649,6 @@ error: static int qemudInitCpus(virConnectPtr conn, - struct qemud_driver *driver, virDomainObjPtr vm) { char *info = NULL; #if HAVE_SCHED_GETAFFINITY @@ -695,7 +686,7 @@ qemudInitCpus(virConnectPtr conn, #endif /* HAVE_SCHED_GETAFFINITY */ /* Allow the CPUS to start executing */ - if (qemudMonitorCommand(driver, vm, "cont", &info) < 0) { + if (qemudMonitorCommand(vm, "cont", &info) < 0) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "%s", _("resume operation failed")); return -1; @@ -912,9 +903,9 @@ static int qemudStartVMDaemon(virConnect POLLIN | POLLERR | POLLHUP, qemudDispatchVMEvent, driver) < 0) || - (qemudWaitForMonitor(conn, driver, vm) < 0) || - (qemudDetectVcpuPIDs(conn, driver, vm) < 0) || - (qemudInitCpus(conn, driver, vm) < 0)) { + (qemudWaitForMonitor(conn, vm) < 0) || + (qemudDetectVcpuPIDs(conn, vm) < 0) || + (qemudInitCpus(conn, vm) < 0)) { qemudShutdownVMDaemon(conn, driver, vm); return -1; } @@ -1043,8 +1034,7 @@ static void qemudDispatchVMEvent(int fd, } static int -qemudMonitorCommand (const struct qemud_driver *driver ATTRIBUTE_UNUSED, - const virDomainObjPtr vm, +qemudMonitorCommand (const virDomainObjPtr vm, const char *cmd, char **reply) { int size = 0; @@ -1496,7 +1486,7 @@ static int qemudDomainSuspend(virDomainP if (vm->state == VIR_DOMAIN_PAUSED) return 0; - if (qemudMonitorCommand(driver, vm, "stop", &info) < 0) { + if (qemudMonitorCommand(vm, "stop", &info) < 0) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("suspend operation failed")); return -1; @@ -1524,7 +1514,7 @@ static int qemudDomainResume(virDomainPt } if (vm->state == VIR_DOMAIN_RUNNING) return 0; - if (qemudMonitorCommand(driver, vm, "cont", &info) < 0) { + if (qemudMonitorCommand(vm, "cont", &info) < 0) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("resume operation failed")); return -1; @@ -1547,7 +1537,7 @@ static int qemudDomainShutdown(virDomain return -1; } - if (qemudMonitorCommand(driver, vm, "system_powerdown", &info) < 0) { + if (qemudMonitorCommand(vm, "system_powerdown", &info) < 0) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("shutdown operation failed")); return -1; @@ -1875,7 +1865,7 @@ static int qemudDomainSave(virDomainPtr } free(safe_path); - if (qemudMonitorCommand(driver, vm, command, &info) < 0) { + if (qemudMonitorCommand(vm, command, &info) < 0) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("migrate operation failed")); VIR_FREE(command); @@ -2201,7 +2191,7 @@ static int qemudDomainRestore(virConnect /* If it was running before, resume it now. */ if (header.was_running) { char *info; - if (qemudMonitorCommand(driver, vm, "cont", &info) < 0) { + if (qemudMonitorCommand(vm, "cont", &info) < 0) { qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("failed to resume domain")); return -1; @@ -2481,7 +2471,7 @@ static int qemudDomainChangeEjectableMed } VIR_FREE(devname); - if (qemudMonitorCommand(driver, vm, cmd, &reply) < 0) { + if (qemudMonitorCommand(vm, cmd, &reply) < 0) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("cannot change cdrom media")); VIR_FREE(cmd); @@ -2552,7 +2542,7 @@ static int qemudDomainAttachDiskDevice(v return ret; } - if (qemudMonitorCommand(driver, vm, cmd, &reply) < 0) { + if (qemudMonitorCommand(vm, cmd, &reply) < 0) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, _("cannot attach %s disk"), type); VIR_FREE(cmd); @@ -2603,7 +2593,7 @@ static int qemudDomainAttachUsbMassstora return ret; } - if (qemudMonitorCommand(driver, vm, cmd, &reply) < 0) { + if (qemudMonitorCommand(vm, cmd, &reply) < 0) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("cannot attach usb disk")); VIR_FREE(cmd); @@ -2662,7 +2652,7 @@ static int qemudDomainAttachHostDevice(v return -1; } - if (qemudMonitorCommand(driver, vm, cmd, &reply) < 0) { + if (qemudMonitorCommand(vm, cmd, &reply) < 0) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("cannot attach usb device")); VIR_FREE(cmd); @@ -2875,7 +2865,7 @@ qemudDomainBlockStats (virDomainPtr dom, return -1; len = strlen (qemu_dev_name); - if (qemudMonitorCommand (driver, vm, "info blockstats", &info) < 0) { + if (qemudMonitorCommand (vm, "info blockstats", &info) < 0) { qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("'info blockstats' command failed")); goto out; @@ -3107,7 +3097,7 @@ qemudDomainMemoryPeek (virDomainPtr dom, /* Issue the memsave command. */ snprintf (cmd, sizeof cmd, "memsave %llu %zi \"%s\"", offset, size, tmp); - if (qemudMonitorCommand (driver, vm, cmd, &info) < 0) { + if (qemudMonitorCommand (vm, cmd, &info) < 0) { qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("'memsave' command failed")); goto done; -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

This patch makes use of the virDomainObjPtr locking infrastructure to provide thread safety in the QEMU driver. The QEMU driver itself gains a global lock. The idea is this lock is only held for short periods of time - typically only while obtaining a virDomainObjPtr instance. The main limitation of the current locking implemented here, is that the virDomainObjPtr lock is held for the duration of all monitor commands. Some of these can take quite a long time to execute, so for a useful level of concurrency we need to drop the lock & reobtain it when issuing monitor commands. I intend to deal with this problem in an add-on patch, since I'd rather keep the initial impl simple to understand. The general model for an API call involving a virDomainPtr is qemudXXXXX(virDomainPtr dom) { struct qemud_driver *driver = conn->privateData; virDomainObjPtr vm; qemudDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, uuid); qemudDriverUnlock(driver); .... do work .... virDomainObjUnlock(vm); return 0; error: virDomainObjUnlock(vm); return -1; } The important thing to note is that if anything goes wrong in the '... do work...' bit of the driver call, it should never do a direct 'return -1;'. It must always 'goto error' instead. This avoids the need to sprinkle calls to virDomainObjUnlock(vm) everywhere. Daniel diff --git a/src/qemu_conf.h b/src/qemu_conf.h --- a/src/qemu_conf.h +++ b/src/qemu_conf.h @@ -48,6 +48,8 @@ enum qemud_cmd_flags { /* Main driver state */ struct qemud_driver { + pthread_mutex_t lock; + unsigned int qemuVersion; int nextvmid; diff --git a/src/qemu_driver.c b/src/qemu_driver.c --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -76,6 +76,22 @@ static int qemudShutdown(void); #define qemudLog(level, msg...) fprintf(stderr, msg) + +static int +qemudDriverLock(struct qemud_driver *driver) +{ + DEBUG("LOCK driver %p %p", (driver), (void*)pthread_self()); + return pthread_mutex_lock(&(driver)->lock); +} + +static int +qemudDriverUnlock(struct qemud_driver *driver) +{ + DEBUG("UNLOCK driver %p %p", (driver), (void*)pthread_self()); + return pthread_mutex_unlock(&(driver)->lock); +} + + static int qemudSetCloseExec(int fd) { int flags; if ((flags = fcntl(fd, F_GETFD)) < 0) @@ -130,13 +146,18 @@ qemudAutostartConfigs(struct qemud_drive unsigned int i; for (i = 0 ; i < driver->domains.count ; i++) { - if (driver->domains.objs[i]->autostart && - !virDomainIsActive(driver->domains.objs[i]) && - qemudStartVMDaemon(NULL, driver, driver->domains.objs[i], NULL) < 0) { + virDomainObjPtr dom = driver->domains.objs[i]; + virDomainLock(dom); + + if (dom->autostart && + !virDomainIsActive(dom) && + qemudStartVMDaemon(NULL, driver, dom, NULL) < 0) { virErrorPtr err = virGetLastError(); qemudLog(QEMUD_ERR, _("Failed to autostart VM '%s': %s\n"), - driver->domains.objs[i]->def->name, err->message); + dom->def->name, err->message); } + + virDomainUnlock(dom); } } @@ -157,6 +178,7 @@ qemudStartup(void) { /* Don't have a dom0 so start from 1 */ qemu_driver->nextvmid = 1; + pthread_mutex_init(&qemu_driver->lock, NULL); if (!uid) { if (asprintf(&qemu_driver->logDir, @@ -203,15 +225,20 @@ qemudStartup(void) { return -1; } + qemudDriverLock(qemu_driver); + if (virDomainLoadAllConfigs(NULL, qemu_driver->caps, &qemu_driver->domains, qemu_driver->configDir, qemu_driver->autostartDir) < 0) { + qemudDriverUnlock(qemu_driver); qemudShutdown(); return -1; } qemudAutostartConfigs(qemu_driver); + + qemudDriverUnlock(qemu_driver); return 0; @@ -235,6 +262,8 @@ qemudReload(void) { if (!qemu_driver) return 0; + qemudDriverLock(qemu_driver); + virDomainLoadAllConfigs(NULL, qemu_driver->caps, &qemu_driver->domains, @@ -242,6 +271,8 @@ qemudReload(void) { qemu_driver->autostartDir); qemudAutostartConfigs(qemu_driver); + + qemudDriverUnlock(qemu_driver); return 0; } @@ -257,16 +288,23 @@ static int static int qemudActive(void) { unsigned int i; + int isactive = 0; if (!qemu_driver) return 0; - for (i = 0 ; i < qemu_driver->domains.count ; i++) + qemudDriverLock(qemu_driver); + + for (i = 0 ; i < qemu_driver->domains.count ; i++) { + virDomainLock(qemu_driver->domains.objs[i]); if (virDomainIsActive(qemu_driver->domains.objs[i])) - return 1; + isactive = 1; + virDomainUnlock(qemu_driver->domains.objs[i]); + } - /* Otherwise we're happy to deal with a shutdown */ - return 0; + qemudDriverUnlock(qemu_driver); + + return isactive; } /** @@ -1222,12 +1260,16 @@ static char *qemudGetCapabilities(virCon struct qemud_driver *driver = (struct qemud_driver *)conn->privateData; char *xml; + qemudDriverLock(driver); + if ((xml = virCapabilitiesFormatXML(driver->caps)) == NULL) { + qemudDriverUnlock(driver); qemudReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, "%s", _("failed to allocate space for capabilities support")); return NULL; } + qemudDriverUnlock(driver); return xml; } @@ -1327,8 +1369,12 @@ static virDomainPtr qemudDomainLookupByI static virDomainPtr qemudDomainLookupByID(virConnectPtr conn, int id) { struct qemud_driver *driver = (struct qemud_driver *)conn->privateData; - virDomainObjPtr vm = virDomainFindByID(&driver->domains, id); + virDomainObjPtr vm; virDomainPtr dom; + + qemudDriverLock(driver); + vm = virDomainFindByID(&driver->domains, id); + qemudDriverUnlock(driver); if (!vm) { qemudReportError(conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL); @@ -1337,13 +1383,19 @@ static virDomainPtr qemudDomainLookupByI dom = virGetDomain(conn, vm->def->name, vm->def->uuid); if (dom) dom->id = vm->def->id; + + virDomainUnlock(vm); return dom; } static virDomainPtr qemudDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { struct qemud_driver *driver = (struct qemud_driver *)conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, uuid); + virDomainObjPtr vm; virDomainPtr dom; + + qemudDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, uuid); + qemudDriverUnlock(driver); if (!vm) { qemudReportError(conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL); @@ -1352,13 +1404,19 @@ static virDomainPtr qemudDomainLookupByU dom = virGetDomain(conn, vm->def->name, vm->def->uuid); if (dom) dom->id = vm->def->id; + + virDomainUnlock(vm); return dom; } static virDomainPtr qemudDomainLookupByName(virConnectPtr conn, const char *name) { struct qemud_driver *driver = (struct qemud_driver *)conn->privateData; - virDomainObjPtr vm = virDomainFindByName(&driver->domains, name); + virDomainObjPtr vm; virDomainPtr dom; + + qemudDriverLock(driver); + vm = virDomainFindByName(&driver->domains, name); + qemudDriverUnlock(driver); if (!vm) { qemudReportError(conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL); @@ -1367,15 +1425,23 @@ static virDomainPtr qemudDomainLookupByN dom = virGetDomain(conn, vm->def->name, vm->def->uuid); if (dom) dom->id = vm->def->id; + + virDomainUnlock(vm); return dom; } static int qemudGetVersion(virConnectPtr conn, unsigned long *version) { struct qemud_driver *driver = (struct qemud_driver *)conn->privateData; - if (qemudExtractVersion(conn, driver) < 0) + qemudDriverLock(driver); + + if (qemudExtractVersion(conn, driver) < 0) { + qemudDriverUnlock(driver); return -1; + } *version = qemu_driver->qemuVersion; + + qemudDriverUnlock(driver); return 0; } @@ -1405,39 +1471,51 @@ static int qemudListDomains(virConnectPt struct qemud_driver *driver = (struct qemud_driver *)conn->privateData; int got = 0, i; - for (i = 0 ; i < driver->domains.count && got < nids ; i++) + qemudDriverLock(driver); + for (i = 0 ; i < driver->domains.count && got < nids ; i++) { + virDomainLock(driver->domains.objs[i]); if (virDomainIsActive(driver->domains.objs[i])) ids[got++] = driver->domains.objs[i]->def->id; + virDomainUnlock(driver->domains.objs[i]); + } + qemudDriverUnlock(driver); return got; } + static int qemudNumDomains(virConnectPtr conn) { struct qemud_driver *driver = (struct qemud_driver *)conn->privateData; int n = 0, i; - for (i = 0 ; i < driver->domains.count ; i++) + qemudDriverLock(driver); + for (i = 0 ; i < driver->domains.count ; i++) { + virDomainLock(driver->domains.objs[i]); if (virDomainIsActive(driver->domains.objs[i])) n++; + virDomainUnlock(driver->domains.objs[i]); + } + qemudDriverUnlock(driver); return n; } static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, unsigned int flags ATTRIBUTE_UNUSED) { virDomainDefPtr def; - virDomainObjPtr vm; + virDomainObjPtr vm = NULL; virDomainPtr dom; struct qemud_driver *driver = (struct qemud_driver *)conn->privateData; + qemudDriverLock(driver); + if (!(def = virDomainDefParseString(conn, driver->caps, xml))) - return NULL; + goto cleanup; vm = virDomainFindByName(&driver->domains, def->name); if (vm) { qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, _("domain '%s' is already defined"), def->name); - virDomainDefFree(def); - return NULL; + goto cleanup; } vm = virDomainFindByUUID(&driver->domains, def->uuid); if (vm) { @@ -1447,149 +1525,225 @@ static virDomainPtr qemudDomainCreate(vi qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, _("domain with uuid '%s' is already defined"), uuidstr); - virDomainDefFree(def); - return NULL; + goto cleanup; } if (!(vm = virDomainAssignDef(conn, &driver->domains, - def))) { - virDomainDefFree(def); - return NULL; - } + def))) + goto cleanup; - if (qemudStartVMDaemon(conn, driver, vm, NULL) < 0) { - virDomainRemoveInactive(&driver->domains, - vm); - return NULL; - } + /* XXX should figure out how to release driver lock here + * so we don't block during startup */ + if (qemudStartVMDaemon(conn, driver, vm, NULL) < 0) + goto cleanup; dom = virGetDomain(conn, vm->def->name, vm->def->uuid); if (dom) dom->id = vm->def->id; + + virDomainUnlock(vm); + qemudDriverUnlock(driver); return dom; + +cleanup: + if (vm) { + virDomainUnlock(vm); + virDomainRemoveInactive(&driver->domains, + vm); + } else { + virDomainDefFree(def); + } + + qemudDriverUnlock(driver); + return NULL; } static int qemudDomainSuspend(virDomainPtr dom) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - char *info; - virDomainObjPtr vm = virDomainFindByID(&driver->domains, dom->id); + char *info = NULL; + virDomainObjPtr vm; + + qemudDriverLock(driver); + vm = virDomainFindByID(&driver->domains, dom->id); + qemudDriverUnlock(driver); + if (!vm) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, _("no domain with matching id %d"), dom->id); - return -1; + goto cleanup; } if (!virDomainIsActive(vm)) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("domain is not running")); - return -1; + goto cleanup; } - if (vm->state == VIR_DOMAIN_PAUSED) - return 0; - if (qemudMonitorCommand(vm, "stop", &info) < 0) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, - "%s", _("suspend operation failed")); - return -1; + if (vm->state != VIR_DOMAIN_PAUSED) { + if (qemudMonitorCommand(vm, "stop", &info) < 0) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("suspend operation failed")); + goto cleanup; + } + vm->state = VIR_DOMAIN_PAUSED; + qemudDebug("Reply %s", info); + VIR_FREE(info); } - vm->state = VIR_DOMAIN_PAUSED; - qemudDebug("Reply %s", info); + + virDomainUnlock(vm); + return 0; + +cleanup: VIR_FREE(info); - return 0; + if (vm) + virDomainUnlock(vm); + return -1; } static int qemudDomainResume(virDomainPtr dom) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - char *info; - virDomainObjPtr vm = virDomainFindByID(&driver->domains, dom->id); + char *info = NULL; + virDomainObjPtr vm; + + qemudDriverLock(driver); + vm = virDomainFindByID(&driver->domains, dom->id); + qemudDriverUnlock(driver); + if (!vm) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, _("no domain with matching id %d"), dom->id); - return -1; + goto cleanup; } if (!virDomainIsActive(vm)) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("domain is not running")); - return -1; + goto cleanup; } - if (vm->state == VIR_DOMAIN_RUNNING) - return 0; - if (qemudMonitorCommand(vm, "cont", &info) < 0) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, - "%s", _("resume operation failed")); - return -1; + if (vm->state != VIR_DOMAIN_RUNNING) { + if (qemudMonitorCommand(vm, "cont", &info) < 0) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + "%s", _("resume operation failed")); + goto cleanup; + } + vm->state = VIR_DOMAIN_RUNNING; + qemudDebug("Reply %s", info); + VIR_FREE(info); } - vm->state = VIR_DOMAIN_RUNNING; - qemudDebug("Reply %s", info); + + virDomainUnlock(vm); + return 0; + +cleanup: VIR_FREE(info); - return 0; + if (vm) + virDomainUnlock(vm); + return -1; } static int qemudDomainShutdown(virDomainPtr dom) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByID(&driver->domains, dom->id); - char* info; + virDomainObjPtr vm; + char* info = NULL; + + qemudDriverLock(driver); + vm = virDomainFindByID(&driver->domains, dom->id); + qemudDriverUnlock(driver); if (!vm) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, _("no domain with matching id %d"), dom->id); - return -1; + goto cleanup; } if (qemudMonitorCommand(vm, "system_powerdown", &info) < 0) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("shutdown operation failed")); - return -1; + goto cleanup; } VIR_FREE(info); + virDomainUnlock(vm); return 0; +cleanup: + VIR_FREE(info); + if (vm) + virDomainUnlock(vm); + return -1; } static int qemudDomainDestroy(virDomainPtr dom) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByID(&driver->domains, dom->id); + virDomainObjPtr vm; + + qemudDriverLock(driver); + vm = virDomainFindByID(&driver->domains, dom->id); + qemudDriverUnlock(driver); if (!vm) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, _("no domain with matching id %d"), dom->id); - return -1; + goto cleanup; } qemudShutdownVMDaemon(dom->conn, driver, vm); - if (!vm->persistent) + if (!vm->persistent) { + virDomainUnlock(vm); virDomainRemoveInactive(&driver->domains, vm); + return 0; + } + virDomainUnlock(vm); return 0; + +cleanup: + if (vm) + virDomainUnlock(vm); + return -1; } static char *qemudDomainGetOSType(virDomainPtr dom) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); + virDomainObjPtr vm; char *type; + + qemudDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemudDriverUnlock(driver); if (!vm) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, "%s", _("no domain with matching uuid")); - return NULL; + goto cleanup; } if (!(type = strdup(vm->def->os.type))) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_MEMORY, "%s", _("failed to allocate space for ostype")); - return NULL; + goto cleanup; } + + virDomainUnlock(vm); return type; + +cleanup: + if (vm) + virDomainUnlock(vm); + return NULL; } /* Returns max memory in kb, 0 if error */ static unsigned long qemudDomainGetMaxMemory(virDomainPtr dom) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); + virDomainObjPtr vm; + unsigned long ret; + + qemudDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemudDriverUnlock(driver); if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; @@ -1597,15 +1751,27 @@ static unsigned long qemudDomainGetMaxMe virUUIDFormat(dom->uuid, uuidstr); qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); - return 0; + goto cleanup; } - return vm->def->maxmem; + ret = vm->def->maxmem; + virDomainUnlock(vm); + + return ret; + +cleanup: + if (vm) + virDomainUnlock(vm); + return 0; } static int qemudDomainSetMaxMemory(virDomainPtr dom, unsigned long newmax) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); + virDomainObjPtr vm; + + qemudDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemudDriverUnlock(driver); if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; @@ -1613,22 +1779,32 @@ static int qemudDomainSetMaxMemory(virDo virUUIDFormat(dom->uuid, uuidstr); qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); - return -1; + goto cleanup; } if (newmax < vm->def->memory) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_ARG, "%s", _("cannot set max memory lower than current memory")); - return -1; + goto cleanup; } vm->def->maxmem = newmax; + virDomainUnlock(vm); return 0; + +cleanup: + if (vm) + virDomainUnlock(vm); + return -1; } static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); + virDomainObjPtr vm; + + qemudDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemudDriverUnlock(driver); if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; @@ -1636,33 +1812,44 @@ static int qemudDomainSetMemory(virDomai virUUIDFormat(dom->uuid, uuidstr); qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); - return -1; + goto cleanup; } if (virDomainIsActive(vm)) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT, "%s", _("cannot set memory of an active domain")); - return -1; + goto cleanup; } if (newmem > vm->def->maxmem) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_ARG, "%s", _("cannot set memory higher than max memory")); - return -1; + goto cleanup; } vm->def->memory = newmem; + virDomainUnlock(vm); return 0; + +cleanup: + if (vm) + virDomainUnlock(vm); + return -1; } static int qemudDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); + virDomainObjPtr vm; + + qemudDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemudDriverUnlock(driver); + if (!vm) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, "%s", _("no domain with matching uuid")); - return -1; + goto cleanup; } info->state = vm->state; @@ -1671,15 +1858,23 @@ static int qemudDomainGetInfo(virDomainP info->cpuTime = 0; } else { if (qemudGetProcessInfo(&(info->cpuTime), vm->pid) < 0) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, ("cannot read cputime for domain")); - return -1; + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, + _("cannot read cputime for domain")); + goto cleanup; } } info->maxMem = vm->def->maxmem; info->memory = vm->def->memory; info->nrVirtCpu = vm->def->vcpus; + + virDomainUnlock(vm); return 0; + +cleanup: + if (vm) + virDomainUnlock(vm); + return -1; } @@ -1779,12 +1974,16 @@ static int qemudDomainSave(virDomainPtr static int qemudDomainSave(virDomainPtr dom, const char *path) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByID(&driver->domains, dom->id); + virDomainObjPtr vm; char *command, *info; - int fd; + int fd = -1; char *safe_path; - char *xml; + char *xml = NULL; struct qemud_save_header header; + + qemudDriverLock(driver); + vm = virDomainFindByID(&driver->domains, dom->id); + qemudDriverUnlock(driver); memset(&header, 0, sizeof(header)); memcpy(header.magic, QEMUD_SAVE_MAGIC, sizeof(header.magic)); @@ -1793,13 +1992,13 @@ static int qemudDomainSave(virDomainPtr if (!vm) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, _("no domain with matching id %d"), dom->id); - return -1; + goto cleanup; } if (!virDomainIsActive(vm)) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("domain is not running")); - return -1; + goto cleanup; } /* Pause */ @@ -1808,7 +2007,7 @@ static int qemudDomainSave(virDomainPtr if (qemudDomainSuspend(dom) != 0) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("failed to pause domain")); - return -1; + goto cleanup; } } @@ -1817,7 +2016,7 @@ static int qemudDomainSave(virDomainPtr if (!xml) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("failed to get domain xml")); - return -1; + goto cleanup; } header.xml_len = strlen(xml) + 1; @@ -1825,24 +2024,19 @@ static int qemudDomainSave(virDomainPtr if ((fd = open(path, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR)) < 0) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, _("failed to create '%s'"), path); - VIR_FREE(xml); - return -1; + goto cleanup; } if (safewrite(fd, &header, sizeof(header)) != sizeof(header)) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("failed to write save header")); - close(fd); - VIR_FREE(xml); - return -1; + goto cleanup; } if (safewrite(fd, xml, header.xml_len) != header.xml_len) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("failed to write xml")); - close(fd); - VIR_FREE(xml); - return -1; + goto cleanup; } close(fd); @@ -1890,18 +2084,33 @@ static int qemudDomainSave(virDomainPtr /* Shut it down */ qemudShutdownVMDaemon(dom->conn, driver, vm); - if (!vm->persistent) + if (!vm->persistent) { + virDomainUnlock(vm); virDomainRemoveInactive(&driver->domains, vm); + return 0; + } + virDomainUnlock(vm); return 0; + +cleanup: + if (fd != -1) + close(fd); + VIR_FREE(xml); + virDomainUnlock(vm); + return -1; } static int qemudDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); + virDomainObjPtr vm; int max; + + qemudDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemudDriverUnlock(driver); if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; @@ -1909,30 +2118,37 @@ static int qemudDomainSetVcpus(virDomain virUUIDFormat(dom->uuid, uuidstr); qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); - return -1; + goto cleanup; } if (virDomainIsActive(vm)) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT, "%s", _("cannot change vcpu count of an active domain")); - return -1; + goto cleanup; } if ((max = qemudDomainGetMaxVcpus(dom)) < 0) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, "%s", _("could not determine max vcpus for the domain")); - return -1; + goto cleanup; } if (nvcpus > max) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_ARG, _("requested vcpus is greater than max allowable" " vcpus for the domain: %d > %d"), nvcpus, max); - return -1; + goto cleanup; } vm->def->vcpus = nvcpus; + + virDomainUnlock(vm); return 0; + +cleanup: + if (vm) + virDomainUnlock(vm); + return -1; } @@ -1943,26 +2159,39 @@ qemudDomainPinVcpu(virDomainPtr dom, unsigned char *cpumap, int maplen) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); + virDomainObjPtr vm; cpu_set_t mask; int i, maxcpu; virNodeInfo nodeinfo; + qemudDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemudDriverUnlock(driver); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + if (!virDomainIsActive(vm)) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_ARG, "%s",_("cannot pin vcpus on an inactive domain")); - return -1; + goto cleanup; } if (vcpu > (vm->nvcpupids-1)) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_ARG, _("vcpu number out of range %d > %d"), vcpu, vm->nvcpupids); - return -1; + goto cleanup; } if (virNodeInfoPopulate(dom->conn, &nodeinfo) < 0) - return -1; + goto cleanup; maxcpu = maplen * 8; if (maxcpu > nodeinfo.cpus) @@ -1978,15 +2207,21 @@ qemudDomainPinVcpu(virDomainPtr dom, if (sched_setaffinity(vm->vcpupids[vcpu], sizeof(mask), &mask) < 0) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_ARG, _("cannot set affinity: %s"), strerror(errno)); - return -1; + goto cleanup; } } else { qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT, "%s", _("cpu affinity is not supported")); - return -1; + goto cleanup; } + virDomainUnlock(vm); return 0; + +cleanup: + if (vm) + virDomainUnlock(vm); + return -1; } static int @@ -1996,18 +2231,31 @@ qemudDomainGetVcpus(virDomainPtr dom, unsigned char *cpumaps, int maplen) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); + virDomainObjPtr vm; virNodeInfo nodeinfo; int i, v, maxcpu; + + qemudDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemudDriverUnlock(driver); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + virUUIDFormat(dom->uuid, uuidstr); + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } if (!virDomainIsActive(vm)) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_ARG, "%s",_("cannot pin vcpus on an inactive domain")); - return -1; + goto cleanup; } if (virNodeInfoPopulate(dom->conn, &nodeinfo) < 0) - return -1; + goto cleanup; maxcpu = maplen * 8; if (maxcpu > nodeinfo.cpus) @@ -2017,8 +2265,10 @@ qemudDomainGetVcpus(virDomainPtr dom, if (maxinfo > vm->nvcpupids) maxinfo = vm->nvcpupids; - if (maxinfo < 1) + if (maxinfo < 1) { + virDomainUnlock(vm); return 0; + } if (info != NULL) { memset(info, 0, sizeof(*info) * maxinfo); @@ -2040,7 +2290,7 @@ qemudDomainGetVcpus(virDomainPtr dom, if (sched_getaffinity(vm->vcpupids[v], sizeof(mask), &mask) < 0) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_ARG, _("cannot get affinity: %s"), strerror(errno)); - return -1; + goto cleanup; } for (i = 0 ; i < maxcpu ; i++) @@ -2050,20 +2300,30 @@ qemudDomainGetVcpus(virDomainPtr dom, } else { qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT, "%s", _("cpu affinity is not available")); - return -1; + goto cleanup; } } + virDomainUnlock(vm); return maxinfo; + +cleanup: + if (vm) + virDomainUnlock(vm); + return -1; } #endif /* HAVE_SCHED_GETAFFINITY */ static int qemudDomainGetMaxVcpus(virDomainPtr dom) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); + virDomainObjPtr vm; const char *type; int ret; + + qemudDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemudDriverUnlock(driver); if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; @@ -2071,21 +2331,27 @@ static int qemudDomainGetMaxVcpus(virDom virUUIDFormat(dom->uuid, uuidstr); qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); - return -1; + goto cleanup; } if (!(type = virDomainVirtTypeToString(vm->def->virtType))) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, _("unknown virt type in domain definition '%d'"), vm->def->virtType); - return -1; + goto cleanup; } if ((ret = qemudGetMaxVCPUs(dom->conn, type)) < 0) { - return -1; + goto cleanup; } + virDomainUnlock(vm); return ret; + +cleanup: + if (vm) + virDomainUnlock(vm); + return -1; } @@ -2093,10 +2359,10 @@ static int qemudDomainRestore(virConnect const char *path) { struct qemud_driver *driver = (struct qemud_driver *)conn->privateData; virDomainDefPtr def; - virDomainObjPtr vm; - int fd; + virDomainObjPtr vm = NULL; + int fd = -1; int ret; - char *xml; + char *xml = NULL; struct qemud_save_header header; /* Verify the header and read the XML */ @@ -2143,13 +2409,14 @@ static int qemudDomainRestore(virConnect return -1; } + + qemudDriverLock(driver); + /* Create a domain from this XML */ if (!(def = virDomainDefParseString(conn, driver->caps, xml))) { qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("failed to parse XML")); - close(fd); - VIR_FREE(xml); - return -1; + goto cleanup; } VIR_FREE(xml); @@ -2160,9 +2427,10 @@ static int qemudDomainRestore(virConnect if (vm && virDomainIsActive(vm)) { qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, _("domain is already active as '%s'"), vm->def->name); - close(fd); - return -1; + goto cleanup; } + if (vm) + virDomainUnlock(vm); if (!(vm = virDomainAssignDef(conn, &driver->domains, @@ -2170,22 +2438,25 @@ static int qemudDomainRestore(virConnect qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("failed to assign new VM")); virDomainDefFree(def); - close(fd); - return -1; + goto cleanup; } /* Set the migration source and start it up. */ vm->stdin_fd = fd; ret = qemudStartVMDaemon(conn, driver, vm, "stdio"); close(fd); + fd = -1; vm->stdin_fd = -1; if (ret < 0) { qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("failed to start VM")); - if (!vm->persistent) + if (!vm->persistent) { + virDomainUnlock(vm); virDomainRemoveInactive(&driver->domains, vm); - return -1; + vm = NULL; + } + goto cleanup; } /* If it was running before, resume it now. */ @@ -2194,30 +2465,57 @@ static int qemudDomainRestore(virConnect if (qemudMonitorCommand(vm, "cont", &info) < 0) { qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("failed to resume domain")); - return -1; + goto cleanup; } VIR_FREE(info); vm->state = VIR_DOMAIN_RUNNING; } + virDomainUnlock(vm); + qemudDriverUnlock(driver); return 0; + +cleanup: + if (fd != -1) + close(fd); + VIR_FREE(xml); + if (vm) + virDomainUnlock(vm); + qemudDriverUnlock(driver); + return -1; } static char *qemudDomainDumpXML(virDomainPtr dom, int flags ATTRIBUTE_UNUSED) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); + virDomainObjPtr vm; + char *ret; + + qemudDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemudDriverUnlock(driver); + if (!vm) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, "%s", _("no domain with matching uuid")); - return NULL; + goto cleanup; } - return virDomainDefFormat(dom->conn, - (flags & VIR_DOMAIN_XML_INACTIVE) && vm->newDef ? - vm->newDef : vm->def, - flags); + ret = virDomainDefFormat(dom->conn, + (flags & VIR_DOMAIN_XML_INACTIVE) && vm->newDef ? + vm->newDef : vm->def, + flags); + if (!ret) + goto cleanup; + + virDomainUnlock(vm); + return ret; + +cleanup: + if (vm) + virDomainUnlock(vm); + return NULL; } @@ -2226,15 +2524,20 @@ static int qemudListDefinedDomains(virCo struct qemud_driver *driver = (struct qemud_driver *)conn->privateData; int got = 0, i; + qemudDriverLock(driver); for (i = 0 ; i < driver->domains.count && got < nnames ; i++) { + virDomainLock(driver->domains.objs[i]); if (!virDomainIsActive(driver->domains.objs[i])) { if (!(names[got++] = strdup(driver->domains.objs[i]->def->name))) { qemudReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, "%s", _("failed to allocate space for VM name string")); + virDomainUnlock(driver->domains.objs[i]); goto cleanup; } } + virDomainUnlock(driver->domains.objs[i]); } + qemudDriverUnlock(driver); return got; @@ -2248,9 +2551,14 @@ static int qemudNumDefinedDomains(virCon struct qemud_driver *driver = (struct qemud_driver *)conn->privateData; int n = 0, i; - for (i = 0 ; i < driver->domains.count ; i++) + qemudDriverLock(driver); + for (i = 0 ; i < driver->domains.count ; i++) { + virDomainLock(driver->domains.objs[i]); if (!virDomainIsActive(driver->domains.objs[i])) n++; + virDomainUnlock(driver->domains.objs[i]); + } + qemudDriverUnlock(driver); return n; } @@ -2258,77 +2566,110 @@ static int qemudNumDefinedDomains(virCon static int qemudDomainStart(virDomainPtr dom) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); + virDomainObjPtr vm; + int ret; + + qemudDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, "%s", _("no domain with matching uuid")); + qemudDriverUnlock(driver); return -1; } - return qemudStartVMDaemon(dom->conn, driver, vm, NULL); + ret = qemudStartVMDaemon(dom->conn, driver, vm, NULL); + virDomainUnlock(vm); + qemudDriverUnlock(driver); + return ret; } static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) { struct qemud_driver *driver = (struct qemud_driver *)conn->privateData; virDomainDefPtr def; - virDomainObjPtr vm; + virDomainObjPtr vm = NULL; virDomainPtr dom; + qemudDriverLock(driver); + if (!(def = virDomainDefParseString(conn, driver->caps, xml))) - return NULL; + goto cleanup; if (!(vm = virDomainAssignDef(conn, &driver->domains, def))) { virDomainDefFree(def); - return NULL; + goto cleanup; } vm->persistent = 1; if (virDomainSaveConfig(conn, driver->configDir, vm->newDef ? vm->newDef : vm->def) < 0) { + virDomainUnlock(vm); virDomainRemoveInactive(&driver->domains, vm); - return NULL; + vm = NULL; + goto cleanup; } dom = virGetDomain(conn, vm->def->name, vm->def->uuid); if (dom) dom->id = vm->def->id; + + virDomainUnlock(vm); + qemudDriverUnlock(driver); return dom; + +cleanup: + if (vm) + virDomainUnlock(vm); + qemudDriverUnlock(driver); + return NULL; } static int qemudDomainUndefine(virDomainPtr dom) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); + virDomainObjPtr vm = NULL; + + qemudDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); if (!vm) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, "%s", _("no domain with matching uuid")); - return -1; + goto cleanup; } if (virDomainIsActive(vm)) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, "%s", _("cannot delete active domain")); - return -1; + goto cleanup; } if (!vm->persistent) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, "%s", _("cannot undefine transient domain")); - return -1; + goto cleanup; } if (virDomainDeleteConfig(dom->conn, driver->configDir, driver->autostartDir, vm) < 0) - return -1; + goto cleanup; + virDomainUnlock(vm); virDomainRemoveInactive(&driver->domains, vm); + qemudDriverUnlock(driver); + return 0; + +cleanup: + if (vm) + virDomainUnlock(vm); + qemudDriverLock(driver); + return -1; } /* Return the disks name for use in monitor commands */ @@ -2681,9 +3022,13 @@ static int qemudDomainAttachDevice(virDo static int qemudDomainAttachDevice(virDomainPtr dom, const char *xml) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); + virDomainObjPtr vm; virDomainDeviceDefPtr dev; int ret = 0, supported = 0; + + qemudDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemudDriverUnlock(driver); if (!vm) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, @@ -2694,13 +3039,11 @@ static int qemudDomainAttachDevice(virDo if (!virDomainIsActive(vm)) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, "%s", _("cannot attach device on inactive domain")); - return -1; + goto cleanup; } - dev = virDomainDeviceDefParse(dom->conn, vm->def, xml); - if (dev == NULL) { - return -1; - } + if (!(dev = virDomainDeviceDefParse(dom->conn, vm->def, xml))) + goto cleanup; if (dev->type == VIR_DOMAIN_DEVICE_DISK) { switch (dev->data.disk->device) { @@ -2723,8 +3066,8 @@ static int qemudDomainAttachDevice(virDo } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && dev->data.hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && dev->data.hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) { - supported = 1; - ret = qemudDomainAttachHostDevice(dom, dev); + supported = 1; + ret = qemudDomainAttachHostDevice(dom, dev); } if (!supported) { @@ -2734,13 +3077,24 @@ static int qemudDomainAttachDevice(virDo } VIR_FREE(dev); + + virDomainUnlock(vm); return ret; + +cleanup: + if (vm) + virDomainUnlock(vm); + return -1; } static int qemudDomainGetAutostart(virDomainPtr dom, int *autostart) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); + virDomainObjPtr vm; + + qemudDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemudDriverUnlock(driver); if (!vm) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, @@ -2750,71 +3104,80 @@ static int qemudDomainGetAutostart(virDo *autostart = vm->autostart; + virDomainUnlock(vm); return 0; } static int qemudDomainSetAutostart(virDomainPtr dom, int autostart) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); + virDomainObjPtr vm; char *configFile = NULL, *autostartLink = NULL; - int ret = -1; + + qemudDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemudDriverUnlock(driver); if (!vm) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, "%s", _("no domain with matching uuid")); - return -1; + goto cleanup; } if (!vm->persistent) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, "%s", _("cannot set autostart for transient domain")); - return -1; + goto cleanup; } autostart = (autostart != 0); - if (vm->autostart == autostart) - return 0; + if (vm->autostart != autostart) { + if ((configFile = virDomainConfigFile(dom->conn, driver->configDir, vm->def->name)) == NULL) + goto cleanup; + if ((autostartLink = virDomainConfigFile(dom->conn, driver->autostartDir, vm->def->name)) == NULL) + goto cleanup; - if ((configFile = virDomainConfigFile(dom->conn, driver->configDir, vm->def->name)) == NULL) - goto cleanup; - if ((autostartLink = virDomainConfigFile(dom->conn, driver->autostartDir, vm->def->name)) == NULL) - goto cleanup; + if (autostart) { + int err; - if (autostart) { - int err; + if ((err = virFileMakePath(driver->autostartDir))) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, + _("cannot create autostart directory %s: %s"), + driver->autostartDir, strerror(err)); + goto cleanup; + } - if ((err = virFileMakePath(driver->autostartDir))) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, - _("cannot create autostart directory %s: %s"), - driver->autostartDir, strerror(err)); - goto cleanup; + if (symlink(configFile, autostartLink) < 0) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, + _("Failed to create symlink '%s to '%s': %s"), + autostartLink, configFile, strerror(errno)); + goto cleanup; + } + } else { + if (unlink(autostartLink) < 0 && errno != ENOENT && errno != ENOTDIR) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, + _("Failed to delete symlink '%s': %s"), + autostartLink, strerror(errno)); + goto cleanup; + } } - if (symlink(configFile, autostartLink) < 0) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, - _("Failed to create symlink '%s to '%s': %s"), - autostartLink, configFile, strerror(errno)); - goto cleanup; - } - } else { - if (unlink(autostartLink) < 0 && errno != ENOENT && errno != ENOTDIR) { - qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, - _("Failed to delete symlink '%s': %s"), - autostartLink, strerror(errno)); - goto cleanup; - } + vm->autostart = autostart; } - vm->autostart = autostart; - ret = 0; + VIR_FREE(configFile); + VIR_FREE(autostartLink); + virDomainUnlock(vm); + return 0; cleanup: + if (vm) + virDomainUnlock(vm); VIR_FREE(configFile); VIR_FREE(autostartLink); - return ret; + return -1; } /* This uses the 'info blockstats' monitor command which was @@ -2833,8 +3196,12 @@ qemudDomainBlockStats (virDomainPtr dom, const char *qemu_dev_name = NULL; size_t len; int i, ret = -1; - const virDomainObjPtr vm = virDomainFindByID(&driver->domains, dom->id); + virDomainObjPtr vm; virDomainDiskDefPtr disk = NULL; + + qemudDriverLock(driver); + vm = virDomainFindByID(&driver->domains, dom->id); + qemudDriverUnlock(driver); if (!vm) { qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, @@ -2844,7 +3211,7 @@ qemudDomainBlockStats (virDomainPtr dom, if (!virDomainIsActive (vm)) { qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("domain is not running")); - return -1; + goto cleanup; } for (i = 0 ; i < vm->def->ndisks ; i++) { @@ -2857,7 +3224,7 @@ qemudDomainBlockStats (virDomainPtr dom, if (!disk) { qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_ARG, _("invalid path: %s"), path); - return -1; + goto cleanup; } qemu_dev_name = qemudDiskDeviceName(dom, disk); @@ -2868,7 +3235,7 @@ qemudDomainBlockStats (virDomainPtr dom, if (qemudMonitorCommand (vm, "info blockstats", &info) < 0) { qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("'info blockstats' command failed")); - goto out; + goto cleanup; } DEBUG ("info blockstats reply: %s", info); @@ -2881,7 +3248,7 @@ qemudDomainBlockStats (virDomainPtr dom, qemudReportError (dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT, "%s", _("'info blockstats' not supported by this qemu")); - goto out; + goto cleanup; } stats->rd_req = -1; @@ -2933,7 +3300,7 @@ qemudDomainBlockStats (virDomainPtr dom, p++; } ret = 0; - goto out; + goto done; } /* Skip to next line. */ @@ -2945,10 +3312,20 @@ qemudDomainBlockStats (virDomainPtr dom, /* If we reach here then the device was not found. */ qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_ARG, _("device not found: %s (%s)"), path, qemu_dev_name); - out: + goto cleanup; + + done: VIR_FREE(qemu_dev_name); VIR_FREE(info); - return ret; + virDomainUnlock(vm); + return 0; + +cleanup: + VIR_FREE(qemu_dev_name); + VIR_FREE(info); + if (vm) + virDomainUnlock(vm); + return -1; } static int @@ -2958,25 +3335,29 @@ qemudDomainInterfaceStats (virDomainPtr { #ifdef __linux__ struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByID(&driver->domains, dom->id); - int i; + virDomainObjPtr vm; + int i, ret; + + qemudDriverLock(driver); + vm = virDomainFindByID(&driver->domains, dom->id); + qemudDriverUnlock(driver); if (!vm) { qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, _("no domain with matching id %d"), dom->id); - return -1; + goto cleanup; } if (!virDomainIsActive(vm)) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("domain is not running")); - return -1; + goto cleanup; } if (!path || path[0] == '\0') { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_ARG, "%s", _("NULL or empty path")); - return -1; + goto cleanup; } /* Check the path is one of the domain's network interfaces. */ @@ -2988,10 +3369,17 @@ qemudDomainInterfaceStats (virDomainPtr qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_ARG, _("invalid path, '%s' is not a known interface"), path); + goto cleanup; + + ok: + ret = linuxDomainInterfaceStats (dom->conn, path, stats); + virDomainUnlock(vm); + return ret; + +cleanup: + if (vm) + virDomainUnlock(vm); return -1; - ok: - - return linuxDomainInterfaceStats (dom->conn, path, stats); #else qemudReportError (dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT, "%s", __FUNCTION__); @@ -3007,19 +3395,23 @@ qemudDomainBlockPeek (virDomainPtr dom, unsigned int flags ATTRIBUTE_UNUSED) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid); + virDomainObjPtr vm; int fd, ret = -1, i; + + qemudDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemudDriverUnlock(driver); if (!vm) { qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, "%s", _("no domain with matching uuid")); - return -1; + goto cleanup; } if (!path || path[0] == '\0') { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_ARG, "%s", _("NULL or empty path")); - return -1; + goto cleanup; } /* Check the path belongs to this domain. */ @@ -3030,7 +3422,7 @@ qemudDomainBlockPeek (virDomainPtr dom, } qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_ARG, "%s", _("invalid path")); - return -1; + goto cleanup; found: /* The path is correct, now try to open it and get its size. */ @@ -3055,7 +3447,14 @@ found: ret = 0; done: if (fd >= 0) close (fd); + + virDomainUnlock(vm); return ret; + +cleanup: + if (vm) + virDomainUnlock(vm); + return -1; } static int @@ -3065,34 +3464,38 @@ qemudDomainMemoryPeek (virDomainPtr dom, unsigned int flags) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; - virDomainObjPtr vm = virDomainFindByID(&driver->domains, dom->id); + virDomainObjPtr vm; char cmd[256], *info; char tmp[] = TEMPDIR "/qemu.mem.XXXXXX"; int fd = -1, ret = -1; + qemudDriverLock(driver); + vm = virDomainFindByID(&driver->domains, dom->id); + qemudDriverUnlock(driver); + if (flags != VIR_MEMORY_VIRTUAL) { qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_ARG, "%s", _("QEMU driver only supports virtual memory addrs")); - return -1; + goto cleanup; } if (!vm) { qemudReportError (dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, _("no domain with matching id %d"), dom->id); - return -1; + goto cleanup; } if (!virDomainIsActive(vm)) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("domain is not running")); - return -1; + goto cleanup; } /* Create a temporary filename. */ if ((fd = mkstemp (tmp)) == -1) { qemudReportError (dom->conn, dom, NULL, VIR_ERR_SYSTEM_ERROR, "%s", strerror (errno)); - return -1; + goto cleanup; } /* Issue the memsave command. */ @@ -3104,7 +3507,7 @@ qemudDomainMemoryPeek (virDomainPtr dom, } DEBUG ("memsave reply: %s", info); - free (info); + VIR_FREE(info); /* Read the memory file into buffer. */ if (saferead (fd, buffer, size) == (ssize_t) -1) { @@ -3117,7 +3520,13 @@ done: done: if (fd >= 0) close (fd); unlink (tmp); + virDomainUnlock(vm); return ret; + +cleanup: + if (vm) + virDomainUnlock(vm); + return -1; } -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|