[libvirt] [PATCH] vz: handle gracefully races on undefining domain

Nikolay Shirokovskiy nshirokovskiy at virtuozzo.com
Mon Jun 20 07:40:37 UTC 2016


  This patch is not critical but nice to have. The original motivation
was error message in logs on undefining domain thru vz driver.
Undefine procedure drops domain lock while waiting for detaching
disks vz sdk call. Meanwhile vz sdk event domain-config-changed
arrives, its handler finds domain and is blocked waiting for job
condition. After undefine API call finishes event processing procedes
and tries to refreshes domain config thru existing vz sdk domain handle.
Domain does not exists anymore and event processing fails. Everything
is fine we just don't want to see error message in log for this
particular case.

  Fortunately domain has flag that domain is removed from list. This
also imply that vz sdk domain is also undefined. Thus if we check
for this flag right after domain is locked again on accuiring
job condition we gracefully handle this situation.

  Actually the race can happen in other situations too. Any
time we wait for job condition in mutualy exclusive job in
time when we acquire it vz sdk domain can cease to exist.
So instead of general internal error we can return domain
not found which is easier to handle. We don't need to patch
other places in mutually exclusive jobs where domain lock
is dropped as if job is started domain can't be undefine
by mutually exclusive undefine job.

  In case of API calls that are not jobs (load snapshots etc) we'd
better to check if domain exists every time domain lock is
reacquired. Fortunately these calls drop domain lock only
once to call appropriate vz sdk API call.

  The code of this patch is quite similar to qemu driver checks
for is domain is active after acquiring a job. The difference
only while qemu domain is operational while process is active
vz domain is operational while domain exists.

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
---
 src/vz/vz_driver.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 src/vz/vz_sdk.c    | 32 +++++++++++++++++++++++++++++---
 2 files changed, 75 insertions(+), 3 deletions(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 6a508b3..67db2d9 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -716,6 +716,22 @@ vzDomainGetAutostart(virDomainPtr domain, int *autostart)
     return 0;
 }
 
+static bool
+vzDomainObjIsExist(virDomainObjPtr dom)
+{
+    char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+    if (!dom->removing)
+        return true;
+
+    virUUIDFormat(dom->def->uuid, uuidstr);
+    virReportError(VIR_ERR_NO_DOMAIN,
+                   _("no domain with matching uuid '%s' (%s)"),
+                   uuidstr, dom->def->name);
+
+    return false;
+}
+
 static virDomainPtr
 vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags)
 {
@@ -782,6 +798,9 @@ vzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags)
                 goto cleanup;
             job = true;
 
+            if (!vzDomainObjIsExist(dom))
+                goto cleanup;
+
             if (prlsdkApplyConfig(driver, dom, def))
                 goto cleanup;
 
