[libvirt] [PATCH 1/9] vz: expand start/stop/... APIs for ACL checks
Maxim Nestratov
mnestratov at virtuozzo.com
Wed Aug 17 20:48:20 UTC 2016
24-Jun-16 17:32, Nikolay Shirokovskiy пишет:
> The original motivation is to expand API calls like start/stop etc so that
> the ACL checks could be added. But this patch has its own befenits.
>
> 1. functions like prlsdkStart/Stop use common routine to wait for
> job without domain lock. They become more self contained and do
> not return intermediate PRL_RESULT.
>
> 2. vzDomainManagedSave do not update cache twice.
>
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
> ---
> src/vz/vz_driver.c | 216 ++++++++++++++++++++++++++++++++++++++++++++++++-----
> src/vz/vz_sdk.c | 172 +++++++++++++++++++++---------------------
> src/vz/vz_sdk.h | 23 ++----
> 3 files changed, 291 insertions(+), 120 deletions(-)
>
> diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
> index faa1f56..0079384 100644
> --- a/src/vz/vz_driver.c
> +++ b/src/vz/vz_driver.c
> @@ -957,36 +957,210 @@ vzConnectDomainEventDeregisterAny(virConnectPtr conn,
> return 0;
> }
>
> -static int vzDomainSuspend(virDomainPtr domain)
> +static int
> +vzDomainSuspend(virDomainPtr domain)
> {
> - return prlsdkDomainChangeState(domain, prlsdkPause);
> + vzConnPtr privconn = domain->conn->privateData;
> + virDomainObjPtr dom;
> + int ret = -1;
> + bool job = false;
> +
> + if (!(dom = vzDomObjFromDomainRef(domain)))
> + return -1;
> +
> + if (vzDomainObjBeginJob(dom) < 0)
> + goto cleanup;
> + job = true;
> +
> + if (!vzDomainObjIsExist(dom))
> + goto cleanup;
> +
s/vzDomainObjIsExist/vzEnsureDomainExists
> + if (prlsdkPause(dom) < 0)
> + goto cleanup;
> +
> + if (prlsdkUpdateDomain(privconn->driver, dom) < 0)
> + goto cleanup;
> +
> + ret = 0;
> +
> + cleanup:
> + if (job)
> + vzDomainObjEndJob(dom);
> + virDomainObjEndAPI(&dom);
> +
> + return ret;
> }
>
> -static int vzDomainResume(virDomainPtr domain)
> +static int
> +vzDomainResume(virDomainPtr domain)
> {
> - return prlsdkDomainChangeState(domain, prlsdkResume);
> + vzConnPtr privconn = domain->conn->privateData;
> + virDomainObjPtr dom;
> + int ret = -1;
> + bool job = false;
> +
> + if (!(dom = vzDomObjFromDomainRef(domain)))
> + return -1;
> +
> + if (vzDomainObjBeginJob(dom) < 0)
> + goto cleanup;
> + job = true;
> +
> + if (!vzDomainObjIsExist(dom))
> + goto cleanup;
> +
same here and below
> + if (prlsdkResume(dom) < 0)
> + goto cleanup;
> +
> + if (prlsdkUpdateDomain(privconn->driver, dom) < 0)
> + goto cleanup;
> +
> + ret = 0;
> +
> + cleanup:
> + if (job)
> + vzDomainObjEndJob(dom);
> + virDomainObjEndAPI(&dom);
> +
> + return ret;
> }
>
> -static int vzDomainCreate(virDomainPtr domain)
> +static int
> +vzDomainCreate(virDomainPtr domain)
> {
> - return prlsdkDomainChangeState(domain, prlsdkStart);
> + vzConnPtr privconn = domain->conn->privateData;
> + virDomainObjPtr dom;
> + int ret = -1;
> + bool job = false;
> +
> + if (!(dom = vzDomObjFromDomainRef(domain)))
> + return -1;
> +
> + if (vzDomainObjBeginJob(dom) < 0)
> + goto cleanup;
> + job = true;
> +
> + if (!vzDomainObjIsExist(dom))
> + goto cleanup;
> +
> + if (prlsdkStart(dom) < 0)
> + goto cleanup;
> +
> + if (prlsdkUpdateDomain(privconn->driver, dom) < 0)
> + goto cleanup;
> +
> + ret = 0;
> +
> + cleanup:
> + if (job)
> + vzDomainObjEndJob(dom);
> + virDomainObjEndAPI(&dom);
> +
> + return ret;
> }
>
> -static int vzDomainDestroy(virDomainPtr domain)
> +static int
> +vzDomainDestroy(virDomainPtr domain)
> {
> - return prlsdkDomainChangeState(domain, prlsdkKill);
> + vzConnPtr privconn = domain->conn->privateData;
> + virDomainObjPtr dom;
> + int ret = -1;
> + bool job = false;
> +
> + if (!(dom = vzDomObjFromDomainRef(domain)))
> + return -1;
> +
> + if (vzDomainObjBeginJob(dom) < 0)
> + goto cleanup;
> + job = true;
> +
> + if (!vzDomainObjIsExist(dom))
> + goto cleanup;
> +
> + if (prlsdkKill(dom) < 0)
> + goto cleanup;
> +
> + if (prlsdkUpdateDomain(privconn->driver, dom) < 0)
> + goto cleanup;
> +
> + ret = 0;
> +
> + cleanup:
> + if (job)
> + vzDomainObjEndJob(dom);
> + virDomainObjEndAPI(&dom);
> +
> + return ret;
> }
>
> -static int vzDomainShutdown(virDomainPtr domain)
> +static int
> +vzDomainShutdown(virDomainPtr domain)
> {
> - return prlsdkDomainChangeState(domain, prlsdkStop);
> + vzConnPtr privconn = domain->conn->privateData;
> + virDomainObjPtr dom;
> + int ret = -1;
> + bool job = false;
> +
> + if (!(dom = vzDomObjFromDomainRef(domain)))
> + return -1;
> +
> + if (vzDomainObjBeginJob(dom) < 0)
> + goto cleanup;
> + job = true;
> +
> + if (!vzDomainObjIsExist(dom))
> + goto cleanup;
> +
> + if (prlsdkStop(dom) < 0)
> + goto cleanup;
> +
> + if (prlsdkUpdateDomain(privconn->driver, dom) < 0)
> + goto cleanup;
> +
> + ret = 0;
> +
> + cleanup:
> + if (job)
> + vzDomainObjEndJob(dom);
> + virDomainObjEndAPI(&dom);
> +
> + return ret;
> }
>
> -static int vzDomainReboot(virDomainPtr domain,
> - unsigned int flags)
> +static int
> +vzDomainReboot(virDomainPtr domain, unsigned int flags)
> {
> + vzConnPtr privconn = domain->conn->privateData;
> + virDomainObjPtr dom;
> + int ret = -1;
> + bool job = false;
> +
> virCheckFlags(0, -1);
> - return prlsdkDomainChangeState(domain, prlsdkRestart);
> +
> + if (!(dom = vzDomObjFromDomainRef(domain)))
> + return -1;
> +
> + if (vzDomainObjBeginJob(dom) < 0)
> + goto cleanup;
> + job = true;
> +
> + if (!vzDomainObjIsExist(dom))
> + goto cleanup;
> +
> + if (prlsdkRestart(dom) < 0)
> + goto cleanup;
> +
> + if (prlsdkUpdateDomain(privconn->driver, dom) < 0)
> + goto cleanup;
> +
> + ret = 0;
> +
> + cleanup:
> + if (job)
> + vzDomainObjEndJob(dom);
> + virDomainObjEndAPI(&dom);
> +
> + return ret;
> }
>
> static int vzDomainIsActive(virDomainPtr domain)
> @@ -1095,13 +1269,17 @@ vzDomainManagedSave(virDomainPtr domain, unsigned int flags)
>
> state = virDomainObjGetState(dom, &reason);
>
> - if (state == VIR_DOMAIN_RUNNING && (flags & VIR_DOMAIN_SAVE_PAUSED)) {
> - ret = prlsdkDomainChangeStateLocked(privconn->driver, dom, prlsdkPause);
> - if (ret)
> - goto cleanup;
> - }
> + if (state == VIR_DOMAIN_RUNNING && (flags & VIR_DOMAIN_SAVE_PAUSED) &&
> + prlsdkPause(dom) < 0)
> + goto cleanup;
>
> - ret = prlsdkDomainChangeStateLocked(privconn->driver, dom, prlsdkSuspend);
> + if (prlsdkSuspend(dom) < 0)
> + goto cleanup;
> +
> + if (prlsdkUpdateDomain(privconn->driver, dom) < 0)
> + goto cleanup;
> +
> + ret = 0;
>
> cleanup:
> if (job)
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index 77193e6..02cbb3b 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -40,6 +40,8 @@
>
> static int
> prlsdkUUIDParse(const char *uuidstr, unsigned char *uuid);
> +static void
> +prlsdkConvertError(PRL_RESULT pret);
>
> VIR_LOG_INIT("parallels.sdk");
>
> @@ -2004,131 +2006,129 @@ void prlsdkUnsubscribeFromPCSEvents(vzDriverPtr driver)
> logPrlError(ret);
> }
>
> -PRL_RESULT prlsdkStart(PRL_HANDLE sdkdom)
> +int prlsdkStart(virDomainObjPtr dom)
> {
> PRL_HANDLE job = PRL_INVALID_HANDLE;
> + vzDomObjPtr privdom = dom->privateData;
> + PRL_RESULT pret;
> +
> + job = PrlVm_StartEx(privdom->sdkdom, PSM_VM_START, 0);
> + if (PRL_FAILED(pret = waitDomainJob(job, dom))) {
> + prlsdkConvertError(pret);
> + return -1;
> + }
>
> - job = PrlVm_StartEx(sdkdom, PSM_VM_START, 0);
> - return waitJob(job);
> + return 0;
> }
>
> -static PRL_RESULT prlsdkStopEx(PRL_HANDLE sdkdom, PRL_UINT32 mode)
> +int prlsdkKill(virDomainObjPtr dom)
> {
> PRL_HANDLE job = PRL_INVALID_HANDLE;
> + vzDomObjPtr privdom = dom->privateData;
> + PRL_RESULT pret;
>
> - job = PrlVm_StopEx(sdkdom, mode, 0);
> - return waitJob(job);
> -}
> + job = PrlVm_StopEx(privdom->sdkdom, PSM_KILL, 0);
> + if (PRL_FAILED(pret = waitDomainJob(job, dom))) {
> + prlsdkConvertError(pret);
> + return -1;
> + }
>
> -PRL_RESULT prlsdkKill(PRL_HANDLE sdkdom)
> -{
> - return prlsdkStopEx(sdkdom, PSM_KILL);
> + return 0;
> }
>
> -PRL_RESULT prlsdkStop(PRL_HANDLE sdkdom)
> +int prlsdkStop(virDomainObjPtr dom)
> {
> - return prlsdkStopEx(sdkdom, PSM_SHUTDOWN);
> + PRL_HANDLE job = PRL_INVALID_HANDLE;
> + vzDomObjPtr privdom = dom->privateData;
> + PRL_RESULT pret;
> +
> + job = PrlVm_StopEx(privdom->sdkdom, PSM_SHUTDOWN, 0);
> + if (PRL_FAILED(pret = waitDomainJob(job, dom))) {
> + prlsdkConvertError(pret);
> + return -1;
> + }
> +
> + return 0;
> }
>
> -PRL_RESULT prlsdkPause(PRL_HANDLE sdkdom)
> +int prlsdkPause(virDomainObjPtr dom)
> {
> PRL_HANDLE job = PRL_INVALID_HANDLE;
> + vzDomObjPtr privdom = dom->privateData;
> + PRL_RESULT pret;
> +
> + job = PrlVm_Pause(privdom->sdkdom, false);
> + if (PRL_FAILED(pret = waitDomainJob(job, dom))) {
> + prlsdkConvertError(pret);
> + return -1;
> + }
>
> - job = PrlVm_Pause(sdkdom, false);
> - return waitJob(job);
> + return 0;
> }
>
> -PRL_RESULT prlsdkResume(PRL_HANDLE sdkdom)
> +int prlsdkResume(virDomainObjPtr dom)
> {
> PRL_HANDLE job = PRL_INVALID_HANDLE;
> + vzDomObjPtr privdom = dom->privateData;
> + PRL_RESULT pret;
>
> - job = PrlVm_Resume(sdkdom);
> - return waitJob(job);
> + job = PrlVm_Resume(privdom->sdkdom);
> + if (PRL_FAILED(pret = waitDomainJob(job, dom))) {
> + prlsdkConvertError(pret);
> + return -1;
> + }
> +
> + return 0;
> }
>
> -PRL_RESULT prlsdkSuspend(PRL_HANDLE sdkdom)
> +int prlsdkSuspend(virDomainObjPtr dom)
> {
> PRL_HANDLE job = PRL_INVALID_HANDLE;
> + vzDomObjPtr privdom = dom->privateData;
> + PRL_RESULT pret;
>
> - job = PrlVm_Suspend(sdkdom);
> - return waitJob(job);
> + job = PrlVm_Suspend(privdom->sdkdom);
> + if (PRL_FAILED(pret = waitDomainJob(job, dom))) {
> + prlsdkConvertError(pret);
> + return -1;
> + }
> +
> + return 0;
> }
>
> -PRL_RESULT prlsdkRestart(PRL_HANDLE sdkdom)
> +int prlsdkRestart(virDomainObjPtr dom)
> {
> PRL_HANDLE job = PRL_INVALID_HANDLE;
> + vzDomObjPtr privdom = dom->privateData;
> + PRL_RESULT pret;
>
> - job = PrlVm_Restart(sdkdom);
> - return waitJob(job);
> + job = PrlVm_Restart(privdom->sdkdom);
> + if (PRL_FAILED(pret = waitDomainJob(job, dom))) {
> + prlsdkConvertError(pret);
> + return -1;
> + }
> +
> + return 0;
> }
>
> -int
> -prlsdkDomainChangeStateLocked(vzDriverPtr driver,
> - virDomainObjPtr dom,
> - prlsdkChangeStateFunc chstate)
> +static void
> +prlsdkConvertError(PRL_RESULT pret)
> {
> - vzDomObjPtr pdom;
> - PRL_RESULT pret;
> virErrorNumber virerr;
>
> - pdom = dom->privateData;
> - virObjectUnlock(dom);
> - pret = chstate(pdom->sdkdom);
> - virObjectLock(dom);
> - if (PRL_FAILED(pret)) {
> - virResetLastError();
> -
> - switch (pret) {
> - case PRL_ERR_DISP_VM_IS_NOT_STARTED:
> - case PRL_ERR_DISP_VM_IS_NOT_STOPPED:
> - case PRL_ERR_INVALID_ACTION_REQUESTED:
> - case PRL_ERR_UNIMPLEMENTED:
> - virerr = VIR_ERR_OPERATION_INVALID;
> - break;
> - default:
> - virerr = VIR_ERR_OPERATION_FAILED;
> - }
> -
> - virReportError(virerr, "%s", _("Can't change domain state."));
> - return -1;
> - }
> -
> - return prlsdkUpdateDomain(driver, dom);
> -}
> -
> -int
> -prlsdkDomainChangeState(virDomainPtr domain,
> - prlsdkChangeStateFunc chstate)
> -{
> - vzConnPtr privconn = domain->conn->privateData;
> - virDomainObjPtr dom;
> - int ret = -1;
> - bool job = false;
> -
> - if (!(dom = vzDomObjFromDomainRef(domain)))
> - return -1;
> -
> - if (vzDomainObjBeginJob(dom) < 0)
> - goto cleanup;
> - job = true;
> -
> - if (dom->removing) {
> - char uuidstr[VIR_UUID_STRING_BUFLEN];
> -
> - virUUIDFormat(dom->def->uuid, uuidstr);
> - virReportError(VIR_ERR_NO_DOMAIN,
> - _("no domain with matching uuid '%s' (%s)"),
> - uuidstr, dom->def->name);
> - goto cleanup;
> + switch (pret) {
> + case PRL_ERR_DISP_VM_IS_NOT_STARTED:
> + case PRL_ERR_DISP_VM_IS_NOT_STOPPED:
> + case PRL_ERR_INVALID_ACTION_REQUESTED:
> + case PRL_ERR_UNIMPLEMENTED:
> + virerr = VIR_ERR_OPERATION_INVALID;
> + break;
> + default:
> + virerr = VIR_ERR_OPERATION_FAILED;
> }
>
> - ret = prlsdkDomainChangeStateLocked(privconn->driver, dom, chstate);
> -
> - cleanup:
> - if (job)
> - vzDomainObjEndJob(dom);
> - virDomainObjEndAPI(&dom);
> - return ret;
> + virResetLastError();
> + virReportError(virerr, "%s", _("Can't change domain state."));
> }
>
> static int
> diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h
> index e32001a..e9d7169 100644
> --- a/src/vz/vz_sdk.h
> +++ b/src/vz/vz_sdk.h
> @@ -37,22 +37,15 @@ prlsdkAddDomainByName(vzDriverPtr driver, const char *name);
> int prlsdkUpdateDomain(vzDriverPtr driver, virDomainObjPtr dom);
> int prlsdkSubscribeToPCSEvents(vzDriverPtr driver);
> void prlsdkUnsubscribeFromPCSEvents(vzDriverPtr driver);
> -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);
> -PRL_RESULT prlsdkRestart(PRL_HANDLE sdkdom);
>
> -typedef PRL_RESULT (*prlsdkChangeStateFunc)(PRL_HANDLE sdkdom);
> -int
> -prlsdkDomainChangeState(virDomainPtr domain,
> - prlsdkChangeStateFunc chstate);
> -int
> -prlsdkDomainChangeStateLocked(vzDriverPtr driver,
> - virDomainObjPtr dom,
> - prlsdkChangeStateFunc chstate);
> +int prlsdkStart(virDomainObjPtr dom);
> +int prlsdkKill(virDomainObjPtr dom);
> +int prlsdkStop(virDomainObjPtr dom);
> +int prlsdkPause(virDomainObjPtr dom);
> +int prlsdkResume(virDomainObjPtr dom);
> +int prlsdkSuspend(virDomainObjPtr dom);
> +int prlsdkRestart(virDomainObjPtr dom);
> +
> int
> prlsdkApplyConfig(vzDriverPtr driver,
> virDomainObjPtr dom,
ACK with mentioned changes
Maxim
More information about the libvir-list
mailing list