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

On 22.08.2014 19:48, Dmitry Guryanov wrote: > Add files parallels_sdk.c and parallels_sdk.h for code

> > @@ -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(); This calls init & deinit several times, for instance, if there's just a single connection that disconnects eventually. Is it possible to call prlsdkInit() in parallelsRegister() and then call prlsdkDeinit() in parallelsUnregister()? I know the latter function doesn't exist yet, which brings up even more interesting question - is Deinit really needed or is it here just for the completeness' sake? Because if it is so, we can drop the numConnsLock mutex :)

> + > + virMutexUnlock(&numConnsLock); > + > > parallelsDriverUnlock(privconn); > virMutexDestroy(&privconn->lock); > > @@ -2483,6 +2515,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_sdk.c b/src/parallels/parallels_sdk.c > new file mode 100644 > index 0000000..bedf32d > --- /dev/null > +++ b/src/parallels/parallels_sdk.c > @@ -0,0 +1,234 @@ > +/* > + * parallels_sdk.c: core driver functions for managing > + * Parallels Cloud Server hosts > + * > + * Copyright (C) 2014 Parallels, Inc. > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library. If not, see > + * <http://www.gnu.org/licenses/>;. > + * > + */ > + > +#include <config.h> > + > +#include "virerror.h" > +#include "viralloc.h" > + > +#include "parallels_sdk.h" > + > +#define VIR_FROM_THIS VIR_FROM_PARALLELS > +#define JOB_INFINIT_WAIT_TIMEOUT UINT_MAX > + > +PRL_UINT32 defaultJobTimeout = JOB_INFINIT_WAIT_TIMEOUT; > + > +/* > + * Log error description > + */ > +void logPrlErrorHelper(PRL_RESULT err, const char *filename, > + const char *funcname, size_t linenr) > +{ > + char *msg1, *msg2; These two ^^ will have random value once the control enters the function... > + PRL_UINT32 len = 0; > + > + /* Get required buffer length */ > + PrlApi_GetResultDescription(err, PRL_TRUE, PRL_FALSE, NULL, &len); > + > + if (VIR_ALLOC_N(msg1, len) < 0) > + goto out; And imagine, VIR_ALLOC() fails ... > + > + /* get short error description */ > + PrlApi_GetResultDescription(err, PRL_TRUE, PRL_FALSE, msg1, &len); > + > + PrlApi_GetResultDescription(err, PRL_FALSE, PRL_FALSE, NULL, &len); > + > + if (VIR_ALLOC_N(msg2, len) < 0) > + goto out; > + > + /* get long error description */ > + PrlApi_GetResultDescription(err, PRL_FALSE, PRL_FALSE, msg2, &len); > + > + virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INTERNAL_ERROR, > + filename, funcname, linenr, > + _("Parallels SDK: %s %s"), msg1, msg2);