Attachment 679629 Details for Bug 896052 – Proposed patch

[patch] Proposed patch

libvirt-storage-Do-not-use-comma-as-seperator-for-lvs-output.patch (text/plain), 10.71 KB, created by J.H.M. Dassen (Ray) on 2013-01-16 14:36:18 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: J.H.M. Dassen (Ray)

Created: 2013-01-16 14:36:18 UTC

Size: 10.71 KB

patch

obsolete

>From a3d7590968d9de8c0e46b80ac05ece78be0608c2 Mon Sep 17 00:00:00 2001
>From: Osier Yang <jyang@redhat.com>
>Date: Mon, 10 Oct 2011 21:33:09 +0800
>Subject: [PATCH] storage: Do not use comma as seperator for lvs output
>To: libvir-list@redhat.com
>
>* 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>
>
>BZ# https://bugzilla.redhat.com/show_bug.cgi?id=727474
>(cherry picked from commit 82c1740ab92682d69ec8f02adb36b13e1902acd1)
>
>Signed-off-by: Daniel Veillard <veillard@redhat.com>
>---
> src/storage/storage_backend_logical.c |  156 +++++++++++++++++++++++++-------
> 1 files changed, 122 insertions(+), 34 deletions(-)
>
>diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
>index ca4166d..909a863 100644
>--- 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
>     };
> 
>-- 
>1.7.4.4
>

Actions: View | Diff

Attachments on bug 896052: 679629