[libvirt] [PATCH] parallels: remove connection wide wait timeout
Dmitry Guryanov
dguryanov at parallels.com
Wed May 13 11:46:59 UTC 2015
On 05/07/2015 03:20 PM, Nikolay Shirokovskiy wrote:
> We have a lot of passing arguments code just to pass connection
> object cause it holds jobTimeout. Taking into account that
> right now this value is defined at compile time let's just
> get rid of it and make arguments list more clear in many
> places.
>
> In case we later need some runtime configurable timeout
> value we can provide this value through arguments
> function already operate such as a parallels domain
> object etc as this timeouts are operation( and thus
> object) specific in practice.
I agree with this patch, at this point hardcoded timeout value has no
sense. But could you, please, rebase the patch and fix
parallelsDomainDetachDeviceFlags?
>
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at parallels.com>
> ---
> src/parallels/parallels_driver.c | 6 +-
> src/parallels/parallels_sdk.c | 98 +++++++++++++++++---------------------
> src/parallels/parallels_sdk.h | 22 ++++-----
> src/parallels/parallels_utils.h | 1 -
> 4 files changed, 57 insertions(+), 70 deletions(-)
>
> diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
> index 3aa87ca..63bf638 100644
> --- a/src/parallels/parallels_driver.c
> +++ b/src/parallels/parallels_driver.c
> @@ -214,7 +214,7 @@ parallelsOpenDefault(virConnectPtr conn)
> goto err_free;
> }
>
> - if (prlsdkInit(privconn)) {
> + if (prlsdkInit()) {
> VIR_DEBUG("%s", _("Can't initialize Parallels SDK"));
> goto err_free;
> }
> @@ -1087,7 +1087,7 @@ parallelsDomainManagedSaveRemove(virDomainPtr domain, unsigned int flags)
> if (!(state == VIR_DOMAIN_SHUTOFF && reason == VIR_DOMAIN_SHUTOFF_SAVED))
> goto cleanup;
>
> - ret = prlsdkDomainManagedSaveRemove(privconn, dom);
> + ret = prlsdkDomainManagedSaveRemove(dom);
>
> cleanup:
> virObjectUnlock(dom);
> @@ -1139,7 +1139,7 @@ static int parallelsDomainAttachDeviceFlags(virDomainPtr dom, const char *xml,
>
> switch (dev->type) {
> case VIR_DOMAIN_DEVICE_DISK:
> - ret = prlsdkAttachVolume(dom->conn, privdom, dev->data.disk);
> + ret = prlsdkAttachVolume(privdom, dev->data.disk);
> if (ret) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("disk attach failed"));
> diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
> index e1ba7cf..83de1de 100644
> --- a/src/parallels/parallels_sdk.c
> +++ b/src/parallels/parallels_sdk.c
> @@ -35,8 +35,6 @@
> #define VIR_FROM_THIS VIR_FROM_PARALLELS
> #define JOB_INFINIT_WAIT_TIMEOUT UINT_MAX
>
> -PRL_UINT32 defaultJobTimeout = JOB_INFINIT_WAIT_TIMEOUT;
> -
> VIR_LOG_INIT("parallels.sdk");
>
> /*
> @@ -179,9 +177,9 @@ getJobResultHelper(PRL_HANDLE job, unsigned int timeout, PRL_HANDLE *result,
> return ret;
> }
>
> -#define getJobResult(job, timeout, result) \
> - getJobResultHelper(job, timeout, result, __FILE__, \
> - __FUNCTION__, __LINE__)
> +#define getJobResult(job, result) \
> + getJobResultHelper(job, JOB_INFINIT_WAIT_TIMEOUT, \
> + result, __FILE__, __FUNCTION__, __LINE__)
>
> static PRL_RESULT
> waitJobHelper(PRL_HANDLE job, unsigned int timeout,
> @@ -197,12 +195,13 @@ waitJobHelper(PRL_HANDLE job, unsigned int timeout,
> return ret;
> }
>
> -#define waitJob(job, timeout) \
> - waitJobHelper(job, timeout, __FILE__, \
> +#define waitJob(job) \
> + waitJobHelper(job, JOB_INFINIT_WAIT_TIMEOUT, __FILE__, \
> __FUNCTION__, __LINE__)
>
> +
> int
> -prlsdkInit(parallelsConnPtr privconn)
> +prlsdkInit(void)
> {
> PRL_RESULT ret;
>
> @@ -212,8 +211,6 @@ prlsdkInit(parallelsConnPtr privconn)
> return -1;
> }
>
> - privconn->jobTimeout = JOB_INFINIT_WAIT_TIMEOUT;
> -
> return 0;
> };
>
> @@ -238,7 +235,7 @@ prlsdkConnect(parallelsConnPtr privconn)
> job = PrlSrv_LoginLocalEx(privconn->server, NULL, 0,
> PSL_HIGH_SECURITY, PACF_NON_INTERACTIVE_MODE);
>
> - if (waitJob(job, privconn->jobTimeout)) {
> + if (waitJob(job)) {
> PrlHandle_Free(privconn->server);
> return -1;
> }
> @@ -252,7 +249,7 @@ prlsdkDisconnect(parallelsConnPtr privconn)
> PRL_HANDLE job;
>
> job = PrlSrv_Logoff(privconn->server);
> - waitJob(job, privconn->jobTimeout);
> + waitJob(job);
>
> PrlHandle_Free(privconn->server);
> }
> @@ -269,7 +266,7 @@ prlsdkSdkDomainLookup(parallelsConnPtr privconn,
> int ret = -1;
>
> job = PrlSrv_GetVmConfig(privconn->server, id, flags);
> - if (PRL_FAILED(getJobResult(job, privconn->jobTimeout, &result)))
> + if (PRL_FAILED(getJobResult(job, &result)))
> goto cleanup;
>
> pret = PrlResult_GetParamByIndex(result, 0, sdkdom);
> @@ -374,9 +371,7 @@ prlsdkGetDomainIds(PRL_HANDLE sdkdom,
> }
>
> static int
> -prlsdkGetDomainState(parallelsConnPtr privconn,
> - PRL_HANDLE sdkdom,
> - VIRTUAL_MACHINE_STATE_PTR vmState)
> +prlsdkGetDomainState(PRL_HANDLE sdkdom, VIRTUAL_MACHINE_STATE_PTR vmState)
> {
> PRL_HANDLE job = PRL_INVALID_HANDLE;
> PRL_HANDLE result = PRL_INVALID_HANDLE;
> @@ -386,7 +381,7 @@ prlsdkGetDomainState(parallelsConnPtr privconn,
>
> job = PrlVm_GetState(sdkdom);
>
> - if (PRL_FAILED(getJobResult(job, privconn->jobTimeout, &result)))
> + if (PRL_FAILED(getJobResult(job, &result)))
> goto cleanup;
>
> pret = PrlResult_GetParamByIndex(result, 0, &vmInfo);
> @@ -1356,7 +1351,7 @@ prlsdkLoadDomain(parallelsConnPtr privconn,
> dom->privateDataFreeFunc = prlsdkDomObjFreePrivate;
> dom->persistent = 1;
>
> - if (prlsdkGetDomainState(privconn, sdkdom, &domainState) < 0)
> + if (prlsdkGetDomainState(sdkdom, &domainState) < 0)
> goto error;
>
> if (prlsdkConvertDomainState(domainState, envId, dom) < 0)
> @@ -1406,7 +1401,7 @@ prlsdkLoadDomains(parallelsConnPtr privconn)
>
> job = PrlSrv_GetVmListEx(privconn->server, PVTF_VM | PVTF_CT);
>
> - if (PRL_FAILED(getJobResult(job, privconn->jobTimeout, &result)))
> + if (PRL_FAILED(getJobResult(job, &result)))
> return -1;
>
> pret = PrlResult_GetParamsCount(result, ¶msCount);
> @@ -1466,7 +1461,7 @@ prlsdkUpdateDomain(parallelsConnPtr privconn, virDomainObjPtr dom)
> parallelsDomObjPtr pdom = dom->privateData;
>
> job = PrlVm_RefreshConfig(pdom->sdkdom);
> - if (waitJob(job, privconn->jobTimeout))
> + if (waitJob(job))
> return -1;
>
> retdom = prlsdkLoadDomain(privconn, pdom->sdkdom, dom);
> @@ -1750,56 +1745,54 @@ void prlsdkUnsubscribeFromPCSEvents(parallelsConnPtr privconn)
> logPrlError(ret);
> }
>
> -PRL_RESULT prlsdkStart(parallelsConnPtr privconn, PRL_HANDLE sdkdom)
> +PRL_RESULT prlsdkStart(PRL_HANDLE sdkdom)
> {
> PRL_HANDLE job = PRL_INVALID_HANDLE;
>
> job = PrlVm_StartEx(sdkdom, PSM_VM_START, 0);
> - return PRL_FAILED(waitJob(job, privconn->jobTimeout)) ? -1 : 0;
> + return waitJob(job);
> }
>
> -static PRL_RESULT prlsdkStopEx(parallelsConnPtr privconn,
> - PRL_HANDLE sdkdom,
> - PRL_UINT32 mode)
> +static PRL_RESULT prlsdkStopEx(PRL_HANDLE sdkdom, PRL_UINT32 mode)
> {
> PRL_HANDLE job = PRL_INVALID_HANDLE;
>
> job = PrlVm_StopEx(sdkdom, mode, 0);
> - return waitJob(job, privconn->jobTimeout);
> + return waitJob(job);
> }
>
> -PRL_RESULT prlsdkKill(parallelsConnPtr privconn, PRL_HANDLE sdkdom)
> +PRL_RESULT prlsdkKill(PRL_HANDLE sdkdom)
> {
> - return prlsdkStopEx(privconn, sdkdom, PSM_KILL);
> + return prlsdkStopEx(sdkdom, PSM_KILL);
> }
>
> -PRL_RESULT prlsdkStop(parallelsConnPtr privconn, PRL_HANDLE sdkdom)
> +PRL_RESULT prlsdkStop(PRL_HANDLE sdkdom)
> {
> - return prlsdkStopEx(privconn, sdkdom, PSM_SHUTDOWN);
> + return prlsdkStopEx(sdkdom, PSM_SHUTDOWN);
> }
>
> -PRL_RESULT prlsdkPause(parallelsConnPtr privconn, PRL_HANDLE sdkdom)
> +PRL_RESULT prlsdkPause(PRL_HANDLE sdkdom)
> {
> PRL_HANDLE job = PRL_INVALID_HANDLE;
>
> job = PrlVm_Pause(sdkdom, false);
> - return waitJob(job, privconn->jobTimeout);
> + return waitJob(job);
> }
>
> -PRL_RESULT prlsdkResume(parallelsConnPtr privconn, PRL_HANDLE sdkdom)
> +PRL_RESULT prlsdkResume(PRL_HANDLE sdkdom)
> {
> PRL_HANDLE job = PRL_INVALID_HANDLE;
>
> job = PrlVm_Resume(sdkdom);
> - return waitJob(job, privconn->jobTimeout);
> + return waitJob(job);
> }
>
> -PRL_RESULT prlsdkSuspend(parallelsConnPtr privconn, PRL_HANDLE sdkdom)
> +PRL_RESULT prlsdkSuspend(PRL_HANDLE sdkdom)
> {
> PRL_HANDLE job = PRL_INVALID_HANDLE;
>
> job = PrlVm_Suspend(sdkdom);
> - return waitJob(job, privconn->jobTimeout);
> + return waitJob(job);
> }
>
> int
> @@ -1812,7 +1805,7 @@ prlsdkDomainChangeStateLocked(parallelsConnPtr privconn,
> virErrorNumber virerr;
>
> pdom = dom->privateData;
> - pret = chstate(privconn, pdom->sdkdom);
> + pret = chstate(pdom->sdkdom);
> if (PRL_FAILED(pret)) {
> virResetLastError();
>
> @@ -2803,7 +2796,7 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom,
> prlsdkCheckRetGoto(pret, cleanup);
>
> job = PrlSrv_AddVirtualNetwork(privconn->server, vnet, 0);
> - if (PRL_FAILED(pret = waitJob(job, privconn->jobTimeout)))
> + if (PRL_FAILED(pret = waitJob(job)))
> goto cleanup;
>
> pret = PrlVmDev_SetEmulatedType(sdknet, PNA_BRIDGED_ETHERNET);
> @@ -2842,7 +2835,7 @@ static void prlsdkDelNet(parallelsConnPtr privconn, virDomainNetDefPtr net)
> prlsdkCheckRetGoto(pret, cleanup);
>
> PrlSrv_DeleteVirtualNetwork(privconn->server, vnet, 0);
> - if (PRL_FAILED(pret = waitJob(job, privconn->jobTimeout)))
> + if (PRL_FAILED(pret = waitJob(job)))
> goto cleanup;
>
> cleanup:
> @@ -3020,23 +3013,20 @@ static int prlsdkAddDisk(PRL_HANDLE sdkdom, virDomainDiskDefPtr disk, bool bootD
> }
>
> int
> -prlsdkAttachVolume(virConnectPtr conn,
> - virDomainObjPtr dom,
> - virDomainDiskDefPtr disk)
> +prlsdkAttachVolume(virDomainObjPtr dom, virDomainDiskDefPtr disk)
> {
> int ret = -1;
> - parallelsConnPtr privconn = conn->privateData;
> parallelsDomObjPtr privdom = dom->privateData;
> PRL_HANDLE job = PRL_INVALID_HANDLE;
>
> job = PrlVm_BeginEdit(privdom->sdkdom);
> - if (PRL_FAILED(waitJob(job, privconn->jobTimeout)))
> + if (PRL_FAILED(waitJob(job)))
> goto cleanup;
>
> ret = prlsdkAddDisk(privdom->sdkdom, disk, false);
> if (ret == 0) {
> job = PrlVm_CommitEx(privdom->sdkdom, PVCF_DETACH_HDD_BUNDLE);
> - if (PRL_FAILED(waitJob(job, privconn->jobTimeout))) {
> + if (PRL_FAILED(waitJob(job))) {
> ret = -1;
> goto cleanup;
> }
> @@ -3217,14 +3207,14 @@ prlsdkApplyConfig(virConnectPtr conn,
> return -1;
>
> job = PrlVm_BeginEdit(sdkdom);
> - if (PRL_FAILED(waitJob(job, privconn->jobTimeout)))
> + if (PRL_FAILED(waitJob(job)))
> return -1;
>
> ret = prlsdkDoApplyConfig(conn, sdkdom, new, dom->def);
>
> if (ret == 0) {
> job = PrlVm_CommitEx(sdkdom, PVCF_DETACH_HDD_BUNDLE);
> - if (PRL_FAILED(waitJob(job, privconn->jobTimeout)))
> + if (PRL_FAILED(waitJob(job)))
> ret = -1;
> }
>
> @@ -3248,7 +3238,7 @@ prlsdkCreateVm(virConnectPtr conn, virDomainDefPtr def)
> prlsdkCheckRetGoto(pret, cleanup);
>
> job = PrlSrv_GetSrvConfig(privconn->server);
> - if (PRL_FAILED(getJobResult(job, privconn->jobTimeout, &result)))
> + if (PRL_FAILED(getJobResult(job, &result)))
> goto cleanup;
>
> pret = PrlResult_GetParamByIndex(result, 0, &srvconf);
> @@ -3265,7 +3255,7 @@ prlsdkCreateVm(virConnectPtr conn, virDomainDefPtr def)
> goto cleanup;
>
> job = PrlVm_Reg(sdkdom, "", 1);
> - if (PRL_FAILED(waitJob(job, privconn->jobTimeout)))
> + if (PRL_FAILED(waitJob(job)))
> ret = -1;
>
> cleanup:
> @@ -3310,7 +3300,7 @@ prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def)
> confParam.nOsVersion = 0;
>
> job = PrlSrv_GetDefaultVmConfig(privconn->server, &confParam, 0);
> - if (PRL_FAILED(getJobResult(job, privconn->jobTimeout, &result)))
> + if (PRL_FAILED(getJobResult(job, &result)))
> goto cleanup;
>
> pret = PrlResult_GetParamByIndex(result, 0, &sdkdom);
> @@ -3328,7 +3318,7 @@ prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def)
>
> job = PrlVm_RegEx(sdkdom, "",
> PACF_NON_INTERACTIVE_MODE | PRNVM_PRESERVE_DISK);
> - if (PRL_FAILED(waitJob(job, privconn->jobTimeout)))
> + if (PRL_FAILED(waitJob(job)))
> ret = -1;
>
> cleanup:
> @@ -3347,7 +3337,7 @@ prlsdkUnregisterDomain(parallelsConnPtr privconn, virDomainObjPtr dom)
> prlsdkDelNet(privconn, dom->def->nets[i]);
>
> job = PrlVm_Unreg(privdom->sdkdom);
> - if (PRL_FAILED(waitJob(job, privconn->jobTimeout)))
> + if (PRL_FAILED(waitJob(job)))
> return -1;
>
> if (prlsdkSendEvent(privconn, dom, VIR_DOMAIN_EVENT_UNDEFINED,
> @@ -3359,13 +3349,13 @@ prlsdkUnregisterDomain(parallelsConnPtr privconn, virDomainObjPtr dom)
> }
>
> int
> -prlsdkDomainManagedSaveRemove(parallelsConnPtr privconn, virDomainObjPtr dom)
> +prlsdkDomainManagedSaveRemove(virDomainObjPtr dom)
> {
> parallelsDomObjPtr privdom = dom->privateData;
> PRL_HANDLE job;
>
> job = PrlVm_DropSuspendedState(privdom->sdkdom);
> - if (PRL_FAILED(waitJob(job, privconn->jobTimeout)))
> + if (PRL_FAILED(waitJob(job)))
> return -1;
>
> return 0;
> diff --git a/src/parallels/parallels_sdk.h b/src/parallels/parallels_sdk.h
> index 00fa44d..027dd0c 100644
> --- a/src/parallels/parallels_sdk.h
> +++ b/src/parallels/parallels_sdk.h
> @@ -24,7 +24,7 @@
>
> #include "parallels_utils.h"
>
> -int prlsdkInit(parallelsConnPtr privconn);
> +int prlsdkInit(void);
> void prlsdkDeinit(void);
> int prlsdkConnect(parallelsConnPtr privconn);
> void prlsdkDisconnect(parallelsConnPtr privconn);
> @@ -35,14 +35,14 @@ prlsdkAddDomain(parallelsConnPtr privconn, const unsigned char *uuid);
> int prlsdkUpdateDomain(parallelsConnPtr privconn, virDomainObjPtr dom);
> int prlsdkSubscribeToPCSEvents(parallelsConnPtr privconn);
> void prlsdkUnsubscribeFromPCSEvents(parallelsConnPtr privconn);
> -PRL_RESULT prlsdkStart(parallelsConnPtr privconn, PRL_HANDLE sdkdom);
> -PRL_RESULT prlsdkKill(parallelsConnPtr privconn, PRL_HANDLE sdkdom);
> -PRL_RESULT prlsdkStop(parallelsConnPtr privconn, PRL_HANDLE sdkdom);
> -PRL_RESULT prlsdkPause(parallelsConnPtr privconn, PRL_HANDLE sdkdom);
> -PRL_RESULT prlsdkResume(parallelsConnPtr privconn, PRL_HANDLE sdkdom);
> -PRL_RESULT prlsdkSuspend(parallelsConnPtr privconn, PRL_HANDLE sdkdom);
> +PRL_RESULT prlsdkStart(PRL_HANDLE sdkdom);
> +PRL_RESULT prlsdkKill(PRL_HANDLE sdkdom);
> +PRL_RESULT prlsdkStop(PRL_HANDLE sdkdom);
> +PRL_RESULT prlsdkPause(PRL_HANDLE sdkdom);
> +PRL_RESULT prlsdkResume(PRL_HANDLE sdkdom);
> +PRL_RESULT prlsdkSuspend(PRL_HANDLE sdkdom);
>
> -typedef PRL_RESULT (*prlsdkChangeStateFunc)(parallelsConnPtr privconn, PRL_HANDLE sdkdom);
> +typedef PRL_RESULT (*prlsdkChangeStateFunc)(PRL_HANDLE sdkdom);
> int
> prlsdkDomainChangeState(virDomainPtr domain,
> prlsdkChangeStateFunc chstate);
> @@ -59,8 +59,6 @@ int prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def);
> int
> prlsdkUnregisterDomain(parallelsConnPtr privconn, virDomainObjPtr dom);
> int
> -prlsdkDomainManagedSaveRemove(parallelsConnPtr privconn, virDomainObjPtr dom);
> +prlsdkDomainManagedSaveRemove(virDomainObjPtr dom);
> int
> -prlsdkAttachVolume(virConnectPtr conn,
> - virDomainObjPtr dom,
> - virDomainDiskDefPtr disk);
> +prlsdkAttachVolume(virDomainObjPtr dom, virDomainDiskDefPtr disk);
> diff --git a/src/parallels/parallels_utils.h b/src/parallels/parallels_utils.h
> index 0f29374..912f450 100644
> --- a/src/parallels/parallels_utils.h
> +++ b/src/parallels/parallels_utils.h
> @@ -59,7 +59,6 @@ struct _parallelsConn {
> virMutex lock;
> virDomainObjListPtr domains;
> PRL_HANDLE server;
> - PRL_UINT32 jobTimeout;
> virStoragePoolObjList pools;
> virNetworkObjListPtr networks;
> virCapsPtr caps;
More information about the libvir-list
mailing list