@@ -1012,6 +1031,9 @@ vzDomainUndefineFlags(virDomainPtr domain,
         goto cleanup;
     job = true;
 
+    if (!vzDomainObjIsExist(dom))
+        goto cleanup;
+
     ret = prlsdkUnregisterDomain(privconn->driver, dom, flags);
 
  cleanup:
@@ -1069,6 +1091,9 @@ vzDomainManagedSave(virDomainPtr domain, unsigned int flags)
         goto cleanup;
     job = true;
 
+    if (!vzDomainObjIsExist(dom))
+        goto cleanup;
+
     state = virDomainObjGetState(dom, &reason);
 
     if (state == VIR_DOMAIN_RUNNING && (flags & VIR_DOMAIN_SAVE_PAUSED)) {
@@ -1159,6 +1184,9 @@ static int vzDomainAttachDeviceFlags(virDomainPtr domain, const char *xml,
         goto cleanup;
     job = true;
 
+    if (!vzDomainObjIsExist(dom))
+        goto cleanup;
+
     switch (dev->type) {
     case VIR_DOMAIN_DEVICE_DISK:
         ret = prlsdkAttachVolume(privconn->driver, dom, dev->data.disk);
@@ -1227,6 +1255,9 @@ static int vzDomainDetachDeviceFlags(virDomainPtr domain, const char *xml,
         goto cleanup;
     job = true;
 
+    if (!vzDomainObjIsExist(dom))
+        goto cleanup;
+
     switch (dev->type) {
     case VIR_DOMAIN_DEVICE_DISK:
         ret = prlsdkDetachVolume(dom, dev->data.disk);
@@ -1284,6 +1315,9 @@ vzDomainSetUserPassword(virDomainPtr domain,
         goto cleanup;
     job = true;
 
+    if (!vzDomainObjIsExist(dom))
+        goto cleanup;
+
     ret = prlsdkDomainSetUserPassword(dom, user, password);
 
  cleanup:
@@ -1617,6 +1651,9 @@ static int vzDomainSetMemoryFlagsImpl(virDomainPtr domain, unsigned long memory,
         goto cleanup;
     job = true;
 
+    if (!vzDomainObjIsExist(dom))
+        goto cleanup;
+
     ret = prlsdkSetMemsize(dom, memory >> 10);
 
  cleanup:
@@ -2101,6 +2138,9 @@ vzDomainSnapshotCreateXML(virDomainPtr domain,
         goto cleanup;
     job = true;
 
+    if (!vzDomainObjIsExist(dom))
+        goto cleanup;
+
     /* snaphot name is ignored, it will be set to auto generated by sdk uuid */
     if (prlsdkCreateSnapshot(dom, def->description) < 0)
         goto cleanup;
@@ -2162,6 +2202,9 @@ vzDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, unsigned int flags)
         goto cleanup;
     job = true;
 
+    if (!vzDomainObjIsExist(dom))
+        goto cleanup;
+
     ret = prlsdkSwitchToSnapshot(dom, snapshot->name,
                                  flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED);
  cleanup:
@@ -2532,6 +2575,9 @@ vzDomainMigratePerformStep(virDomainPtr domain,
         goto cleanup;
     job = true;
 
+    if (!vzDomainObjIsExist(dom))
+        goto cleanup;
+
     if (!(vzuri = vzParseVzURI(miguri)))
         goto cleanup;
 
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 205189a..663e02d 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -1812,6 +1812,9 @@ prlsdkHandleVmConfigEvent(vzDriverPtr driver,
         goto cleanup;
     job = true;
 
+    if (dom->removing)
+        goto cleanup;
+
     if (prlsdkUpdateDomain(driver, dom) < 0)
         goto cleanup;
 
@@ -2078,6 +2081,9 @@ prlsdkDomainChangeState(virDomainPtr domain,
         goto cleanup;
     job = true;
 
+    if (dom->removing)
+        goto cleanup;
+
     ret = prlsdkDomainChangeStateLocked(privconn->driver, dom, chstate);
 
  cleanup:
@@ -3952,6 +3958,20 @@ prlsdkUnregisterDomain(vzDriverPtr driver, virDomainObjPtr dom, unsigned int fla
     return ret;
 }
 
+static void
+vzDomainObjCheckExist(virDomainObjPtr dom)
+{
+    char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+    if (!dom->removing)
+        return;
+
+    virUUIDFormat(dom->def->uuid, uuidstr);
+    virReportError(VIR_ERR_NO_DOMAIN,
+                   _("no domain with matching uuid '%s' (%s)"),
+                   uuidstr, dom->def->name);
+}
+
 int
 prlsdkDomainManagedSaveRemove(virDomainObjPtr dom)
 {
@@ -3959,8 +3979,10 @@ prlsdkDomainManagedSaveRemove(virDomainObjPtr dom)
     PRL_HANDLE job;
 
     job = PrlVm_DropSuspendedState(privdom->sdkdom);
-    if (PRL_FAILED(waitDomainJob(job, dom)))
+    if (PRL_FAILED(waitDomainJob(job, dom))) {
+        vzDomainObjCheckExist(dom);
         return -1;
+    }
 
     return 0;
 }
@@ -4394,8 +4416,10 @@ prlsdkLoadSnapshots(virDomainObjPtr dom)
     char *treexml = NULL;
 
     job = PrlVm_GetSnapshotsTreeEx(privdom->sdkdom, PGST_WITHOUT_SCREENSHOTS);
-    if (PRL_FAILED(getDomainJobResult(job, dom, &result)))
+    if (PRL_FAILED(getDomainJobResult(job, dom, &result))) {
+        vzDomainObjCheckExist(dom);
         goto cleanup;
+    }
 
     if (!(treexml = prlsdkGetStringParamVar(PrlResult_GetParamAsString, result)))
         goto cleanup;
@@ -4427,8 +4451,10 @@ int prlsdkDeleteSnapshot(virDomainObjPtr dom, const char *uuid, bool children)
     PRL_HANDLE job;
 
     job = PrlVm_DeleteSnapshot(privdom->sdkdom, uuid, children);
-    if (PRL_FAILED(waitDomainJob(job, dom)))
+    if (PRL_FAILED(waitDomainJob(job, dom))) {
+        vzDomainObjCheckExist(dom);
         return -1;
+    }
 
     return 0;
 }
-- 
1.8.3.1




More information about the libvir-list mailing list