logical: Create helper virStorageBackendLogicalParseVolExtents

Create a helper routine in order to parse any extents information
including the extent size, length, and the device string contained
within the generated 'lvs' output string.

A future patch would then be able to avoid the code more cleanly

Signed-off-by: John Ferlan <[email protected]>

Author: jferlan

src/storage/storage_backend_logical.c

@@ -72,98 +72,18 @@ struct virStorageBackendLogicalPoolVolData {
 };
 
 static int
-virStorageBackendLogicalMakeVol(char **const groups,
-                                void *opaque)
+virStorageBackendLogicalParseVolExtents(virStorageVolDefPtr vol,
+                                        char **const groups)
 {
-    struct virStorageBackendLogicalPoolVolData *data = opaque;
-    virStoragePoolObjPtr pool = data->pool;
-    virStorageVolDefPtr vol = NULL;
-    bool is_new_vol = false;
-    unsigned long long offset, size, length;
+    int nextents, ret = -1;
     const char *regex_unit = "(\\S+)\\((\\S+)\\)";
     char *regex = NULL;
     regex_t *reg = NULL;
     regmatch_t *vars = NULL;
     char *p = NULL;
     size_t i;
-    int err, nextents, nvars, ret = -1;
-    const char *attrs = groups[9];
-
-    /* Skip inactive volume */
-    if (attrs[4] != 'a')
-        return 0;
-
-    /*
-     * Skip thin pools(t). These show up in normal lvs output
-     * but do not have a corresponding /dev/$vg/$lv device that
-     * is created by udev. This breaks assumptions in later code.
-     */
-    if (attrs[0] == 't')
-        return 0;
-
-    /* See if we're only looking for a specific volume */
-    if (data->vol != NULL) {
-        vol = data->vol;
-        if (STRNEQ(vol->name, groups[0]))
-            return 0;
-    }
-
-    /* Or filling in more data on an existing volume */
-    if (vol == NULL)
-        vol = virStorageVolDefFindByName(pool, groups[0]);
-
-    /* Or a completely new volume */
-    if (vol == NULL) {
-        if (VIR_ALLOC(vol) < 0)
-            return -1;
-
-        is_new_vol = true;
-        vol->type = VIR_STORAGE_VOL_BLOCK;
-
-        if (VIR_STRDUP(vol->name, groups[0]) < 0)
-            goto cleanup;
-
-    }
-
-    if (vol->target.path == NULL) {
-        if (virAsprintf(&vol->target.path, "%s/%s",
-                        pool->def->target.path, vol->name) < 0)
-            goto cleanup;
-    }
-
-    /* Mark the (s) sparse/snapshot lv, e.g. the lv created using
-     * the --virtualsize/-V option. We've already ignored the (t)hin
-     * pool definition. In the manner libvirt defines these, the
-     * thin pool is hidden to the lvs output, except as the name
-     * in brackets [] described for the groups[1] (backingStore).
-     */
-    if (attrs[0] == 's')
-        vol->target.sparse = true;
-
-    /* Skips the backingStore of lv created with "--virtualsize",
-     * its original device "/dev/$vgname/$lvname_vorigin" is
-     * just for lvm internal use, one should never use it.
-     *
-     * (lvs outputs "[$lvname_vorigin] for field "origin" if the
-     *  lv is created with "--virtualsize").
-     */
-    if (groups[1] && STRNEQ(groups[1], "") && (groups[1][0] != '[')) {
-        if (VIR_ALLOC(vol->target.backingStore) < 0)
-            goto cleanup;
-
-        if (virAsprintf(&vol->target.backingStore->path, "%s/%s",
-                        pool->def->target.path, groups[1]) < 0)
-            goto cleanup;
-
-        vol->target.backingStore->format = VIR_STORAGE_POOL_LOGICAL_LVM2;
-    }
-
-    if (!vol->key && VIR_STRDUP(vol->key, groups[2]) < 0)
-        goto cleanup;
-
-    if (virStorageBackendUpdateVolInfo(vol, false,
-                                       VIR_STORAGE_VOL_OPEN_DEFAULT, 0) < 0)
-        goto cleanup;
+    int err, nvars;
+    unsigned long long offset, size, length;
 
     nextents = 1;
     if (STREQ(groups[4], VIR_STORAGE_VOL_LOGICAL_SEGTYPE_STRIPED)) {
@@ -174,7 +94,7 @@ virStorageBackendLogicalMakeVol(char **const groups,
         }
     }
 
-    /* Finally fill in extents information */
+    /* Allocate and fill in extents information */
     if (VIR_REALLOC_N(vol->source.extents,
                       vol->source.nextent + nextents) < 0)
         goto cleanup;
@@ -184,18 +104,13 @@ virStorageBackendLogicalMakeVol(char **const groups,
                        "%s", _("malformed volume extent length value"));
         goto cleanup;
     }
