[libvirt] [PATCH 0/3] vz: domain undefine fixes

Maxim Nestratov (3): vz: delete domains when undefine is called vz: move prlsdkCleanupBridgedNet after domain deletion vz: support additional flags in domain undefine src/vz/vz_driver.c | 5 +- src/vz/vz_sdk.c | 138 +++++++++++++++++++++++++++++++++++++++++++++++++++-- src/vz/vz_sdk.h | 2 +- 3 files changed, 138 insertions(+), 7 deletions(-) -- 2.4.3

Currently vz driver unregisters domains when undefine is called, which is wrong because it contradicts with expected behavior. All vz domains are persistent, which means that when one is defined a new bundle directory containing meta data is created. Undefining domains in a way we do now leaves those directories undeleted, which prevents subsequent define call for the same domain xml. I.e. the following sequence define->undefine->define doesn't work now. The patch fixes the problem by calling PrlVm_Delete instead of PrlVm_Unreg detaching all disks prior actually doing this to prevent images deletion. Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_sdk.c | 45 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 2b031c9..12c8be9 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -3718,6 +3718,46 @@ prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def) return ret; } +/** + * prlsdkDetachDomainHardDisks: + * + * @sdkdom: domain handle + * + * Returns 0 if hard disks were successfully detached or not detected. + */ +static int +prlsdkDetachDomainHardDisks(PRL_HANDLE sdkdom) +{ + int ret = -1; + PRL_RESULT pret; + PRL_UINT32 hddCount; + PRL_UINT32 i; + PRL_HANDLE job; + + job = PrlVm_BeginEdit(sdkdom); + if (PRL_FAILED(waitJob(job))) + goto cleanup; + + pret = PrlVmCfg_GetHardDisksCount(sdkdom, &hddCount); + prlsdkCheckRetGoto(pret, cleanup); + + for (i = 0; i < hddCount; ++i) { + ret = prlsdkDelDisk(sdkdom, i); + if (ret) + goto cleanup; + } + + job = PrlVm_CommitEx(sdkdom, PVCF_DETACH_HDD_BUNDLE); + if (PRL_FAILED(waitJob(job))) + ret = -1; + + cleanup: + + return ret; +} + + + int prlsdkUnregisterDomain(vzConnPtr privconn, virDomainObjPtr dom) { @@ -3728,7 +3768,10 @@ prlsdkUnregisterDomain(vzConnPtr privconn, virDomainObjPtr dom) for (i = 0; i < dom->def->nnets; i++) prlsdkCleanupBridgedNet(privconn, dom->def->nets[i]); - job = PrlVm_Unreg(privdom->sdkdom); + if (prlsdkDetachDomainHardDisks(privdom->sdkdom)) + return -1; + + job = PrlVm_Delete(privdom->sdkdom, PRL_INVALID_HANDLE); if (PRL_FAILED(waitJob(job))) return -1; -- 2.4.3

