[libvirt] [PATCH 0/2] Break out virDomainObjIsDuplicate function
Code to check for a VM duplicate in create/define/restore is present in various forms throughout several drivers. Break this code out into its own function (virDomainObjIsDuplicate) and use where applicable. Cole Robinson (2): qemu: Break out function to check if we can create/define/restore qemu: Use same create/define overwrite logic for migration prepare. src/conf/domain_conf.c | 64 ++++++++++++++++++++++ src/conf/domain_conf.h | 4 ++ src/libvirt_private.syms | 1 + src/lxc/lxc_driver.c | 68 ++--------------------- src/qemu/qemu_driver.c | 132 ++++----------------------------------------- src/test/test_driver.c | 14 +++++- src/uml/uml_driver.c | 20 ++------ 7 files changed, 104 insertions(+), 199 deletions(-)
Use this function in the qemu, uml, lxc, and test drivers. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 64 +++++++++++++++++++++++++++++ src/conf/domain_conf.h | 4 ++ src/libvirt_private.syms | 1 + src/lxc/lxc_driver.c | 68 +++---------------------------- src/qemu/qemu_driver.c | 102 ++++------------------------------------------ src/test/test_driver.c | 14 ++++++- src/uml/uml_driver.c | 20 ++------- 7 files changed, 100 insertions(+), 173 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7dd3ce7..37209d4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5014,6 +5014,70 @@ virDomainFSDefPtr virDomainGetRootFilesystem(virDomainDefPtr def) return NULL; } +/* + * virDomainObjIsDuplicate: + * @doms : virDomainObjListPtr to search + * @def : virDomainDefPtr definition of domain to lookup + * @check_active: If true, ensure that domain is not active + * + * Returns: -1 on error + * 0 if domain is new + * 1 if domain is a duplicate + */ +int +virDomainObjIsDuplicate(virDomainObjListPtr doms, + virDomainDefPtr def, + unsigned int check_active) +{ + int ret = -1; + int dupVM = 0; + virDomainObjPtr vm = NULL; + + /* See if a VM with matching UUID already exists */ + vm = virDomainFindByUUID(doms, def->uuid); + if (vm) { + /* UUID matches, but if names don't match, refuse it */ + if (STRNEQ(vm->def->name, def->name)) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(vm->def->uuid, uuidstr); + virDomainReportError(NULL, VIR_ERR_OPERATION_FAILED, + _("domain '%s' is already defined with uuid %s"), + vm->def->name, uuidstr); + goto cleanup; + } + + if (check_active) { + /* UUID & name match, but if VM is already active, refuse it */ + if (virDomainObjIsActive(vm)) { + virDomainReportError(NULL, VIR_ERR_OPERATION_INVALID, + _("domain is already active as '%s'"), + vm->def->name); + goto cleanup; + } + } + + dupVM = 1; + virDomainObjUnlock(vm); + } else { + /* UUID does not match, but if a name matches, refuse it */ + vm = virDomainFindByName(doms, def->name); + if (vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(vm->def->uuid, uuidstr); + virDomainReportError(NULL, VIR_ERR_OPERATION_FAILED, + _("domain '%s' already exists with uuid %s"), + def->name, uuidstr); + goto cleanup; + } + } + + ret = dupVM; +cleanup: + if (vm) + virDomainObjUnlock(vm); + return ret; +} + void virDomainObjLock(virDomainObjPtr obj) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8599ee7..9d3e9e2 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -788,6 +788,10 @@ virDomainFSDefPtr virDomainGetRootFilesystem(virDomainDefPtr def); int virDomainVideoDefaultType(virDomainDefPtr def); int virDomainVideoDefaultRAM(virDomainDefPtr def, int type); +int virDomainObjIsDuplicate(virDomainObjListPtr doms, + virDomainDefPtr def, + unsigned int check_active); + void virDomainObjLock(virDomainObjPtr obj); void virDomainObjUnlock(virDomainObjPtr obj); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 15d75fd..451a883 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -146,6 +146,7 @@ virDomainObjLock; virDomainObjUnlock; virDomainStateTypeToString; virDomainStateTypeFromString; +virDomainObjIsDuplicate; virDomainObjListGetInactiveNames; virDomainObjListGetActiveIDs; virDomainObjListNumOfDomains; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 9ab94bf..4c237f2 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -273,41 +273,15 @@ static virDomainPtr lxcDomainDefine(virConnectPtr conn, const char *xml) virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; virDomainEventPtr event = NULL; - int newVM = 1; + int dupVM; lxcDriverLock(driver); if (!(def = virDomainDefParseString(conn, driver->caps, xml, VIR_DOMAIN_XML_INACTIVE))) goto cleanup; - /* See if a VM with matching UUID already exists */ - vm = virDomainFindByUUID(&driver->domains, def->uuid); - if (vm) { - /* UUID matches, but if names don't match, refuse it */ - if (STRNEQ(vm->def->name, def->name)) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(vm->def->uuid, uuidstr); - lxcError(conn, NULL, VIR_ERR_OPERATION_FAILED, - _("domain '%s' is already defined with uuid %s"), - vm->def->name, uuidstr); - goto cleanup; - } - - /* UUID & name match */ - virDomainObjUnlock(vm); - newVM = 0; - } else { - /* UUID does not match, but if a name matches, refuse it */ - vm = virDomainFindByName(&driver->domains, def->name); - if (vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(vm->def->uuid, uuidstr); - lxcError(conn, NULL, VIR_ERR_OPERATION_FAILED, - _("domain '%s' is already defined with uuid %s"), - def->name, uuidstr); - goto cleanup; - } - } + if ((dupVM = virDomainObjIsDuplicate(&driver->domains, def, 0)) < 0) + goto cleanup; if ((def->nets != NULL) && !(driver->have_netns)) { lxcError(conn, NULL, VIR_ERR_NO_SUPPORT, @@ -331,7 +305,7 @@ static virDomainPtr lxcDomainDefine(virConnectPtr conn, const char *xml) event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_DEFINED, - newVM ? + !dupVM ? VIR_DOMAIN_EVENT_DEFINED_ADDED : VIR_DOMAIN_EVENT_DEFINED_UPDATED); @@ -1273,38 +1247,8 @@ lxcDomainCreateAndStart(virConnectPtr conn, VIR_DOMAIN_XML_INACTIVE))) goto cleanup; - /* See if a VM with matching UUID already exists */ - vm = virDomainFindByUUID(&driver->domains, def->uuid); - if (vm) { - /* UUID matches, but if names don't match, refuse it */ - if (STRNEQ(vm->def->name, def->name)) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(vm->def->uuid, uuidstr); - lxcError(conn, NULL, VIR_ERR_OPERATION_FAILED, - _("domain '%s' is already defined with uuid %s"), - vm->def->name, uuidstr); - goto cleanup; - } - - /* UUID & name match, but if VM is already active, refuse it */ - if (virDomainObjIsActive(vm)) { - lxcError(conn, NULL, VIR_ERR_OPERATION_FAILED, - _("domain is already active as '%s'"), vm->def->name); - goto cleanup; - } - virDomainObjUnlock(vm); - } else { - /* UUID does not match, but if a name matches, refuse it */ - vm = virDomainFindByName(&driver->domains, def->name); - if (vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(vm->def->uuid, uuidstr); - lxcError(conn, NULL, VIR_ERR_OPERATION_FAILED, - _("domain '%s' is already defined with uuid %s"), - def->name, uuidstr); - goto cleanup; - } - } + if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) + goto cleanup; if ((def->nets != NULL) && !(driver->have_netns)) { lxcError(conn, NULL, VIR_ERR_NO_SUPPORT, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5ff7e94..20621d1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2638,38 +2638,8 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, if (virSecurityDriverVerify(conn, def) < 0) goto cleanup; - /* See if a VM with matching UUID already exists */ - vm = virDomainFindByUUID(&driver->domains, def->uuid); - if (vm) { - /* UUID matches, but if names don't match, refuse it */ - if (STRNEQ(vm->def->name, def->name)) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(vm->def->uuid, uuidstr); - qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain '%s' is already defined with uuid %s"), - vm->def->name, uuidstr); - goto cleanup; - } - - /* UUID & name match, but if VM is already active, refuse it */ - if (virDomainObjIsActive(vm)) { - qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain is already active as '%s'"), vm->def->name); - goto cleanup; - } - virDomainObjUnlock(vm); - } else { - /* UUID does not match, but if a name matches, refuse it */ - vm = virDomainFindByName(&driver->domains, def->name); - if (vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(vm->def->uuid, uuidstr); - qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain '%s' is already defined with uuid %s"), - def->name, uuidstr); - goto cleanup; - } - } + if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) + goto cleanup; if (!(vm = virDomainAssignDef(conn, driver->caps, @@ -3699,38 +3669,8 @@ static int qemudDomainRestore(virConnectPtr conn, goto cleanup; } - /* See if a VM with matching UUID already exists */ - vm = virDomainFindByUUID(&driver->domains, def->uuid); - if (vm) { - /* UUID matches, but if names don't match, refuse it */ - if (STRNEQ(vm->def->name, def->name)) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(vm->def->uuid, uuidstr); - qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain '%s' is already defined with uuid %s"), - vm->def->name, uuidstr); - goto cleanup; - } - - /* UUID & name match, but if VM is already active, refuse it */ - if (virDomainObjIsActive(vm)) { - qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_INVALID, - _("domain is already active as '%s'"), vm->def->name); - goto cleanup; - } - virDomainObjUnlock(vm); - } else { - /* UUID does not match, but if a name matches, refuse it */ - vm = virDomainFindByName(&driver->domains, def->name); - if (vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(vm->def->uuid, uuidstr); - qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain '%s' is already defined with uuid %s"), - def->name, uuidstr); - goto cleanup; - } - } + if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) + goto cleanup; if (!(vm = virDomainAssignDef(conn, driver->caps, @@ -4177,7 +4117,7 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) { virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; virDomainEventPtr event = NULL; - int newVM = 1; + int dupVM; qemuDriverLock(driver); if (!(def = virDomainDefParseString(conn, driver->caps, xml, @@ -4187,34 +4127,8 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) { if (virSecurityDriverVerify(conn, def) < 0) goto cleanup; - /* See if a VM with matching UUID already exists */ - vm = virDomainFindByUUID(&driver->domains, def->uuid); - if (vm) { - /* UUID matches, but if names don't match, refuse it */ - if (STRNEQ(vm->def->name, def->name)) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(vm->def->uuid, uuidstr); - qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain '%s' is already defined with uuid %s"), - vm->def->name, uuidstr); - goto cleanup; - } - - /* UUID & name match */ - virDomainObjUnlock(vm); - newVM = 0; - } else { - /* UUID does not match, but if a name matches, refuse it */ - vm = virDomainFindByName(&driver->domains, def->name); - if (vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(vm->def->uuid, uuidstr); - qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain '%s' is already defined with uuid %s"), - def->name, uuidstr); - goto cleanup; - } - } + if ((dupVM = virDomainObjIsDuplicate(&driver->domains, def, 0)) < 0) + goto cleanup; if (qemudCanonicalizeMachine(driver, def) < 0) goto cleanup; @@ -4239,7 +4153,7 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) { event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_DEFINED, - newVM ? + !dupVM ? VIR_DOMAIN_EVENT_DEFINED_ADDED : VIR_DOMAIN_EVENT_DEFINED_UPDATED); diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 343834c..fb367c9 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1242,6 +1242,9 @@ testDomainCreateXML(virConnectPtr conn, const char *xml, VIR_DOMAIN_XML_INACTIVE)) == NULL) goto cleanup; + if (virDomainObjIsDuplicate(&privconn->domains, def, 1) < 0) + goto cleanup; + if (testDomainGenerateIfnames(conn, def) < 0) goto cleanup; if (!(dom = virDomainAssignDef(conn, privconn->caps, @@ -1785,6 +1788,9 @@ static int testDomainRestore(virConnectPtr conn, if (!def) goto cleanup; + if (virDomainObjIsDuplicate(&privconn->domains, def, 1) < 0) + goto cleanup; + if (testDomainGenerateIfnames(conn, def) < 0) goto cleanup; if (!(dom = virDomainAssignDef(conn, privconn->caps, @@ -2224,12 +2230,16 @@ static virDomainPtr testDomainDefineXML(virConnectPtr conn, virDomainDefPtr def; virDomainObjPtr dom = NULL; virDomainEventPtr event = NULL; + int dupVM; testDriverLock(privconn); if ((def = virDomainDefParseString(conn, privconn->caps, xml, VIR_DOMAIN_XML_INACTIVE)) == NULL) goto cleanup; + if ((dupVM = virDomainObjIsDuplicate(&privconn->domains, def, 0)) < 0) + goto cleanup; + if (testDomainGenerateIfnames(conn, def) < 0) goto cleanup; if (!(dom = virDomainAssignDef(conn, privconn->caps, @@ -2240,7 +2250,9 @@ static virDomainPtr testDomainDefineXML(virConnectPtr conn, event = virDomainEventNewFromObj(dom, VIR_DOMAIN_EVENT_DEFINED, - VIR_DOMAIN_EVENT_DEFINED_ADDED); + !dupVM ? + VIR_DOMAIN_EVENT_DEFINED_ADDED : + VIR_DOMAIN_EVENT_DEFINED_UPDATED); ret = virGetDomain(conn, dom->def->name, dom->def->uuid); if (ret) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 212cd8f..0604742 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1178,23 +1178,8 @@ static virDomainPtr umlDomainCreate(virConnectPtr conn, const char *xml, VIR_DOMAIN_XML_INACTIVE))) goto cleanup; - vm = virDomainFindByName(&driver->domains, def->name); - if (vm) { - umlReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain '%s' is already defined"), - def->name); + if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) goto cleanup; - } - vm = virDomainFindByUUID(&driver->domains, def->uuid); - if (vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - - virUUIDFormat(def->uuid, uuidstr); - umlReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain with uuid '%s' is already defined"), - uuidstr); - goto cleanup; - } if (!(vm = virDomainAssignDef(conn, driver->caps, @@ -1531,6 +1516,9 @@ static virDomainPtr umlDomainDefine(virConnectPtr conn, const char *xml) { VIR_DOMAIN_XML_INACTIVE))) goto cleanup; + if (virDomainObjIsDuplicate(&driver->domains, def, 0) < 0) + goto cleanup; + if (!(vm = virDomainAssignDef(conn, driver->caps, &driver->domains, -- 1.6.5.1
On Wed, Nov 04, 2009 at 03:06:58PM -0500, Cole Robinson wrote:
Use this function in the qemu, uml, lxc, and test drivers.
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 64 +++++++++++++++++++++++++++++ src/conf/domain_conf.h | 4 ++ src/libvirt_private.syms | 1 + src/lxc/lxc_driver.c | 68 +++---------------------------- src/qemu/qemu_driver.c | 102 ++++------------------------------------------ src/test/test_driver.c | 14 ++++++- src/uml/uml_driver.c | 20 ++------- 7 files changed, 100 insertions(+), 173 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7dd3ce7..37209d4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5014,6 +5014,70 @@ virDomainFSDefPtr virDomainGetRootFilesystem(virDomainDefPtr def) return NULL; }
+/* + * virDomainObjIsDuplicate: + * @doms : virDomainObjListPtr to search + * @def : virDomainDefPtr definition of domain to lookup + * @check_active: If true, ensure that domain is not active + * + * Returns: -1 on error + * 0 if domain is new + * 1 if domain is a duplicate + */ +int +virDomainObjIsDuplicate(virDomainObjListPtr doms, + virDomainDefPtr def, + unsigned int check_active) +{ + int ret = -1; + int dupVM = 0; + virDomainObjPtr vm = NULL; + + /* See if a VM with matching UUID already exists */ + vm = virDomainFindByUUID(doms, def->uuid); + if (vm) { + /* UUID matches, but if names don't match, refuse it */ + if (STRNEQ(vm->def->name, def->name)) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(vm->def->uuid, uuidstr); + virDomainReportError(NULL, VIR_ERR_OPERATION_FAILED, + _("domain '%s' is already defined with uuid %s"), + vm->def->name, uuidstr); + goto cleanup; + } + + if (check_active) { + /* UUID & name match, but if VM is already active, refuse it */ + if (virDomainObjIsActive(vm)) { + virDomainReportError(NULL, VIR_ERR_OPERATION_INVALID, + _("domain is already active as '%s'"), + vm->def->name); + goto cleanup; + } + } + + dupVM = 1; + virDomainObjUnlock(vm); + } else { + /* UUID does not match, but if a name matches, refuse it */ + vm = virDomainFindByName(doms, def->name); + if (vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(vm->def->uuid, uuidstr); + virDomainReportError(NULL, VIR_ERR_OPERATION_FAILED, + _("domain '%s' already exists with uuid %s"), + def->name, uuidstr); + goto cleanup; + } + } + + ret = dupVM; +cleanup: + if (vm) + virDomainObjUnlock(vm); + return ret; +} +
void virDomainObjLock(virDomainObjPtr obj) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8599ee7..9d3e9e2 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -788,6 +788,10 @@ virDomainFSDefPtr virDomainGetRootFilesystem(virDomainDefPtr def); int virDomainVideoDefaultType(virDomainDefPtr def); int virDomainVideoDefaultRAM(virDomainDefPtr def, int type);
+int virDomainObjIsDuplicate(virDomainObjListPtr doms, + virDomainDefPtr def, + unsigned int check_active); + void virDomainObjLock(virDomainObjPtr obj); void virDomainObjUnlock(virDomainObjPtr obj);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 15d75fd..451a883 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -146,6 +146,7 @@ virDomainObjLock; virDomainObjUnlock; virDomainStateTypeToString; virDomainStateTypeFromString; +virDomainObjIsDuplicate; virDomainObjListGetInactiveNames; virDomainObjListGetActiveIDs; virDomainObjListNumOfDomains; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 9ab94bf..4c237f2 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -273,41 +273,15 @@ static virDomainPtr lxcDomainDefine(virConnectPtr conn, const char *xml) virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; virDomainEventPtr event = NULL; - int newVM = 1; + int dupVM;
lxcDriverLock(driver); if (!(def = virDomainDefParseString(conn, driver->caps, xml, VIR_DOMAIN_XML_INACTIVE))) goto cleanup;
- /* See if a VM with matching UUID already exists */ - vm = virDomainFindByUUID(&driver->domains, def->uuid); - if (vm) { - /* UUID matches, but if names don't match, refuse it */ - if (STRNEQ(vm->def->name, def->name)) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(vm->def->uuid, uuidstr); - lxcError(conn, NULL, VIR_ERR_OPERATION_FAILED, - _("domain '%s' is already defined with uuid %s"), - vm->def->name, uuidstr); - goto cleanup; - } - - /* UUID & name match */ - virDomainObjUnlock(vm); - newVM = 0; - } else { - /* UUID does not match, but if a name matches, refuse it */ - vm = virDomainFindByName(&driver->domains, def->name); - if (vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(vm->def->uuid, uuidstr); - lxcError(conn, NULL, VIR_ERR_OPERATION_FAILED, - _("domain '%s' is already defined with uuid %s"), - def->name, uuidstr); - goto cleanup; - } - } + if ((dupVM = virDomainObjIsDuplicate(&driver->domains, def, 0)) < 0) + goto cleanup;
if ((def->nets != NULL) && !(driver->have_netns)) { lxcError(conn, NULL, VIR_ERR_NO_SUPPORT, @@ -331,7 +305,7 @@ static virDomainPtr lxcDomainDefine(virConnectPtr conn, const char *xml)
event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_DEFINED, - newVM ? + !dupVM ? VIR_DOMAIN_EVENT_DEFINED_ADDED : VIR_DOMAIN_EVENT_DEFINED_UPDATED);
@@ -1273,38 +1247,8 @@ lxcDomainCreateAndStart(virConnectPtr conn, VIR_DOMAIN_XML_INACTIVE))) goto cleanup;
- /* See if a VM with matching UUID already exists */ - vm = virDomainFindByUUID(&driver->domains, def->uuid); - if (vm) { - /* UUID matches, but if names don't match, refuse it */ - if (STRNEQ(vm->def->name, def->name)) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(vm->def->uuid, uuidstr); - lxcError(conn, NULL, VIR_ERR_OPERATION_FAILED, - _("domain '%s' is already defined with uuid %s"), - vm->def->name, uuidstr); - goto cleanup; - } - - /* UUID & name match, but if VM is already active, refuse it */ - if (virDomainObjIsActive(vm)) { - lxcError(conn, NULL, VIR_ERR_OPERATION_FAILED, - _("domain is already active as '%s'"), vm->def->name); - goto cleanup; - } - virDomainObjUnlock(vm); - } else { - /* UUID does not match, but if a name matches, refuse it */ - vm = virDomainFindByName(&driver->domains, def->name); - if (vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(vm->def->uuid, uuidstr); - lxcError(conn, NULL, VIR_ERR_OPERATION_FAILED, - _("domain '%s' is already defined with uuid %s"), - def->name, uuidstr); - goto cleanup; - } - } + if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) + goto cleanup;
if ((def->nets != NULL) && !(driver->have_netns)) { lxcError(conn, NULL, VIR_ERR_NO_SUPPORT, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5ff7e94..20621d1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2638,38 +2638,8 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, if (virSecurityDriverVerify(conn, def) < 0) goto cleanup;
- /* See if a VM with matching UUID already exists */ - vm = virDomainFindByUUID(&driver->domains, def->uuid); - if (vm) { - /* UUID matches, but if names don't match, refuse it */ - if (STRNEQ(vm->def->name, def->name)) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(vm->def->uuid, uuidstr); - qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain '%s' is already defined with uuid %s"), - vm->def->name, uuidstr); - goto cleanup; - } - - /* UUID & name match, but if VM is already active, refuse it */ - if (virDomainObjIsActive(vm)) { - qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain is already active as '%s'"), vm->def->name); - goto cleanup; - } - virDomainObjUnlock(vm); - } else { - /* UUID does not match, but if a name matches, refuse it */ - vm = virDomainFindByName(&driver->domains, def->name); - if (vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(vm->def->uuid, uuidstr); - qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain '%s' is already defined with uuid %s"), - def->name, uuidstr); - goto cleanup; - } - } + if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) + goto cleanup;
if (!(vm = virDomainAssignDef(conn, driver->caps, @@ -3699,38 +3669,8 @@ static int qemudDomainRestore(virConnectPtr conn, goto cleanup; }
- /* See if a VM with matching UUID already exists */ - vm = virDomainFindByUUID(&driver->domains, def->uuid); - if (vm) { - /* UUID matches, but if names don't match, refuse it */ - if (STRNEQ(vm->def->name, def->name)) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(vm->def->uuid, uuidstr); - qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain '%s' is already defined with uuid %s"), - vm->def->name, uuidstr); - goto cleanup; - } - - /* UUID & name match, but if VM is already active, refuse it */ - if (virDomainObjIsActive(vm)) { - qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_INVALID, - _("domain is already active as '%s'"), vm->def->name); - goto cleanup; - } - virDomainObjUnlock(vm); - } else { - /* UUID does not match, but if a name matches, refuse it */ - vm = virDomainFindByName(&driver->domains, def->name); - if (vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(vm->def->uuid, uuidstr); - qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain '%s' is already defined with uuid %s"), - def->name, uuidstr); - goto cleanup; - } - } + if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) + goto cleanup;
if (!(vm = virDomainAssignDef(conn, driver->caps, @@ -4177,7 +4117,7 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) { virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; virDomainEventPtr event = NULL; - int newVM = 1; + int dupVM;
qemuDriverLock(driver); if (!(def = virDomainDefParseString(conn, driver->caps, xml, @@ -4187,34 +4127,8 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) { if (virSecurityDriverVerify(conn, def) < 0) goto cleanup;
- /* See if a VM with matching UUID already exists */ - vm = virDomainFindByUUID(&driver->domains, def->uuid); - if (vm) { - /* UUID matches, but if names don't match, refuse it */ - if (STRNEQ(vm->def->name, def->name)) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(vm->def->uuid, uuidstr); - qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain '%s' is already defined with uuid %s"), - vm->def->name, uuidstr); - goto cleanup; - } - - /* UUID & name match */ - virDomainObjUnlock(vm); - newVM = 0; - } else { - /* UUID does not match, but if a name matches, refuse it */ - vm = virDomainFindByName(&driver->domains, def->name); - if (vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(vm->def->uuid, uuidstr); - qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain '%s' is already defined with uuid %s"), - def->name, uuidstr); - goto cleanup; - } - } + if ((dupVM = virDomainObjIsDuplicate(&driver->domains, def, 0)) < 0) + goto cleanup;
if (qemudCanonicalizeMachine(driver, def) < 0) goto cleanup; @@ -4239,7 +4153,7 @@ static virDomainPtr qemudDomainDefine(virConnectPtr conn, const char *xml) {
event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_DEFINED, - newVM ? + !dupVM ? VIR_DOMAIN_EVENT_DEFINED_ADDED : VIR_DOMAIN_EVENT_DEFINED_UPDATED);
diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 343834c..fb367c9 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1242,6 +1242,9 @@ testDomainCreateXML(virConnectPtr conn, const char *xml, VIR_DOMAIN_XML_INACTIVE)) == NULL) goto cleanup;
+ if (virDomainObjIsDuplicate(&privconn->domains, def, 1) < 0) + goto cleanup; + if (testDomainGenerateIfnames(conn, def) < 0) goto cleanup; if (!(dom = virDomainAssignDef(conn, privconn->caps, @@ -1785,6 +1788,9 @@ static int testDomainRestore(virConnectPtr conn, if (!def) goto cleanup;
+ if (virDomainObjIsDuplicate(&privconn->domains, def, 1) < 0) + goto cleanup; + if (testDomainGenerateIfnames(conn, def) < 0) goto cleanup; if (!(dom = virDomainAssignDef(conn, privconn->caps, @@ -2224,12 +2230,16 @@ static virDomainPtr testDomainDefineXML(virConnectPtr conn, virDomainDefPtr def; virDomainObjPtr dom = NULL; virDomainEventPtr event = NULL; + int dupVM;
testDriverLock(privconn); if ((def = virDomainDefParseString(conn, privconn->caps, xml, VIR_DOMAIN_XML_INACTIVE)) == NULL) goto cleanup;
+ if ((dupVM = virDomainObjIsDuplicate(&privconn->domains, def, 0)) < 0) + goto cleanup; + if (testDomainGenerateIfnames(conn, def) < 0) goto cleanup; if (!(dom = virDomainAssignDef(conn, privconn->caps, @@ -2240,7 +2250,9 @@ static virDomainPtr testDomainDefineXML(virConnectPtr conn,
event = virDomainEventNewFromObj(dom, VIR_DOMAIN_EVENT_DEFINED, - VIR_DOMAIN_EVENT_DEFINED_ADDED); + !dupVM ? + VIR_DOMAIN_EVENT_DEFINED_ADDED : + VIR_DOMAIN_EVENT_DEFINED_UPDATED);
ret = virGetDomain(conn, dom->def->name, dom->def->uuid); if (ret) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 212cd8f..0604742 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1178,23 +1178,8 @@ static virDomainPtr umlDomainCreate(virConnectPtr conn, const char *xml, VIR_DOMAIN_XML_INACTIVE))) goto cleanup;
- vm = virDomainFindByName(&driver->domains, def->name); - if (vm) { - umlReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain '%s' is already defined"), - def->name); + if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) goto cleanup; - } - vm = virDomainFindByUUID(&driver->domains, def->uuid); - if (vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - - virUUIDFormat(def->uuid, uuidstr); - umlReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain with uuid '%s' is already defined"), - uuidstr); - goto cleanup; - }
if (!(vm = virDomainAssignDef(conn, driver->caps, @@ -1531,6 +1516,9 @@ static virDomainPtr umlDomainDefine(virConnectPtr conn, const char *xml) { VIR_DOMAIN_XML_INACTIVE))) goto cleanup;
+ if (virDomainObjIsDuplicate(&driver->domains, def, 0) < 0) + goto cleanup; + if (!(vm = virDomainAssignDef(conn, driver->caps, &driver->domains,
ACK, very nice cleanup, plus that improves the test driver :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/qemu/qemu_driver.c | 30 ++++-------------------------- 1 files changed, 4 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 20621d1..53f7398 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5978,19 +5978,8 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn, /* Target domain name, maybe renamed. */ dname = dname ? dname : def->name; - /* Ensure the name and UUID don't already exist in an active VM */ - vm = virDomainFindByUUID(&driver->domains, def->uuid); - - if (!vm) vm = virDomainFindByName(&driver->domains, dname); - if (vm) { - if (virDomainObjIsActive(vm)) { - qemudReportError(dconn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain with the same name or UUID already exists as '%s'"), - vm->def->name); - goto cleanup; - } - virDomainObjUnlock(vm); - } + if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) + goto cleanup; if (!(vm = virDomainAssignDef(dconn, driver->caps, @@ -6202,19 +6191,8 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn, /* Target domain name, maybe renamed. */ dname = dname ? dname : def->name; - /* Ensure the name and UUID don't already exist in an active VM */ - vm = virDomainFindByUUID(&driver->domains, def->uuid); - - if (!vm) vm = virDomainFindByName(&driver->domains, dname); - if (vm) { - if (virDomainObjIsActive(vm)) { - qemudReportError (dconn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain with the same name or UUID already exists as '%s'"), - vm->def->name); - goto cleanup; - } - virDomainObjUnlock(vm); - } + if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) + goto cleanup; if (!(vm = virDomainAssignDef(dconn, driver->caps, -- 1.6.5.1
On Wed, Nov 04, 2009 at 03:06:59PM -0500, Cole Robinson wrote:
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/qemu/qemu_driver.c | 30 ++++-------------------------- 1 files changed, 4 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 20621d1..53f7398 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5978,19 +5978,8 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn, /* Target domain name, maybe renamed. */ dname = dname ? dname : def->name;
- /* Ensure the name and UUID don't already exist in an active VM */ - vm = virDomainFindByUUID(&driver->domains, def->uuid); - - if (!vm) vm = virDomainFindByName(&driver->domains, dname); - if (vm) { - if (virDomainObjIsActive(vm)) { - qemudReportError(dconn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain with the same name or UUID already exists as '%s'"), - vm->def->name); - goto cleanup; - } - virDomainObjUnlock(vm); - } + if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) + goto cleanup;
if (!(vm = virDomainAssignDef(dconn, driver->caps, @@ -6202,19 +6191,8 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn, /* Target domain name, maybe renamed. */ dname = dname ? dname : def->name;
- /* Ensure the name and UUID don't already exist in an active VM */ - vm = virDomainFindByUUID(&driver->domains, def->uuid); - - if (!vm) vm = virDomainFindByName(&driver->domains, dname); - if (vm) { - if (virDomainObjIsActive(vm)) { - qemudReportError (dconn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain with the same name or UUID already exists as '%s'"), - vm->def->name); - goto cleanup; - } - virDomainObjUnlock(vm); - } + if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) + goto cleanup;
if (!(vm = virDomainAssignDef(dconn, driver->caps,
Hum, there is a slight change of semantic in that case, if VM migrated from A to B, then gets renamed on B, if you try to migrate it back to A that will fail because the uuid match/name mismatch failure will be raised while this won't be the case with current code, right ? Maybe it's the right thing to do, but that's a change I think Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 11/06/2009 08:26 AM, Daniel Veillard wrote:
On Wed, Nov 04, 2009 at 03:06:59PM -0500, Cole Robinson wrote:
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/qemu/qemu_driver.c | 30 ++++-------------------------- 1 files changed, 4 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 20621d1..53f7398 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5978,19 +5978,8 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn, /* Target domain name, maybe renamed. */ dname = dname ? dname : def->name;
- /* Ensure the name and UUID don't already exist in an active VM */ - vm = virDomainFindByUUID(&driver->domains, def->uuid); - - if (!vm) vm = virDomainFindByName(&driver->domains, dname); - if (vm) { - if (virDomainObjIsActive(vm)) { - qemudReportError(dconn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain with the same name or UUID already exists as '%s'"), - vm->def->name); - goto cleanup; - } - virDomainObjUnlock(vm); - } + if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) + goto cleanup;
if (!(vm = virDomainAssignDef(dconn, driver->caps, @@ -6202,19 +6191,8 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn, /* Target domain name, maybe renamed. */ dname = dname ? dname : def->name;
- /* Ensure the name and UUID don't already exist in an active VM */ - vm = virDomainFindByUUID(&driver->domains, def->uuid); - - if (!vm) vm = virDomainFindByName(&driver->domains, dname); - if (vm) { - if (virDomainObjIsActive(vm)) { - qemudReportError (dconn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain with the same name or UUID already exists as '%s'"), - vm->def->name); - goto cleanup; - } - virDomainObjUnlock(vm); - } + if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) + goto cleanup;
if (!(vm = virDomainAssignDef(dconn, driver->caps,
Hum, there is a slight change of semantic in that case, if VM migrated from A to B, then gets renamed on B, if you try to migrate it back to A that will fail because the uuid match/name mismatch failure will be raised while this won't be the case with current code, right ? Maybe it's the right thing to do, but that's a change I think
Daniel
Yeah, that's why I had this as a separate patch, since it's a behavior change with no existing precedent. I think the change makes sense here, we are essentially invoking CreateXML on the new host, and this ensures migrate uses the same semantics as CreateXML. I've pushed these two patches now. - Cole
On Fri, Nov 06, 2009 at 02:26:04PM +0100, Daniel Veillard wrote:
On Wed, Nov 04, 2009 at 03:06:59PM -0500, Cole Robinson wrote:
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/qemu/qemu_driver.c | 30 ++++-------------------------- 1 files changed, 4 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 20621d1..53f7398 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5978,19 +5978,8 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn, /* Target domain name, maybe renamed. */ dname = dname ? dname : def->name;
- /* Ensure the name and UUID don't already exist in an active VM */ - vm = virDomainFindByUUID(&driver->domains, def->uuid); - - if (!vm) vm = virDomainFindByName(&driver->domains, dname); - if (vm) { - if (virDomainObjIsActive(vm)) { - qemudReportError(dconn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain with the same name or UUID already exists as '%s'"), - vm->def->name); - goto cleanup; - } - virDomainObjUnlock(vm); - } + if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) + goto cleanup;
if (!(vm = virDomainAssignDef(dconn, driver->caps, @@ -6202,19 +6191,8 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn, /* Target domain name, maybe renamed. */ dname = dname ? dname : def->name;
- /* Ensure the name and UUID don't already exist in an active VM */ - vm = virDomainFindByUUID(&driver->domains, def->uuid); - - if (!vm) vm = virDomainFindByName(&driver->domains, dname); - if (vm) { - if (virDomainObjIsActive(vm)) { - qemudReportError (dconn, NULL, NULL, VIR_ERR_OPERATION_FAILED, - _("domain with the same name or UUID already exists as '%s'"), - vm->def->name); - goto cleanup; - } - virDomainObjUnlock(vm); - } + if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) + goto cleanup;
if (!(vm = virDomainAssignDef(dconn, driver->caps,
Hum, there is a slight change of semantic in that case, if VM migrated from A to B, then gets renamed on B, if you try to migrate it back to A that will fail because the uuid match/name mismatch failure will be raised while this won't be the case with current code, right ? Maybe it's the right thing to do, but that's a change I think
Yes that is the correct behaviour - if the admin renames the guest on B, then they should either undefine its config on A, or rename it on A too. Daniel -- |: 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 :|
participants (3)
-
Cole Robinson -
Daniel P. Berrange -
Daniel Veillard