[libvirt] [PATCH 3/4] vz: remove code duplication from LoadDomain()

[Nikolay Shirokovskiy]

prlsdkLoadDomain works in 2 different cases:

1. Called first time for a domain. Then it creates and
initializes it. Then updates it from sdk handle.
2. Called when domain is already in list. In this case it updates domain from sdk handle.

I think we could end up in a better series if we split
this function into 2: first is to create and initialize, 
second update domain from sdk handle (load).

On 23.01.2016 11:42, Mikhail Feoktistov wrote:
> Now we create new domain by calling prlsdkNewDomain().
> In LoadDomain() we just load info from vm instance to domain object.
> So remove code to create domain from LoadDomain().
> 
> ---
>  src/vz/vz_sdk.c | 55 +++++++++++++++++--------------------------------------
>  1 file changed, 17 insertions(+), 38 deletions(-)
> 
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index 765f5f1..d9f2127 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -1260,26 +1260,29 @@ prlsdkLoadDomain(vzConnPtr privconn,
>      PRL_UINT32 ram;
>      PRL_UINT32 envId;
>      PRL_VM_AUTOSTART_OPTION autostart;
> +    unsigned char uuid[VIR_UUID_BUFLEN];
> +    char *name;
>  
>      virCheckNonNullArgGoto(privconn, error);
>      virCheckNonNullArgGoto(sdkdom, error);
>  
> -    if (!(def = virDomainDefNew()))
> +    if (prlsdkGetDomainIds(sdkdom, &name, uuid) < 0)
>          goto error;
>  
>      if (!olddom) {
> -        if (VIR_ALLOC(pdom) < 0)
> -            goto error;
> +        dom = prlsdkNewDomain(privconn, name, uuid);
> +        def = dom->def;
> +        pdom = dom->privateData;
>      } else {
> +        if (!(def = virDomainDefNewFull(name, uuid, -1)))
> +            goto error;
>          pdom = olddom->privateData;
> +        if (STREQ(privconn->drivername, "vz"))
> +            def->virtType = VIR_DOMAIN_VIRT_VZ;
> +        else
> +            def->virtType = VIR_DOMAIN_VIRT_PARALLELS;
>      }
> -
> -    if (STREQ(privconn->drivername, "vz"))
> -        def->virtType = VIR_DOMAIN_VIRT_VZ;
> -    else
> -        def->virtType = VIR_DOMAIN_VIRT_PARALLELS;
> -
> -    def->id = -1;
> +    VIR_FREE(name);
>  
>      /* we will remove this field in the near future, so let's set it
>       * to NULL temporarily */
> @@ -1292,9 +1295,6 @@ prlsdkLoadDomain(vzConnPtr privconn,
>          goto error;
>      }
>  
> -    if (prlsdkGetDomainIds(sdkdom, &def->name, def->uuid) < 0)
> -        goto error;
> -
>      def->onReboot = VIR_DOMAIN_LIFECYCLE_RESTART;
>      def->onPoweroff = VIR_DOMAIN_LIFECYCLE_DESTROY;
>      def->onCrash = VIR_DOMAIN_LIFECYCLE_CRASH_DESTROY;
> @@ -1369,23 +1369,10 @@ prlsdkLoadDomain(vzConnPtr privconn,
>      }
>  
>      if (olddom) {
> -        /* assign new virDomainDef without any checks */
> -        /* we can't use virDomainObjAssignDef, because it checks
> -         * for state and domain name */
>          dom = olddom;
>          virDomainDefFree(dom->def);
>          dom->def = def;
> -    } else {
> -        if (!(dom = virDomainObjListAdd(privconn->domains, def,
> -                                        privconn->xmlopt,
> -                                        0, NULL)))
> -            goto error;
>      }
> -    /* dom is locked here */
> -
> -    dom->privateData = pdom;
> -    dom->privateDataFreeFunc = prlsdkDomObjFreePrivate;
> -    dom->persistent = 1;
>  
>      switch (autostart) {
>      case PAO_VM_START_ON_LOAD:
> @@ -1411,19 +1398,11 @@ prlsdkLoadDomain(vzConnPtr privconn,
>  
>      return dom;
>   error:
> -    if (dom && !olddom) {
> -        /* Domain isn't persistent means that we haven't yet set
> -         * prlsdkDomObjFreePrivate and should call it manually
> -         */
> -        if (!dom->persistent)
> -            prlsdkDomObjFreePrivate(pdom);
> -
> +    VIR_FREE(name);
> +    if (!olddom && dom)
>          virDomainObjListRemove(privconn->domains, dom);
> -    }
> -    /* Delete newly allocated def only if we haven't assigned it to domain
> -     * Otherwise we will end up with domain having invalid def within it
> -     */
> -    if (!dom)
> +
> +    if (olddom && !dom)
>          virDomainDefFree(def);
>  
>      return NULL;
>