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 <nshirokovs...@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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to