Re: [PATCH] parallels: Cleanup partly opened drivers on connect open failure

Maxim Nestratov <mnestratov@xxxxxxxxxxxxx> · Wed, 4 Mar 2015 11:52:34 +0300

26.02.2015 19:29, Michal Privoznik пишет:
Well, the parallelsConnectOpen() joins several sub-driver openings
into one big if condition. If any of sub-driver fails to open, the
whole API finishes immediately. The problem is, sub-drivers may have
left some memory allocated. Fortunately, we have a free function for
that: parallelsConnectClose(). This is, however, not prepared for
partially allocated driver structure. So, prepare the free function
for it and call it at the right place, in the if body.

Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---

This commit assumes that virNetworkObjListFree() accepts NULL,
and thus depends on this patch (which has been ACKed already):

https://www.redhat.com/archives/libvir-list/2015-February/msg01127.html

Should I push both during freeze or wait until after the release?

  src/parallels/parallels_driver.c  | 7 ++++++-
  src/parallels/parallels_network.c | 4 ++++
  src/parallels/parallels_storage.c | 6 ++++++
  3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index c9338b5..650b790 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -264,8 +264,10 @@ parallelsConnectOpen(virConnectPtr conn,
  
      if ((ret = parallelsOpenDefault(conn)) != VIR_DRV_OPEN_SUCCESS ||
          (ret = parallelsStorageOpen(conn, flags)) != VIR_DRV_OPEN_SUCCESS ||
-        (ret = parallelsNetworkOpen(conn, flags)) != VIR_DRV_OPEN_SUCCESS)
+        (ret = parallelsNetworkOpen(conn, flags)) != VIR_DRV_OPEN_SUCCESS) {
+        parallelsConnectClose(conn);
          return ret;
+    }
  
      return VIR_DRV_OPEN_SUCCESS;
  }
@@ -275,6 +277,9 @@ parallelsConnectClose(virConnectPtr conn)
  {
      parallelsConnPtr privconn = conn->privateData;
  
+    if (!privconn)
+        return 0;
+
      parallelsNetworkClose(conn);
      parallelsStorageClose(conn);
  
diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c
index 3e7087d..c5834c5 100644
--- a/src/parallels/parallels_network.c
+++ b/src/parallels/parallels_network.c
@@ -336,6 +336,10 @@ parallelsNetworkOpen(virConnectPtr conn,
  int parallelsNetworkClose(virConnectPtr conn)
  {
      parallelsConnPtr privconn = conn->privateData;
+
+    if (!privconn)
+        return 0;
+
      parallelsDriverLock(privconn);
      virNetworkObjListFree(&privconn->networks);
      parallelsDriverUnlock(privconn);
diff --git a/src/parallels/parallels_storage.c b/src/parallels/parallels_storage.c
index d2c5bf2..56c1b42 100644
--- a/src/parallels/parallels_storage.c
+++ b/src/parallels/parallels_storage.c
@@ -72,9 +72,15 @@ parallelsStorageClose(virConnectPtr conn)
  {
      parallelsConnPtr privconn = conn->privateData;
  
+    if (!privconn)
+        return 0;
+
      virStorageDriverStatePtr storageState = privconn->storageState;
      privconn->storageState = NULL;
  
+    if (!storageState)
+        return 0;
+
      parallelsStorageLock(storageState);
      virStoragePoolObjListFree(&privconn->pools);
      VIR_FREE(storageState->configDir);
Ack and thank you.

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








