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

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

 



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@xxxxxxxxxxxxx>
---
  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);


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





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]