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

Re: [libvirt] [PATCH V3]Sheepdog: Auto Adding volume and pool refresh.



Howto check size of **char

Thx.

+        char *line = *(lines + i);
+        cells = virStringSplit(line, " ", 0);
+        size_t j;
+        for (j = 0; *(cells + j); j++) {

You can eliminate this whole loop by checking if cells has at least two
entries and using cells[1] directly.


Le 03/02/2014 14:07, Ján Tomko a écrit :
On 01/31/2014 01:02 PM, joel SIMOES wrote:
From: Joel <joel simoes laposte net>

Fix bug 1055479
https://bugzilla.redhat.com/show_bug.cgi?id=1055479
Libvirt lose sheepdogs volumes on pool refresh or restart.
When restarting sheepdog pool, all volumes are missing.
This patch add automatically all volume from the added pool.
---
  src/storage/storage_backend_sheepdog.c | 74 +++++++++++++++++++++++++++++++++-
  1 file changed, 72 insertions(+), 2 deletions(-)

diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c
index a6981ce..a836f27 100644
--- a/src/storage/storage_backend_sheepdog.c
+++ b/src/storage/storage_backend_sheepdog.c
@@ -109,6 +109,70 @@ virStorageBackendSheepdogAddHostArg(virCommandPtr cmd,
      virCommandAddArgFormat(cmd, "%d", port);
  }
+static int
+virStorageBackendSheepdogRefreshAllVol(virConnectPtr conn ATTRIBUTE_UNUSED,
+                                       virStoragePoolObjPtr pool)
+{
+    int ret;
int ret = -1;

+    char *output = NULL;
'lines' and 'cells' should be declared here as well, not below the virCommand
calls.

+
+    virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "list", "-r", NULL);
+    virStorageBackendSheepdogAddHostArg(cmd, pool);
+    virCommandSetOutputBuffer(cmd, &output);
+    ret = virCommandRun(cmd, NULL);
No need to save the return value, see [1]

+    char** lines;
+    char** cells;
+
+    if (ret < 0)
+        goto cleanup;
+
+    ret = -1;
+    lines = virStringSplit(output, "\n", 0);
The return value of virStringSplit needs to be checked for NULL.

+    size_t i;
This should be declared at the beginning of the function.

+    for (i = 0; *(lines + i); i++) {
lines[i] looks nicer than *(lines + i), see [2]

+        char *line = *(lines + i);
+        cells = virStringSplit(line, " ", 0);
+        size_t j;
+        for (j = 0; *(cells + j); j++) {
You can eliminate this whole loop by checking if cells has at least two
entries and using cells[1] directly.

+
+            char *cell = *(cells + j);
+            if (j == 1) {
+                virStorageVolDefPtr vol = NULL;
+                if (VIR_ALLOC(vol) < 0)
+                    goto cleanup;
+
+                if (VIR_STRDUP(vol->name, cell) < 0)
+                    goto cleanup;
+
+                vol->type = VIR_STORAGE_VOL_BLOCK;
+
+                if (VIR_EXPAND_N(pool->volumes.objs, pool->volumes.count, 1) < 0)
+                    goto cleanup;
+                pool->volumes.objs[pool->volumes.count - 1] = vol;
+
+                if (virStorageBackendSheepdogRefreshVol(conn, pool, vol) < 0)
+                    goto cleanup;
+
+                vol = NULL;
+            }
+
+            VIR_FREE(*(cells + j));
No need to free every string separately,

+        }
+
+        VIR_FREE(cells);
use virStringFreeList(cells) instead.

+        VIR_FREE(*(lines + i));
Same here.

+    }
+
+
+    VIR_FREE(lines);
+    ret = 0;
+
+cleanup:
+    virCommandFree(cmd);
+    VIR_FREE(lines);
+    VIR_FREE(cells);
+    return ret;
+}
static int
  virStorageBackendSheepdogRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED,
@@ -122,9 +186,15 @@ virStorageBackendSheepdogRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED,
      virStorageBackendSheepdogAddHostArg(cmd, pool);
      virCommandSetOutputBuffer(cmd, &output);
      ret = virCommandRun(cmd, NULL);
-    if (ret == 0)
-        ret = virStorageBackendSheepdogParseNodeInfo(pool->def, output);
+    if (ret < 0)
+        goto cleanup;
+ ret = virStorageBackendSheepdogParseNodeInfo(pool->def, output);
+    if (ret < 0)
+        goto cleanup;
There's no need to overwrite the 'ret' variable with every call.
if (functionCall() < 0)
   goto cleanup;

works too, since most of the functions only return 0 and -1.

+    ret = virStorageBackendSheepdogRefreshAllVol(conn, pool);
+
+cleanup:
      virCommandFree(cmd);
      VIR_FREE(output);
      return ret;

Jan

[1] https://www.redhat.com/archives/libvir-list/2014-January/msg01232.html
[2] https://www.redhat.com/archives/libvir-list/2014-January/msg01215.html



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