storage: Do not use comma as seperator for lvs output

* src/storage/storage_backend_logical.c:

If a logical vol is created as striped. (e.g. --stripes 3),
the "device" field of lvs output will have multiple fileds which are
seperated by comma. Thus the RE we write in the codes will not
work well anymore. E.g. (lvs output for a stripped vol, uses "#" as
seperator here):

test_stripes##fSLSZH-zAS2-yAIb-n4mV-Al9u-HA3V-oo9K1B#\
/dev/sdc1(10240),/dev/sdd1(0)#42949672960#4194304

The RE we use:

    const char *regexes[] = {
        "^\\s*(\\S+),(\\S*),(\\S+),(\\S+)\\((\\S+)\\),(\\S+),([0-9]+),?\\s*$"
    };

Also the RE doesn't match the "devices" field of striped vol properly,
it contains multiple "device path" and "offset".

This patch mainly does:
    1) Change the seperator into "#"
    2) Change the RE for "devices" field from "(\\S+)\\((\\S+)\\)"
       into "(\\S+)".
    3) Add two new options for lvs command, (segtype, stripes)
    4) Extend the RE to match the value for the two new fields.
    5) Parse the "devices" field seperately in virStorageBackendLogicalMakeVol,
       multiple "extents" info are generated if the vol is striped. The
       number of "extents" is equal to the stripes number of the striped vol.

A incidental fix: (virStorageBackendLogicalMakeVol)
    Free "vol" if it's new created and there is error.

Demo on striped vol with the patch applied:

% virsh vol-dumpxml /dev/test_vg/vol_striped2
<volume>
  <name>vol_striped2</name>
  <key>QuWqmn-kIkZ-IATt-67rc-OWEP-1PHX-Cl2ICs</key>
  <source>
    <device path='/dev/sda5'>
      <extent start='79691776' end='88080384'/>
    </device>
    <device path='/dev/sda6'>
      <extent start='62914560' end='71303168'/>
    </device>
  </source>
  <capacity>8388608</capacity>
  <allocation>8388608</allocation>
  <target>
    <path>/dev/test_vg/vol_striped2</path>
    <permissions>
      <mode>0660</mode>
      <owner>0</owner>
      <group>6</group>
      <label>system_u:object_r:fixed_disk_device_t:s0</label>
    </permissions>
  </target>
</volume>

RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=727474

diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
index 589f486fe7c09fda53b4aa72cff6f8601b02cf8d..6137278950936fbaf0df5de1adc30c173aa8e667 100644 (file)
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -62,13 +62,22 @@ virStorageBackendLogicalSetActive(virStoragePoolObjPtr pool,
 }
 
 
