Re: [PATCH 2/2] parallels: login to parallels SDK

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

On 21.08.2014 22:36, Dmitry Guryanov wrote:
> Add files parallels_sdk.c and parallels_sdk.h for code
> which works with SDK, so libvirt's code will not mix with
> dealing with parallels SDK.
>
> To use Parallels SDK you must first call PrlApi_InitEx function,
> and then you will be able to connect to a server with
> PrlSrv_LoginLocalEx function. When you've done you must call
> PrlApi_Deinit. So let's call PrlApi_InitEx on first .connectOpen,
> count number of connections and deinitialize, when this counter
> becomes zero.
>
> Signed-off-by: Dmitry Guryanov <dguryanov@xxxxxxxxxxxxx>
> ---
>   po/POTFILES.in                   |  1 +
>   src/Makefile.am                  |  4 +++-
>   src/parallels/parallels_driver.c | 46 +++++++++++++++++++++++++++++++++++++++-
>   src/parallels/parallels_utils.h  |  3 +++
>   4 files changed, 52 insertions(+), 2 deletions(-)

This fails 'make syntax-check' for me.

> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index f17b35f..4c1302d 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -96,6 +96,7 @@ src/openvz/openvz_driver.c
>   src/openvz/openvz_util.c
>   src/parallels/parallels_driver.c
>   src/parallels/parallels_network.c
> +src/parallels/parallels_sdk.c
>   src/parallels/parallels_utils.c
>   src/parallels/parallels_utils.h
>   src/parallels/parallels_storage.c
> diff --git a/src/Makefile.am b/src/Makefile.am
> index dad7c7f..d4c6465 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -787,7 +787,9 @@ PARALLELS_DRIVER_SOURCES =					\
>   		parallels/parallels_utils.c			\
>   		parallels/parallels_utils.h			\
>   		parallels/parallels_storage.c		\
> -		parallels/parallels_network.c
> +		parallels/parallels_network.c		\
> +		parallels/parallels_sdk.h			\
> +		parallels/parallels_sdk.c
>
>   BHYVE_DRIVER_SOURCES =						\
>   		bhyve/bhyve_capabilities.c			\
> diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
> index bb9538f..7dc9963 100644
> --- a/src/parallels/parallels_driver.c
> +++ b/src/parallels/parallels_driver.c
> @@ -55,6 +55,7 @@
>
>   #include "parallels_driver.h"
>   #include "parallels_utils.h"
> +#include "parallels_sdk.h"
>
>   #define VIR_FROM_THIS VIR_FROM_PARALLELS
>
> @@ -73,6 +74,9 @@ VIR_LOG_INIT("parallels.parallels_driver");
>
>   #define IS_CT(def)  (STREQ_NULLABLE(def->os.type, "exe"))
>
> +unsigned int numConns = 0;
> +virMutex numConnsLock;

1: ^^^

> +
>   static int parallelsConnectClose(virConnectPtr conn);
>
>   static const char * parallelsGetDiskBusName(int bus) {
> @@ -929,9 +933,25 @@ parallelsOpenDefault(virConnectPtr conn)
>       if (virMutexInit(&privconn->lock) < 0) {
>           virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                          _("cannot initialize mutex"));
> -        goto error;
> +        goto err_free;
>       }
>
> +    virMutexLock(&numConnsLock);
> +    numConns++;
> +
> +    if (numConns == 1) {
> +        if (prlsdkInit()) {
> +            VIR_DEBUG("%s", _("Can't initialize Parallels SDK"));
> +            virMutexUnlock(&numConnsLock);
> +            goto err_free;
> +        }
> +    }
> +
> +    virMutexUnlock(&numConnsLock);
> +
> +    if (prlsdkConnect(privconn) < 0)
> +        goto err_free;
> +
>       if (!(privconn->caps = parallelsBuildCapabilities()))
>           goto error;
>
> @@ -953,6 +973,9 @@ parallelsOpenDefault(virConnectPtr conn)
>       virObjectUnref(privconn->domains);
>       virObjectUnref(privconn->caps);
>       virStoragePoolObjListFree(&privconn->pools);
> +    prlsdkDisconnect(privconn);
> +    prlsdkDeinit();
> + err_free:
>       VIR_FREE(privconn);
>       return VIR_DRV_OPEN_ERROR;
>   }
> @@ -999,8 +1022,17 @@ parallelsConnectClose(virConnectPtr conn)
>       virObjectUnref(privconn->caps);
>       virObjectUnref(privconn->xmlopt);
>       virObjectUnref(privconn->domains);
> +    prlsdkDisconnect(privconn);
>       conn->privateData = NULL;
>
> +    virMutexLock(&numConnsLock);
> +    numConns--;
> +
> +    if (numConns == 0)
> +        prlsdkDeinit();
> +
> +    virMutexUnlock(&numConnsLock);
> +
>       parallelsDriverUnlock(privconn);
>       virMutexDestroy(&privconn->lock);
>
> @@ -2453,6 +2485,12 @@ static virDriver parallelsDriver = {
>       .connectIsAlive = parallelsConnectIsAlive, /* 1.2.5 */
>   };
>
> +static virStateDriver parallelsStateDriver = {
> +	.name = "parallels",
> +	.stateInitialize = parallelsStateInitialize,
> +	.stateCleanup = parallelsStateCleanup,
> +};
> +

This is not called (read registered) anywhere so the 
state{Initialize,Cleanup} members are not gonna be called at all.

Moreover, if you intent to turn this into state driver, then the [1] are 
good candidates to become members of some privateData struct of the 
state driver.

>   /**
>    * parallelsRegister:
>    *
> @@ -2471,6 +2509,12 @@ parallelsRegister(void)
>
>       VIR_FREE(prlctl_path);
>
> +    if (virMutexInit(&numConnsLock) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("cannot initialize mutex"));
> +        return 0;
> +    }
> +
>       if (virRegisterDriver(&parallelsDriver) < 0)
>           return -1;
>       if (parallelsStorageRegister())
> diff --git a/src/parallels/parallels_utils.h b/src/parallels/parallels_utils.h
> index 599e2c5..095c104 100644
> --- a/src/parallels/parallels_utils.h
> +++ b/src/parallels/parallels_utils.h
> @@ -23,6 +23,8 @@
>   #ifndef PARALLELS_UTILS_H
>   # define PARALLELS_UTILS_H
>
> +# include <Parallels.h>
> +
>   # include "driver.h"
>   # include "conf/domain_conf.h"
>   # include "conf/storage_conf.h"
> @@ -40,6 +42,7 @@
>   struct _parallelsConn {
>       virMutex lock;
>       virDomainObjListPtr domains;
> +    PRL_HANDLE server;
>       virStoragePoolObjList pools;
>       virNetworkObjList networks;
>       virCapsPtr caps;

Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list