ACL check on perform step should be in API call itself to make ACL checking script pass. Thus we need to reorganize code to obtain domain object in perform API itself. Most of this is straight forward, the only nuance is dropping locks on lengthy remote operations. The other motivation is to have only perform step ACL checks for p2p migration instead of both begin in perform if we can leave ACL check in vzDomainMigratePerformStep. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx> --- src/vz/vz_driver.c | 114 +++++++++++++++++++++++++++++++---------------------- 1 file changed, 66 insertions(+), 48 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 12abe82..9fa377e 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -2601,43 +2601,55 @@ vzEatCookie(const char *cookiein, int cookieinlen, unsigned int flags) NULL static char * -vzDomainMigrateBegin3Params(virDomainPtr domain, - virTypedParameterPtr params, - int nparams, - char **cookieout, - int *cookieoutlen, - unsigned int flags) +vzDomainMigrateBeginStep(virDomainObjPtr dom, + vzDriverPtr driver, + virTypedParameterPtr params, + int nparams, + char **cookieout, + int *cookieoutlen) { - char *xml = NULL; - virDomainObjPtr dom = NULL; - vzConnPtr privconn = domain->conn->privateData; - - virCheckFlags(VZ_MIGRATION_FLAGS, NULL); - - if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0) - goto cleanup; - /* we can't do this check via VZ_MIGRATION_PARAMETERS as on preparation * step domain xml will be passed via this parameter and it is a common * style to use single allowed parameter list definition in all steps */ if (virTypedParamsGet(params, nparams, VIR_MIGRATE_PARAM_DEST_XML)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("Changing destination XML is not supported")); - goto cleanup; + return NULL; } - if (!(dom = vzDomObjFromDomain(domain))) - goto cleanup; - /* session uuid, domain uuid and domain name are for backward compat */ - if (vzBakeCookie(privconn->driver, dom, cookieout, cookieoutlen, + if (vzBakeCookie(driver, dom, cookieout, cookieoutlen, VZ_MIGRATION_COOKIE_SESSION_UUID | VZ_MIGRATION_COOKIE_DOMAIN_UUID | VZ_MIGRATION_COOKIE_DOMAIN_NAME) < 0) + return NULL; + + return virDomainDefFormat(dom->def, driver->caps, + VIR_DOMAIN_XML_MIGRATABLE); +} + +static char * +vzDomainMigrateBegin3Params(virDomainPtr domain, + virTypedParameterPtr params, + int nparams, + char **cookieout, + int *cookieoutlen, + unsigned int flags) +{ + char *xml = NULL; + virDomainObjPtr dom = NULL; + vzConnPtr privconn = domain->conn->privateData; + + virCheckFlags(VZ_MIGRATION_FLAGS, NULL); + + if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0) + goto cleanup; + + if (!(dom = vzDomObjFromDomain(domain))) goto cleanup; - xml = virDomainDefFormat(dom->def, privconn->driver->caps, - VIR_DOMAIN_XML_MIGRATABLE); + xml = vzDomainMigrateBeginStep(dom, privconn->driver, params, nparams, + cookieout, cookieoutlen); cleanup: @@ -2765,7 +2777,8 @@ vzParseVzURI(const char *uri_str) } static int -vzDomainMigratePerformStep(virDomainPtr domain, +vzDomainMigratePerformStep(virDomainObjPtr dom, + vzDriverPtr driver, virTypedParameterPtr params, int nparams, const char *cookiein, @@ -2773,20 +2786,13 @@ vzDomainMigratePerformStep(virDomainPtr domain, unsigned int flags) { int ret = -1; - virDomainObjPtr dom = NULL; - vzDomObjPtr privdom; + vzDomObjPtr privdom = dom->privateData; virURIPtr vzuri = NULL; - vzConnPtr privconn = domain->conn->privateData; const char *miguri = NULL; const char *dname = NULL; vzMigrationCookiePtr mig = NULL; bool job = false; - virCheckFlags(VZ_MIGRATION_FLAGS, -1); - - if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0) - goto cleanup; - if (virTypedParamsGetString(params, nparams, VIR_MIGRATE_PARAM_URI, &miguri) < 0 || virTypedParamsGetString(params, nparams, @@ -2803,13 +2809,9 @@ vzDomainMigratePerformStep(virDomainPtr domain, VZ_MIGRATION_COOKIE_SESSION_UUID))) goto cleanup; - if (!(dom = vzDomObjFromDomainRef(domain))) - goto cleanup; - if (vzDomainObjBeginJob(dom) < 0) goto cleanup; job = true; - privdom = dom->privateData; privdom->job.hasProgress = true; if (!vzDomainObjIsExist(dom)) @@ -2821,7 +2823,7 @@ vzDomainMigratePerformStep(virDomainPtr domain, if (prlsdkMigrate(dom, vzuri, mig->session_uuid, dname, flags) < 0) goto cleanup; - virDomainObjListRemove(privconn->driver->domains, dom); + virDomainObjListRemove(driver->domains, dom); virObjectLock(dom); ret = 0; @@ -2829,7 +2831,6 @@ vzDomainMigratePerformStep(virDomainPtr domain, cleanup: if (job) vzDomainObjEndJob(dom); - virDomainObjEndAPI(&dom); virURIFree(vzuri); vzMigrationCookieFree(mig); @@ -2837,7 +2838,8 @@ vzDomainMigratePerformStep(virDomainPtr domain, } static int -vzDomainMigratePerformP2P(virDomainPtr domain, +vzDomainMigratePerformP2P(virDomainObjPtr dom, + vzDriverPtr driver, const char *dconnuri, virTypedParameterPtr orig_params, int nparams, @@ -2862,19 +2864,22 @@ vzDomainMigratePerformP2P(virDomainPtr domain, if (!(dconn = virConnectOpen(dconnuri))) goto done; - if (!(dom_xml = vzDomainMigrateBegin3Params(domain, params, nparams, - &cookieout, &cookieoutlen, - flags))) + if (!(dom_xml = vzDomainMigrateBeginStep(dom, driver, params, nparams, + &cookieout, &cookieoutlen))) goto done; cookiein = cookieout; cookieinlen = cookieoutlen; cookieout = NULL; cookieoutlen = 0; - if (dconn->driver->domainMigratePrepare3Params + virObjectUnlock(dom); + ret = dconn->driver->domainMigratePrepare3Params (dconn, params, nparams, cookiein, cookieinlen, - &cookieout, &cookieoutlen, &uri, flags) < 0) + &cookieout, &cookieoutlen, &uri, flags); + virObjectLock(dom); + if (ret < 0) goto done; + ret = -1; /* preparation step was successful, thus on any error we must perform * finish step to finalize migration on target @@ -2890,7 +2895,7 @@ vzDomainMigratePerformP2P(virDomainPtr domain, cookieinlen = cookieoutlen; cookieout = NULL; cookieoutlen = 0; - if (vzDomainMigratePerformStep(domain, params, nparams, cookiein, + if (vzDomainMigratePerformStep(dom, driver, params, nparams, cookiein, cookieinlen, flags) < 0) { orig_err = virSaveLastError(); goto finish; @@ -2903,12 +2908,14 @@ vzDomainMigratePerformP2P(virDomainPtr domain, VIR_MIGRATE_PARAM_DEST_NAME, NULL) <= 0 && virTypedParamsReplaceString(¶ms, &nparams, VIR_MIGRATE_PARAM_DEST_NAME, - domain->name) < 0) + dom->def->name) < 0) goto done; + virObjectUnlock(dom); ddomain = dconn->driver->domainMigrateFinish3Params(dconn, params, nparams, NULL, 0, NULL, NULL, flags, cancelled); + virObjectLock(dom); if (ddomain) ret = 0; virObjectUnref(ddomain); @@ -2940,17 +2947,28 @@ vzDomainMigratePerform3Params(virDomainPtr domain, int *cookieoutlen ATTRIBUTE_UNUSED, unsigned int flags) { + int ret; + virDomainObjPtr dom; + vzConnPtr privconn = domain->conn->privateData; + virCheckFlags(VZ_MIGRATION_FLAGS, -1); if (virTypedParamsValidate(params, nparams, VZ_MIGRATION_PARAMETERS) < 0) return -1; + if (!(dom = vzDomObjFromDomainRef(domain))) + return -1; + if (flags & VIR_MIGRATE_PEER2PEER) - return vzDomainMigratePerformP2P(domain, dconnuri, params, nparams, flags); + ret = vzDomainMigratePerformP2P(dom, privconn->driver, dconnuri, + params, nparams, flags); else - return vzDomainMigratePerformStep(domain, params, nparams, - cookiein, cookieinlen, flags); + ret = vzDomainMigratePerformStep(dom, privconn->driver, params, nparams, + cookiein, cookieinlen, flags); + virDomainObjEndAPI(&dom); + + return ret; } static virDomainPtr -- 1.8.3.1 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list