On Tue, 2015-12-22 at 18:29 +0300, Maxim Nestratov wrote:
Currently vz driver unregisters domains when undefine is called, which is wrong because it contradicts with expected behavior. All vz domains are persistent, which means that when one is defined a new bundle directory containing meta data is created. Undefining domains in a way we do now leaves those directories undeleted, which prevents subsequent define call for the same domain xml. I.e. the following sequence define->undefine->define doesn't work now. The patch fixes the problem by calling PrlVm_Delete instead of PrlVm_Unreg detaching all disks prior actually doing this to prevent images deletion.
ACK
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_sdk.c | 45 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-)
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 2b031c9..12c8be9 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -3718,6 +3718,46 @@ prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def) return ret; } +/** + * prlsdkDetachDomainHardDisks: + * + * @sdkdom: domain handle + * + * Returns 0 if hard disks were successfully detached or not detected. + */ +static int +prlsdkDetachDomainHardDisks(PRL_HANDLE sdkdom) +{ + int ret = -1; + PRL_RESULT pret; + PRL_UINT32 hddCount; + PRL_UINT32 i; + PRL_HANDLE job; + + job = PrlVm_BeginEdit(sdkdom); + if (PRL_FAILED(waitJob(job))) + goto cleanup; + + pret = PrlVmCfg_GetHardDisksCount(sdkdom, &hddCount); + prlsdkCheckRetGoto(pret, cleanup); + + for (i = 0; i < hddCount; ++i) { + ret = prlsdkDelDisk(sdkdom, i); + if (ret) + goto cleanup; + } + + job = PrlVm_CommitEx(sdkdom, PVCF_DETACH_HDD_BUNDLE); + if (PRL_FAILED(waitJob(job))) + ret = -1; + + cleanup: + + return ret; +} + + + int prlsdkUnregisterDomain(vzConnPtr privconn, virDomainObjPtr dom) { @@ -3728,7 +3768,10 @@ prlsdkUnregisterDomain(vzConnPtr privconn, virDomainObjPtr dom) for (i = 0; i < dom->def->nnets; i++) prlsdkCleanupBridgedNet(privconn, dom->def->nets[i]); - job = PrlVm_Unreg(privdom->sdkdom); + if (prlsdkDetachDomainHardDisks(privdom->sdkdom)) + return -1; + + job = PrlVm_Delete(privdom->sdkdom, PRL_INVALID_HANDLE); if (PRL_FAILED(waitJob(job))) return -1;

prlsdkCleanupBridgedNet call should be made strongly after any actual domain deletion accurs. By doing this we avoid any potential problems connected with second undefine call when it is made after first one fails by some reason, and we detect that network is already deleted. Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_sdk.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 12c8be9..fb6d3f4 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -3765,8 +3765,6 @@ prlsdkUnregisterDomain(vzConnPtr privconn, virDomainObjPtr dom) PRL_HANDLE job; size_t i; - for (i = 0; i < dom->def->nnets; i++) - prlsdkCleanupBridgedNet(privconn, dom->def->nets[i]); if (prlsdkDetachDomainHardDisks(privdom->sdkdom)) return -1; @@ -3775,6 +3773,9 @@ prlsdkUnregisterDomain(vzConnPtr privconn, virDomainObjPtr dom) if (PRL_FAILED(waitJob(job))) return -1; + for (i = 0; i < dom->def->nnets; i++) + prlsdkCleanupBridgedNet(privconn, dom->def->nets[i]); + if (prlsdkSendEvent(privconn, dom, VIR_DOMAIN_EVENT_UNDEFINED, VIR_DOMAIN_EVENT_UNDEFINED_REMOVED) < 0) return -1; -- 2.4.3

On Tue, 2015-12-22 at 18:29 +0300, Maxim Nestratov wrote:
prlsdkCleanupBridgedNet call should be made strongly after any actual domain deletion accurs. By doing this we avoid any potential problems connected with second undefine call when it is made after first one fails by some reason, and we detect that network is already deleted.
ACK
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_sdk.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 12c8be9..fb6d3f4 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -3765,8 +3765,6 @@ prlsdkUnregisterDomain(vzConnPtr privconn, virDomainObjPtr dom) PRL_HANDLE job; size_t i; - for (i = 0; i < dom->def->nnets; i++) - prlsdkCleanupBridgedNet(privconn, dom->def->nets[i]); if (prlsdkDetachDomainHardDisks(privdom->sdkdom)) return -1; @@ -3775,6 +3773,9 @@ prlsdkUnregisterDomain(vzConnPtr privconn, virDomainObjPtr dom) if (PRL_FAILED(waitJob(job))) return -1; + for (i = 0; i < dom->def->nnets; i++) + prlsdkCleanupBridgedNet(privconn, dom->def->nets[i]); + if (prlsdkSendEvent(privconn, dom, VIR_DOMAIN_EVENT_UNDEFINED, VIR_DOMAIN_EVENT_UNDEFINED_REMOVED) < 0) return -1;

Implement VIR_DOMAIN_UNDEFINE_MANAGED_SAVE and VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA flags support. Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_driver.c | 5 ++-- src/vz/vz_sdk.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++++- src/vz/vz_sdk.h | 2 +- 3 files changed, 91 insertions(+), 4 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 2ef47e4..2452d96 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -961,12 +961,13 @@ vzDomainUndefineFlags(virDomainPtr domain, virDomainObjPtr dom = NULL; int ret; - virCheckFlags(0, -1); + virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE | + VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, -1); if (!(dom = vzDomObjFromDomain(domain))) return -1; - ret = prlsdkUnregisterDomain(privconn, dom); + ret = prlsdkUnregisterDomain(privconn, dom, flags); if (ret) virObjectUnlock(dom); diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index fb6d3f4..b78c413 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -3756,15 +3756,101 @@ prlsdkDetachDomainHardDisks(PRL_HANDLE sdkdom) return ret; } +/** + * prlsdkDomainHasSnapshots: + * + * This function detects where a domain specified by @sdkdom + * has snapshots. It doesn't count them correctly. + * + * @sdkdom: domain handle + * @found: a value more than zero if snapshots present + * + * Returns 0 if function succeeds, -1 otherwise. + */ +static int +prlsdkDomainHasSnapshots(PRL_HANDLE sdkdom, int* found) +{ + int ret = -1; + PRL_RESULT pret; + PRL_HANDLE job; + PRL_HANDLE result; + char *snapshotxml = NULL; + unsigned int len, paramsCount; + xmlDocPtr xml = NULL; + xmlXPathContextPtr ctxt = NULL; + + if (!found) + goto cleanup; + job = PrlVm_GetSnapshotsTreeEx(sdkdom, PGST_WITHOUT_SCREENSHOTS); + if (PRL_FAILED(getJobResult(job, &result))) + goto cleanup; + + pret = PrlResult_GetParamsCount(result, ¶msCount); + prlsdkCheckRetGoto(pret, cleanup); + + if (!paramsCount) + goto cleanup; + + pret = PrlResult_GetParamAsString(result, 0, &len); + prlsdkCheckRetGoto(pret, cleanup); + + if (VIR_ALLOC_N(snapshotxml, len+1) < 0) + goto cleanup; + + pret = PrlResult_GetParamAsString(result, snapshotxml, &len); + prlsdkCheckRetGoto(pret, cleanup); + + if (len <= 1) { + /* The document is empty that means no snapshots */ + *found = 0; + ret = 0; + goto cleanup; + } + + if (!(xml = virXMLParseStringCtxt(snapshotxml, "SavedStateItem", &ctxt))) + goto cleanup; + + *found = virXMLChildElementCount(ctxt->node); + ret = 0; + + cleanup: + + xmlXPathFreeContext(ctxt); + xmlFreeDoc(xml); + VIR_FREE(snapshotxml); + return ret; +} int -prlsdkUnregisterDomain(vzConnPtr privconn, virDomainObjPtr dom) +prlsdkUnregisterDomain(vzConnPtr privconn, virDomainObjPtr dom, unsigned int flags) { vzDomObjPtr privdom = dom->privateData; PRL_HANDLE job; size_t i; + int snapshotfound = 0; + VIRTUAL_MACHINE_STATE domainState; + + if (prlsdkGetDomainState(privdom->sdkdom, &domainState) < 0) + return -1; + + if (VMS_SUSPENDED == domainState && + !(flags & VIR_DOMAIN_UNDEFINE_MANAGED_SAVE)) { + + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Refusing to undefine while domain managed " + "save image exists")); + return -1; + } + + if (prlsdkDomainHasSnapshots(privdom->sdkdom, &snapshotfound) < 0) + return -1; + if (snapshotfound && !(flags & VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Refusing to undefine while snapshots exist")); + return -1; + } if (prlsdkDetachDomainHardDisks(privdom->sdkdom)) return -1; diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h index 88ee7d9..ff6be07 100644 --- a/src/vz/vz_sdk.h +++ b/src/vz/vz_sdk.h @@ -58,7 +58,7 @@ prlsdkApplyConfig(virConnectPtr conn, int prlsdkCreateVm(virConnectPtr conn, virDomainDefPtr def); int prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def); int -prlsdkUnregisterDomain(vzConnPtr privconn, virDomainObjPtr dom); +prlsdkUnregisterDomain(vzConnPtr privconn, virDomainObjPtr dom, unsigned int flags); int prlsdkDomainManagedSaveRemove(virDomainObjPtr dom); int -- 2.4.3