+#define VIR_STORAGE_VOL_LOGICAL_SEGTYPE_STRIPED "striped"
+
 static int
 virStorageBackendLogicalMakeVol(virStoragePoolObjPtr pool,
                                 char **const groups,
                                 void *data)
 {
     virStorageVolDefPtr vol = NULL;
+    bool is_new_vol = false;
     unsigned long long offset, size, length;
+    const char *regex_unit = "(\\S+)\\((\\S+)\\)";
+    char *regex = NULL;
+    regex_t *reg = NULL;
+    regmatch_t *vars = NULL;
+    char *p = NULL;
+    int i, err, nextents, nvars, ret = -1;
 
     /* See if we're only looking for a specific volume */
     if (data != NULL) {
@@ -88,19 +97,18 @@ virStorageBackendLogicalMakeVol(virStoragePoolObjPtr pool,
             return -1;
         }
 
+        is_new_vol = true;
         vol->type = VIR_STORAGE_VOL_BLOCK;
 
         if ((vol->name = strdup(groups[0])) == NULL) {
             virReportOOMError();
-            virStorageVolDefFree(vol);
-            return -1;
+            goto cleanup;
         }
 
         if (VIR_REALLOC_N(pool->volumes.objs,
                           pool->volumes.count + 1)) {
             virReportOOMError();
-            virStorageVolDefFree(vol);
-            return -1;
+            goto cleanup;
         }
         pool->volumes.objs[pool->volumes.count++] = vol;
     }
@@ -109,8 +117,7 @@ virStorageBackendLogicalMakeVol(virStoragePoolObjPtr pool,
         if (virAsprintf(&vol->target.path, "%s/%s",
                         pool->def->target.path, vol->name) < 0) {
             virReportOOMError();
-            virStorageVolDefFree(vol);
-            return -1;
+            goto cleanup;
         }
     }
 
@@ -118,8 +125,7 @@ virStorageBackendLogicalMakeVol(virStoragePoolObjPtr pool,
         if (virAsprintf(&vol->backingStore.path, "%s/%s",
                         pool->def->target.path, groups[1]) < 0) {
             virReportOOMError();
-            virStorageVolDefFree(vol);
-            return -1;
+            goto cleanup;
         }
 
         vol->backingStore.format = VIR_STORAGE_POOL_LOGICAL_LVM2;
@@ -128,47 +134,127 @@ virStorageBackendLogicalMakeVol(virStoragePoolObjPtr pool,
     if (vol->key == NULL &&
         (vol->key = strdup(groups[2])) == NULL) {
         virReportOOMError();
-        return -1;
+        goto cleanup;
     }
 
     if (virStorageBackendUpdateVolInfo(vol, 1) < 0)
-        return -1;
+        goto cleanup;
 
+    nextents = 1;
+    if (STREQ(groups[4], VIR_STORAGE_VOL_LOGICAL_SEGTYPE_STRIPED)) {
+        if (virStrToLong_i(groups[5], NULL, 10, &nextents) < 0) {
+            virStorageReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                                  _("malformed volume extent stripes value"));
+            goto cleanup;
+        }
+    }
 
     /* Finally fill in extents information */
     if (VIR_REALLOC_N(vol->source.extents,
-                      vol->source.nextent + 1) < 0) {
+                      vol->source.nextent + nextents) < 0) {
         virReportOOMError();
-        return -1;
+        goto cleanup;
     }
 
-    if ((vol->source.extents[vol->source.nextent].path =
-         strdup(groups[3])) == NULL) {
+    if (virStrToLong_ull(groups[6], NULL, 10, &length) < 0) {
+        virStorageReportError(VIR_ERR_INTERNAL_ERROR,
+                              "%s", _("malformed volume extent length value"));
+        goto cleanup;
+    }
+    if (virStrToLong_ull(groups[7], NULL, 10, &size) < 0) {
+        virStorageReportError(VIR_ERR_INTERNAL_ERROR,
+                              "%s", _("malformed volume extent size value"));
+        goto cleanup;
+    }
+
+    /* Now parse the "devices" feild seperately */
+    regex = strdup(regex_unit);
+
+    for (i = 1; i < nextents; i++) {
+        if (VIR_REALLOC_N(regex, strlen(regex) + strlen(regex_unit) + 2) < 0) {
+            virReportOOMError();
+            goto cleanup;
+        }
+        /* "," is the seperator of "devices" field */
+        strcat(regex, ",");
+        strncat(regex, regex_unit, strlen(regex_unit));
+    }
+
+    if (VIR_ALLOC(reg) < 0) {
         virReportOOMError();
-        return -1;
+        goto cleanup;
     }
 
-    if (virStrToLong_ull(groups[4], NULL, 10, &offset) < 0) {
-        virStorageReportError(VIR_ERR_INTERNAL_ERROR,
-                              "%s", _("malformed volume extent offset value"));
-        return -1;
+    /* Each extent has a "path:offset" pair, and vars[0] will
+     * be the whole matched string.
+     */
+    nvars = (nextents * 2) + 1;
+    if (VIR_ALLOC_N(vars, nvars) < 0) {
+        virReportOOMError();
+        goto cleanup;
     }
-    if (virStrToLong_ull(groups[5], NULL, 10, &length) < 0) {
+
+    err = regcomp(reg, regex, REG_EXTENDED);
+    if (err != 0) {
+        char error[100];
+        regerror(err, reg, error, sizeof(error));
         virStorageReportError(VIR_ERR_INTERNAL_ERROR,
-                              "%s", _("malformed volume extent length value"));
-        return -1;
+                              _("Failed to compile regex %s"),
+                              error);
+        goto cleanup;
     }
-    if (virStrToLong_ull(groups[6], NULL, 10, &size) < 0) {
-        virStorageReportError(VIR_ERR_INTERNAL_ERROR,
-                              "%s", _("malformed volume extent size value"));
-        return -1;
+
+    if (regexec(reg, groups[3], nvars, vars, 0) != 0) {
+        virStorageReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                              _("malformed volume extent devices value"));
+        goto cleanup;
     }
 
-    vol->source.extents[vol->source.nextent].start = offset * size;
-    vol->source.extents[vol->source.nextent].end = (offset * size) + length;
-    vol->source.nextent++;
+    p = groups[3];
 
-    return 0;
+    /* vars[0] is skipped */
+    for (i = 0; i < nextents; i++) {
+        int j, len;
+        const char *offset_str = NULL;
+
+        j = (i * 2) + 1;
+        len = vars[j].rm_eo - vars[j].rm_so;
+        p[vars[j].rm_eo] = '\0';
+
+        if ((vol->source.extents[vol->source.nextent].path =
+            strndup(p + vars[j].rm_so, len)) == NULL) {
+            virReportOOMError();
+            goto cleanup;
+        }
+
+        len = vars[j + 1].rm_eo - vars[j + 1].rm_so;
+        if (!(offset_str = strndup(p + vars[j + 1].rm_so, len))) {
+            virReportOOMError();
+            goto cleanup;
+        }
+
+        if (virStrToLong_ull(offset_str, NULL, 10, &offset) < 0) {
+            virStorageReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                                  _("malformed volume extent offset value"));
+            goto cleanup;
+        }
+
+        VIR_FREE(offset_str);
+
+        vol->source.extents[vol->source.nextent].start = offset * size;
+        vol->source.extents[vol->source.nextent].end = (offset * size) + length;
+        vol->source.nextent++;
+    }
+
+    ret = 0;
+
+cleanup:
+    VIR_FREE(regex);
+    VIR_FREE(reg);
+    VIR_FREE(vars);
+    if (is_new_vol && (ret == -1))
+        virStorageVolDefFree(vol);
+    return ret;
 }
 
 static int
@@ -191,17 +277,19 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool,
      *
      * NB Encrypted logical volumes can print ':' in their name, so it is
      *    not a suitable separator (rhbz 470693).
+     * NB "devices" field has multiple device paths and "," if the volume is
+     *    striped, so "," is not a suitable separator either (rhbz 727474).
      */
     const char *regexes[] = {
-        "^\\s*(\\S+),(\\S*),(\\S+),(\\S+)\\((\\S+)\\),(\\S+),([0-9]+),?\\s*$"
+       "^\\s*(\\S+)#(\\S*)#(\\S+)#(\\S+)#(\\S+)#([0-9]+)#(\\S+)#([0-9]+)#?\\s*$"
     };
     int vars[] = {
-        7
+        8
     };
     const char *prog[] = {
-        LVS, "--separator", ",", "--noheadings", "--units", "b",
+        LVS, "--separator", "#", "--noheadings", "--units", "b",
         "--unbuffered", "--nosuffix", "--options",
-        "lv_name,origin,uuid,devices,seg_size,vg_extent_size",
+        "lv_name,origin,uuid,devices,segtype,stripes,seg_size,vg_extent_size",
         pool->def->source.name, NULL
     };