Re: [PATCH V5] Sheepdog: Adding volume and on pool and refresh.

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

 



Thx


Le 11/02/2014 12:33, Daniel P. Berrange a écrit :
On Fri, Feb 07, 2014 at 12:45:42PM +0100, joel SIMOES wrote:

+static int
+virStorageBackendSheepdogAddVolume(virConnectPtr conn ATTRIBUTE_UNUSED,
+                                  virStoragePoolObjPtr pool, const char *diskInfo)
+{
+    virStorageVolDefPtr vol = NULL;
+
+    if (diskInfo == NULL)
+        goto error;
Missing error report

+
+    if (VIR_ALLOC(vol) < 0 || VIR_STRDUP(vol->name, diskInfo) < 0)
+        goto error;
+
+    vol->type = VIR_STORAGE_VOL_NETWORK;
+
+    if (VIR_EXPAND_N(pool->volumes.objs, pool->volumes.count, 1) < 0)
+        goto error;
+
+    pool->volumes.objs[pool->volumes.count - 1] = vol;
+
+    if (virStorageBackendSheepdogRefreshVol(conn, pool, vol) < 0)
+        goto error;
If refreshing fails, when we free 'vol', but we've already added
it to the pool->volumes array. So the refresh call needs to be
moved up a few lines.

+
+    return 0;
+
+error:
+    virStorageVolDefFree(vol);
+    return -1;
+}
+
+static int
+virStorageBackendSheepdogRefreshAllVol(virConnectPtr conn ATTRIBUTE_UNUSED,
+                                       virStoragePoolObjPtr pool)
+{
+    int ret = -1;
+    char *output = NULL;
+    char **lines = NULL;
+    char **cells = NULL;
+    size_t i;
+
+    virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "list", "-r", NULL);
+    virStorageBackendSheepdogAddHostArg(cmd, pool);
+    virCommandSetOutputBuffer(cmd, &output);
+    if (virCommandRun(cmd, NULL) < 0)
+        goto cleanup;
+
+    lines = virStringSplit(output, "\n", 0);
+    if (lines == NULL)
+        goto cleanup;
+
+    for (i = 0; lines[i]; i++) {
+        char *line = lines[i];
This could be marked const.

+        if (line == NULL)
+            break;
+
+        cells = virStringSplit(line, " ", 0);
+
+        if (cells != NULL && virStringListLength(cells) > 2) {
+            if (virStorageBackendSheepdogAddVolume(conn, pool, cells[1]) < 0)
+                goto cleanup;
+        }
+
+        virStringFreeList(cells);
+        cells = NULL;
+    }
+
+    ret = 0;
+
+cleanup:
+    virCommandFree(cmd);
+    virStringFreeList(lines);
+    virStringFreeList(cells);
We need VIR_FREE(output) here.

+    return ret;
+}

I'm going to commit this with the following change applied:


diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c
index 08e5473..82df274 100644
--- a/src/storage/storage_backend_sheepdog.c
+++ b/src/storage/storage_backend_sheepdog.c
@@ -115,22 +115,25 @@ virStorageBackendSheepdogAddVolume(virConnectPtr conn ATTRIBUTE_UNUSED,
  {
      virStorageVolDefPtr vol = NULL;
- if (diskInfo == NULL)
+    if (diskInfo == NULL) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Missing disk info when adding volume"));
          goto error;
+    }
if (VIR_ALLOC(vol) < 0 || VIR_STRDUP(vol->name, diskInfo) < 0)
          goto error;
vol->type = VIR_STORAGE_VOL_NETWORK; + if (virStorageBackendSheepdogRefreshVol(conn, pool, vol) < 0)
+        goto error;
+
      if (VIR_EXPAND_N(pool->volumes.objs, pool->volumes.count, 1) < 0)
          goto error;
pool->volumes.objs[pool->volumes.count - 1] = vol; - if (virStorageBackendSheepdogRefreshVol(conn, pool, vol) < 0)
-        goto error;
-
      return 0;
error:
@@ -159,7 +162,7 @@ virStorageBackendSheepdogRefreshAllVol(virConnectPtr conn ATTRIBUTE_UNUSED,
          goto cleanup;
for (i = 0; lines[i]; i++) {
-        char *line = lines[i];
+        const char *line = lines[i];
          if (line == NULL)
              break;
@@ -180,6 +183,7 @@ cleanup:
      virCommandFree(cmd);
      virStringFreeList(lines);
      virStringFreeList(cells);
+    VIR_FREE(output);
      return ret;
  }

Thanks for your contribution !

Regards,
Daniel

--
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]