[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/2] qemu: Break out function to check if we can create/define/restore



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/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]