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

[Joel Simoes]

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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list