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);
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list