diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index 7568f93742..61fe468fde 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -313,40 +313,6 @@ virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr doms, } -int -virDomainObjListRenameAddNew(virDomainObjListPtr doms, - virDomainObjPtr vm, - const char *name) -{ - int ret = -1; - virObjectLock(doms); - - /* Add new name into the hash table of domain names. */ - if (virHashAddEntry(doms->objsName, name, vm) < 0) - goto cleanup; - - /* Okay, this is crazy. virHashAddEntry() does not increment - * the refcounter of @vm, but virHashRemoveEntry() does - * decrement it. We need to work around it. */ - virObjectRef(vm); - - ret = 0; - cleanup: - virObjectUnlock(doms); - return ret; -} - - -int -virDomainObjListRenameRemove(virDomainObjListPtr doms, const char *name) -{ - virObjectLock(doms); - virHashRemoveEntry(doms->objsName, name); - virObjectUnlock(doms); - return 0; -} - - /* * The caller must hold a lock on the driver owning 'doms', * and must also have locked 'dom', to ensure no one else @@ -372,6 +338,72 @@ void virDomainObjListRemove(virDomainObjListPtr doms, } +/** + * virDomainObjListRename: + * + * The caller must hold a lock on dom. Callbacks should not + * sleep/wait otherwise operations on all domains will be blocked + * as the callback is called with domains lock hold. Domain lock + * is dropped/reacquired during this operation thus domain + * consistency must not rely on this lock solely. + */ +int +virDomainObjListRename(virDomainObjListPtr doms, + virDomainObjPtr dom, + const char *new_name, + unsigned int flags, + virDomainObjListRenameCallback callback, + void *opaque) +{ + int ret = -1; + char *old_name = NULL; + int rc; + + if (STREQ(dom->def->name, new_name)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Can't rename domain to itself")); + return ret; + } + + if (VIR_STRDUP(old_name, dom->def->name) < 0) + return ret; + + /* doms and dom locks must be attained in right order thus relock dom. */ + /* dom reference is touched for the benefit of those callers that + * hold a lock on dom but not refcount it. */ + virObjectRef(dom); + virObjectUnlock(dom); + virObjectLock(doms); + virObjectLock(dom); + virObjectUnref(dom); + + if (virHashLookup(doms->objsName, new_name) != NULL) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("domain with name '%s' already exists"), + new_name); + goto cleanup; + } + + if (virHashAddEntry(doms->objsName, new_name, dom) < 0) + goto cleanup; + + /* Okay, this is crazy. virHashAddEntry() does not increment + * the refcounter of @dom, but virHashRemoveEntry() does + * decrement it. We need to work around it. */ + virObjectRef(dom); + + rc = callback(dom, new_name, flags, opaque); + virHashRemoveEntry(doms->objsName, rc < 0 ? new_name : old_name); + if (rc < 0) + goto cleanup; + + ret = 0; + cleanup: + virObjectUnlock(doms); + VIR_FREE(old_name); + return ret; +} + /* The caller must hold lock on 'doms' in addition to 'virDomainObjListRemove' * requirements * diff --git a/src/conf/virdomainobjlist.h b/src/conf/virdomainobjlist.h index f47959825b..c0f856c6e7 100644 --- a/src/conf/virdomainobjlist.h +++ b/src/conf/virdomainobjlist.h @@ -51,11 +51,17 @@ virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr doms, unsigned int flags, virDomainDefPtr *oldDef); -int virDomainObjListRenameAddNew(virDomainObjListPtr doms, - virDomainObjPtr vm, - const char *name); -int virDomainObjListRenameRemove(virDomainObjListPtr doms, - const char *name); +typedef int (*virDomainObjListRenameCallback)(virDomainObjPtr dom, + const char *new_name, + unsigned int flags, + void *opaque); +int virDomainObjListRename(virDomainObjListPtr doms, + virDomainObjPtr dom, + const char *new_name, + unsigned int flags, + virDomainObjListRenameCallback callback, + void *opaque); + void virDomainObjListRemove(virDomainObjListPtr doms, virDomainObjPtr dom); void virDomainObjListRemoveLocked(virDomainObjListPtr doms, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 049180ac84..5f4322ff64 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -894,8 +894,7 @@ virDomainObjListNew; virDomainObjListNumOfDomains; virDomainObjListRemove; virDomainObjListRemoveLocked; -virDomainObjListRenameAddNew; -virDomainObjListRenameRemove; +virDomainObjListRename; # cpu/cpu.h diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b6133b741e..51b9aad00d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19999,14 +19999,14 @@ qemuDomainSetUserPassword(virDomainPtr dom, } -static int qemuDomainRename(virDomainPtr dom, - const char *new_name, - unsigned int flags) +static int +qemuDomainRenameCallback(virDomainObjPtr vm, + const char *new_name, + unsigned int flags, + void *opaque) { - virQEMUDriverPtr driver = dom->conn->privateData; + virQEMUDriverPtr driver = opaque; virQEMUDriverConfigPtr cfg = NULL; - virDomainObjPtr vm = NULL; - virDomainObjPtr tmp_dom = NULL; virObjectEventPtr event_new = NULL; virObjectEventPtr event_old = NULL; int ret = -1; @@ -20016,14 +20016,75 @@ static int qemuDomainRename(virDomainPtr dom, virCheckFlags(0, ret); + cfg = virQEMUDriverGetConfig(driver); + + if (VIR_STRDUP(new_dom_name, new_name) < 0) + goto cleanup; + + if (!(old_dom_cfg_file = virDomainConfigFile(cfg->configDir, + vm->def->name))) { + goto cleanup; + } + + event_old = virDomainEventLifecycleNewFromObj(vm, + VIR_DOMAIN_EVENT_UNDEFINED, + VIR_DOMAIN_EVENT_UNDEFINED_RENAMED); + + /* Switch name in domain definition. */ + old_dom_name = vm->def->name; + vm->def->name = new_dom_name; + new_dom_name = NULL; + + if (virDomainSaveConfig(cfg->configDir, vm->def) < 0) + goto rollback; + + if (virFileExists(old_dom_cfg_file) && + unlink(old_dom_cfg_file) < 0) { + virReportSystemError(errno, + _("cannot remove old domain config file %s"), + old_dom_cfg_file); + goto rollback; + } + + event_new = virDomainEventLifecycleNewFromObj(vm, + VIR_DOMAIN_EVENT_DEFINED, + VIR_DOMAIN_EVENT_DEFINED_RENAMED); + ret = 0; + + cleanup: + VIR_FREE(old_dom_cfg_file); + VIR_FREE(old_dom_name); + VIR_FREE(new_dom_name); + qemuDomainEventQueue(driver, event_old); + qemuDomainEventQueue(driver, event_new); + virObjectUnref(cfg); + return ret; + + rollback: + if (old_dom_name) { + new_dom_name = vm->def->name; + vm->def->name = old_dom_name; + old_dom_name = NULL; + } + goto cleanup; +} + +static int qemuDomainRename(virDomainPtr dom, + const char *new_name, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + int ret = -1; + + virCheckFlags(0, ret); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; if (virDomainRenameEnsureACL(dom->conn, vm->def) < 0) goto cleanup; - cfg = virQEMUDriverGetConfig(driver); - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; @@ -20051,64 +20112,9 @@ static int qemuDomainRename(virDomainPtr dom, goto endjob; } - if (STREQ(vm->def->name, new_name)) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("Can't rename domain to itself")); + if (virDomainObjListRename(driver->domains, vm, new_name, flags, + qemuDomainRenameCallback, driver) < 0) goto endjob; - } - - /* - * This is a rather racy check, but still better than reporting - * internal error. And since new_name != name here, there's no - * deadlock imminent. - */ - tmp_dom = virDomainObjListFindByName(driver->domains, new_name); - if (tmp_dom) { - virObjectUnlock(tmp_dom); - virObjectUnref(tmp_dom); - virReportError(VIR_ERR_OPERATION_INVALID, - _("domain with name '%s' already exists"), - new_name); - goto endjob; - } - - if (VIR_STRDUP(new_dom_name, new_name) < 0) - goto endjob; - - if (!(old_dom_cfg_file = virDomainConfigFile(cfg->configDir, - vm->def->name))) { - goto endjob; - } - - if (virDomainObjListRenameAddNew(driver->domains, vm, new_name) < 0) - goto endjob; - - event_old = virDomainEventLifecycleNewFromObj(vm, - VIR_DOMAIN_EVENT_UNDEFINED, - VIR_DOMAIN_EVENT_UNDEFINED_RENAMED); - - /* Switch name in domain definition. */ - old_dom_name = vm->def->name; - vm->def->name = new_dom_name; - new_dom_name = NULL; - - if (virDomainSaveConfig(cfg->configDir, vm->def) < 0) - goto rollback; - - if (virFileExists(old_dom_cfg_file) && - unlink(old_dom_cfg_file) < 0) { - virReportSystemError(errno, - _("cannot remove old domain config file %s"), - old_dom_cfg_file); - goto rollback; - } - - /* Remove old domain name from table. */ - virDomainObjListRenameRemove(driver->domains, old_dom_name); - - event_new = virDomainEventLifecycleNewFromObj(vm, - VIR_DOMAIN_EVENT_DEFINED, - VIR_DOMAIN_EVENT_DEFINED_RENAMED); /* Success, domain has been renamed. */ ret = 0; @@ -20118,23 +20124,7 @@ static int qemuDomainRename(virDomainPtr dom, cleanup: virDomainObjEndAPI(&vm); - VIR_FREE(old_dom_cfg_file); - VIR_FREE(old_dom_name); - VIR_FREE(new_dom_name); - qemuDomainEventQueue(driver, event_old); - qemuDomainEventQueue(driver, event_new); - virObjectUnref(cfg); return ret; - - rollback: - if (old_dom_name) { - new_dom_name = vm->def->name; - vm->def->name = old_dom_name; - old_dom_name = NULL; - } - - virDomainObjListRenameRemove(driver->domains, new_name); - goto endjob; } static virHypervisorDriver qemuHypervisorDriver = {