[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/3] parallels: move up updating autostart parameter in prlsdkLoadDomain



21.05.2015 9:55, Dmitry Guryanov пишет:


On 05/18/2015 05:44 PM, Maxim Nestratov wrote:
It is better to get all necessary parameters and check them on newly
created configuration before actually creating a domain with them or
applying them to an existing domain.

What problem could it cause?
First of all if we ask something from parallels dispatcher it takes time and doing such things with locked domain isn't a good practice. I am not saying that PrlVmCfg_GetAutoStart will go to dispatcher but other SDK functions will. What I wanted here is to group SDK calls together. Secondly, there can be a race when we call prlsdkLoadDomain function from domain creation event handler after the domain is deleted by concurrent extern call. So it's better to fail before we actually create a domain.

Signed-off-by: Maxim Nestratov <mnestratov parallels com>
---
  src/parallels/parallels_sdk.c | 18 +++++++++---------
  1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index faae1ae..5c15e94 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -1312,6 +1312,9 @@ prlsdkLoadDomain(parallelsConnPtr privconn,
              *s = '\0';
      }
  +    pret = PrlVmCfg_GetAutoStart(sdkdom, &autostart);
+    prlsdkCheckRetGoto(pret, error);
+
      if (virDomainDefAddImplicitControllers(def) < 0)
          goto error;
  @@ -1349,15 +1352,6 @@ prlsdkLoadDomain(parallelsConnPtr privconn,
      dom->privateDataFreeFunc = prlsdkDomObjFreePrivate;
      dom->persistent = 1;
  -    if (prlsdkGetDomainState(sdkdom, &domainState) < 0)
-        goto error;
-
-    if (prlsdkConvertDomainState(domainState, envId, dom) < 0)
-        goto error;
-
-    pret = PrlVmCfg_GetAutoStart(sdkdom, &autostart);
-    prlsdkCheckRetGoto(pret, error);
-
      switch (autostart) {
      case PAO_VM_START_ON_LOAD:
          dom->autostart = 1;
@@ -1371,6 +1365,12 @@ prlsdkLoadDomain(parallelsConnPtr privconn,
          goto error;
      }
  +    if (prlsdkGetDomainState(sdkdom, &domainState) < 0)
+        goto error;
+

Why don't you move this ^^ call?

Makes sence even more than PrlVmCfg_GetAutoStart. Just missed it.
I'll add it in the second version of the patchset.

+    if (prlsdkConvertDomainState(domainState, envId, dom) < 0)
+        goto error;
+
      if (!pdom->sdkdom) {
          pret = PrlHandle_AddRef(sdkdom);
          prlsdkCheckRetGoto(pret, error);



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]