On Tue, 2015-12-22 at 18:29 +0300, Maxim Nestratov wrote:
Implement VIR_DOMAIN_UNDEFINE_MANAGED_SAVE and VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA flags support.
ACKed and pushed, thanks!
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_driver.c | 5 ++-- src/vz/vz_sdk.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++++- src/vz/vz_sdk.h | 2 +- 3 files changed, 91 insertions(+), 4 deletions(-)
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 2ef47e4..2452d96 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -961,12 +961,13 @@ vzDomainUndefineFlags(virDomainPtr domain, virDomainObjPtr dom = NULL; int ret; - virCheckFlags(0, -1); + virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE | + VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, -1); if (!(dom = vzDomObjFromDomain(domain))) return -1; - ret = prlsdkUnregisterDomain(privconn, dom); + ret = prlsdkUnregisterDomain(privconn, dom, flags); if (ret) virObjectUnlock(dom); diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index fb6d3f4..b78c413 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -3756,15 +3756,101 @@ prlsdkDetachDomainHardDisks(PRL_HANDLE sdkdom) return ret; } +/** + * prlsdkDomainHasSnapshots: + * + * This function detects where a domain specified by @sdkdom + * has snapshots. It doesn't count them correctly. + * + * @sdkdom: domain handle + * @found: a value more than zero if snapshots present + * + * Returns 0 if function succeeds, -1 otherwise. + */ +static int +prlsdkDomainHasSnapshots(PRL_HANDLE sdkdom, int* found) +{ + int ret = -1; + PRL_RESULT pret; + PRL_HANDLE job; + PRL_HANDLE result; + char *snapshotxml = NULL; + unsigned int len, paramsCount; + xmlDocPtr xml = NULL; + xmlXPathContextPtr ctxt = NULL; + + if (!found) + goto cleanup; + job = PrlVm_GetSnapshotsTreeEx(sdkdom, PGST_WITHOUT_SCREENSHOTS); + if (PRL_FAILED(getJobResult(job, &result))) + goto cleanup; + + pret = PrlResult_GetParamsCount(result, ¶msCount); + prlsdkCheckRetGoto(pret, cleanup); + + if (!paramsCount) + goto cleanup; + + pret = PrlResult_GetParamAsString(result, 0, &len); + prlsdkCheckRetGoto(pret, cleanup); + + if (VIR_ALLOC_N(snapshotxml, len+1) < 0) + goto cleanup; + + pret = PrlResult_GetParamAsString(result, snapshotxml, &len); + prlsdkCheckRetGoto(pret, cleanup); + + if (len <= 1) { + /* The document is empty that means no snapshots */ + *found = 0; + ret = 0; + goto cleanup; + } + + if (!(xml = virXMLParseStringCtxt(snapshotxml, "SavedStateItem", &ctxt))) + goto cleanup; + + *found = virXMLChildElementCount(ctxt->node); + ret = 0; + + cleanup: + + xmlXPathFreeContext(ctxt); + xmlFreeDoc(xml); + VIR_FREE(snapshotxml); + return ret; +} int -prlsdkUnregisterDomain(vzConnPtr privconn, virDomainObjPtr dom) +prlsdkUnregisterDomain(vzConnPtr privconn, virDomainObjPtr dom, unsigned int flags) { vzDomObjPtr privdom = dom->privateData; PRL_HANDLE job; size_t i; + int snapshotfound = 0; + VIRTUAL_MACHINE_STATE domainState; + + if (prlsdkGetDomainState(privdom->sdkdom, &domainState) < 0) + return -1; + + if (VMS_SUSPENDED == domainState && + !(flags & VIR_DOMAIN_UNDEFINE_MANAGED_SAVE)) { + + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Refusing to undefine while domain managed " + "save image exists")); + return -1; + } + + if (prlsdkDomainHasSnapshots(privdom->sdkdom, &snapshotfound) < 0) + return -1; + if (snapshotfound && !(flags & VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("Refusing to undefine while snapshots exist")); + return -1; + } if (prlsdkDetachDomainHardDisks(privdom->sdkdom)) return -1; diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h index 88ee7d9..ff6be07 100644 --- a/src/vz/vz_sdk.h +++ b/src/vz/vz_sdk.h @@ -58,7 +58,7 @@ prlsdkApplyConfig(virConnectPtr conn, int prlsdkCreateVm(virConnectPtr conn, virDomainDefPtr def); int prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def); int -prlsdkUnregisterDomain(vzConnPtr privconn, virDomainObjPtr dom); +prlsdkUnregisterDomain(vzConnPtr privconn, virDomainObjPtr dom, unsigned int flags); int prlsdkDomainManagedSaveRemove(virDomainObjPtr dom); int
participants (2)
-
Dmitry Guryanov
-
Maxim Nestratov