[libvirt] [PATCH 2/4] vz: Remove prlsdkAddDomain() and use new functions

[Nikolay Shirokovskiy]

You use 'and' in description like this patch have to distinct purposes. 
Looks like a candidate for splitting.

Say in first patch you use inroduced functions and thus fix a race. By
the way this could be done in one step with introducing the functions.

In second you refactor prlsdkHandleVmAddedEvent. But I don't like the
way you do it much as you get non-cleanup goto in the new version.

On 23.01.2016 11:42, Mikhail Feoktistov wrote:
> Use prlsdkNewDomain() and prlsdkSetDomainInstance() instead of prlsdkAddDomain()
> New algorithm for creating an instance:
> In vzDomainDefineXMLFlags() we add new domain to domain list by calling prlsdkNewDomain()
> and only after that we call CreateVm() to create VM.
> It means that we "reserve" domain object with the specific uuid.
> After creation of new VM we add info from this VM
> to reserved domain object by calling sdkSetDomainInstance().
> 
> In notification handler prlsdkHandleVmAddedEvent() we check
> the existence of a domain and if it doesn't exist we add new domain by calling
> prlsdkLoadDomain().
> ---
>  src/vz/vz_driver.c | 14 ++++++++++++--
>  src/vz/vz_sdk.c    | 38 +++++++++++++++-----------------------
>  src/vz/vz_sdk.h    |  2 --
>  3 files changed, 27 insertions(+), 27 deletions(-)
> 
> diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
> index f73f8ef..d1cf8d0 100644
> --- a/src/vz/vz_driver.c
> +++ b/src/vz/vz_driver.c
> @@ -687,6 +687,7 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags)
>      virDomainPtr retdom = NULL;
>      virDomainDefPtr def;
>      virDomainObjPtr olddom = NULL;
> +    virDomainObjPtr newdom = NULL;
>      unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE;
>  
>      virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL);
> @@ -702,6 +703,10 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags)
>      olddom = virDomainObjListFindByUUID(privconn->domains, def->uuid);
>      if (olddom == NULL) {
>          virResetLastError();
> +        newdom = prlsdkNewDomain(privconn, def->name, def->uuid);
> +        if (!newdom)
> +            goto cleanup;
> +
>          if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) {
>              if (prlsdkCreateVm(conn, def))
>                  goto cleanup;
> @@ -715,8 +720,7 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags)
>              goto cleanup;
>          }
>  
> -        olddom = prlsdkAddDomain(privconn, def->uuid);
> -        if (!olddom)
> +        if (prlsdkSetDomainInstance(privconn, newdom, def->uuid))
>              goto cleanup;
>      } else {
>          int state, reason;
> @@ -757,6 +761,12 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags)
>   cleanup:
>      if (olddom)
>          virObjectUnlock(olddom);
> +    if (newdom) {
> +        if (!retdom)
> +             virDomainObjListRemove(privconn->domains, newdom);
> +        else
> +             virObjectUnlock(newdom);
> +    }
>      virDomainDefFree(def);
>      vzDriverUnlock(privconn);
>      return retdom;
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index c705517..765f5f1 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -1473,27 +1473,6 @@ prlsdkLoadDomains(vzConnPtr privconn)
>      return -1;
>  }
>  
> -virDomainObjPtr
> -prlsdkAddDomain(vzConnPtr privconn, const unsigned char *uuid)
> -{
> -    PRL_HANDLE sdkdom = PRL_INVALID_HANDLE;
> -    virDomainObjPtr dom;
> -
> -    dom = virDomainObjListFindByUUID(privconn->domains, uuid);
> -    if (dom) {
> -        /* domain is already in the list */
> -        return dom;
> -    }
> -
> -    sdkdom = prlsdkSdkDomainLookupByUUID(privconn, uuid);
> -    if (sdkdom == PRL_INVALID_HANDLE)
> -        return NULL;
> -
> -    dom = prlsdkLoadDomain(privconn, sdkdom, NULL);
> -    PrlHandle_Free(sdkdom);
> -    return dom;
> -}
> -
>  int
>  prlsdkUpdateDomain(vzConnPtr privconn, virDomainObjPtr dom)
>  {
> @@ -1672,11 +1651,24 @@ prlsdkHandleVmAddedEvent(vzConnPtr privconn,
>                           unsigned char *uuid)
>  {
>      virDomainObjPtr dom = NULL;
> +    PRL_HANDLE sdkdom = PRL_INVALID_HANDLE;
>  
> -    dom = prlsdkAddDomain(privconn, uuid);
> -    if (dom == NULL)
> +    dom = virDomainObjListFindByUUID(privconn->domains, uuid);
> +    if (dom) {
> +        /* domain is already in the list */
> +        goto send_event;
> +    }
> +
> +    sdkdom = prlsdkSdkDomainLookupByUUID(privconn, uuid);
> +    if (sdkdom == PRL_INVALID_HANDLE)
> +        return;
> +
> +    dom = prlsdkLoadDomain(privconn, sdkdom, NULL);
> +    PrlHandle_Free(sdkdom);
> +    if (!dom)
>          return;
>  
> + send_event:
>      prlsdkSendEvent(privconn, dom, VIR_DOMAIN_EVENT_DEFINED,
>                      VIR_DOMAIN_EVENT_DEFINED_ADDED);
>  
> diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h
> index 060635e..19bce88 100644
> --- a/src/vz/vz_sdk.h
> +++ b/src/vz/vz_sdk.h
> @@ -31,8 +31,6 @@ void prlsdkDisconnect(vzConnPtr privconn);
>  int
>  prlsdkLoadDomains(vzConnPtr privconn);
>  virDomainObjPtr
> -prlsdkAddDomain(vzConnPtr privconn, const unsigned char *uuid);
> -virDomainObjPtr
>  prlsdkNewDomain(vzConnPtr privconn,
>                  char *name,
>                  const unsigned char *uuid);
>