+
     if (virStrToLong_ull(groups[7], NULL, 10, &size) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        "%s", _("malformed volume extent size value"));
         goto cleanup;
     }
-    if (virStrToLong_ull(groups[8], NULL, 10, &vol->target.allocation) < 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       "%s", _("malformed volume allocation value"));
-        goto cleanup;
-    }
 
-    /* Now parse the "devices" field separately */
     if (VIR_STRDUP(regex, regex_unit) < 0)
         goto cleanup;
 
@@ -269,16 +184,119 @@ virStorageBackendLogicalMakeVol(char **const groups,
         vol->source.nextent++;
     }
 
+    ret = 0;
+
+ cleanup:
+    VIR_FREE(regex);
+    VIR_FREE(reg);
+    VIR_FREE(vars);
+    return ret;
+}
+
+
+static int
+virStorageBackendLogicalMakeVol(char **const groups,
+                                void *opaque)
+{
+    struct virStorageBackendLogicalPoolVolData *data = opaque;
+    virStoragePoolObjPtr pool = data->pool;
+    virStorageVolDefPtr vol = NULL;
+    bool is_new_vol = false;
+    int ret = -1;
+    const char *attrs = groups[9];
+
+    /* Skip inactive volume */
+    if (attrs[4] != 'a')
+        return 0;
+
+    /*
+     * Skip thin pools(t). These show up in normal lvs output
+     * but do not have a corresponding /dev/$vg/$lv device that
+     * is created by udev. This breaks assumptions in later code.
+     */
+    if (attrs[0] == 't')
+        return 0;
+
+    /* See if we're only looking for a specific volume */
+    if (data->vol != NULL) {
+        vol = data->vol;
+        if (STRNEQ(vol->name, groups[0]))
+            return 0;
+    }
+
+    /* Or filling in more data on an existing volume */
+    if (vol == NULL)
+        vol = virStorageVolDefFindByName(pool, groups[0]);
+
+    /* Or a completely new volume */
+    if (vol == NULL) {
+        if (VIR_ALLOC(vol) < 0)
+            return -1;
+
+        is_new_vol = true;
+        vol->type = VIR_STORAGE_VOL_BLOCK;
+
+        if (VIR_STRDUP(vol->name, groups[0]) < 0)
+            goto cleanup;
+
+    }
+
+    if (vol->target.path == NULL) {
+        if (virAsprintf(&vol->target.path, "%s/%s",
+                        pool->def->target.path, vol->name) < 0)
+            goto cleanup;
+    }
+
+    /* Mark the (s) sparse/snapshot lv, e.g. the lv created using
+     * the --virtualsize/-V option. We've already ignored the (t)hin
+     * pool definition. In the manner libvirt defines these, the
+     * thin pool is hidden to the lvs output, except as the name
+     * in brackets [] described for the groups[1] (backingStore).
+     */
+    if (attrs[0] == 's')
+        vol->target.sparse = true;
+
+    /* Skips the backingStore of lv created with "--virtualsize",
+     * its original device "/dev/$vgname/$lvname_vorigin" is
+     * just for lvm internal use, one should never use it.
+     *
+     * (lvs outputs "[$lvname_vorigin] for field "origin" if the
+     *  lv is created with "--virtualsize").
+     */
+    if (groups[1] && STRNEQ(groups[1], "") && (groups[1][0] != '[')) {
+        if (VIR_ALLOC(vol->target.backingStore) < 0)
+            goto cleanup;
+
+        if (virAsprintf(&vol->target.backingStore->path, "%s/%s",
+                        pool->def->target.path, groups[1]) < 0)
+            goto cleanup;
+
+        vol->target.backingStore->format = VIR_STORAGE_POOL_LOGICAL_LVM2;
+    }
+
+    if (!vol->key && VIR_STRDUP(vol->key, groups[2]) < 0)
+        goto cleanup;
+
+    if (virStorageBackendUpdateVolInfo(vol, false,
+                                       VIR_STORAGE_VOL_OPEN_DEFAULT, 0) < 0)
+        goto cleanup;
+
+    if (virStrToLong_ull(groups[8], NULL, 10, &vol->target.allocation) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       "%s", _("malformed volume allocation value"));
+        goto cleanup;
+    }
+
+    if (virStorageBackendLogicalParseVolExtents(vol, groups) < 0)
+        goto cleanup;
+
     if (is_new_vol &&
         VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, vol) < 0)
         goto cleanup;
 
     ret = 0;
 
  cleanup:
-    VIR_FREE(regex);
-    VIR_FREE(reg);
-    VIR_FREE(vars);
     if (is_new_vol && (ret == -1))
         virStorageVolDefFree(vol);
     return ret;