[libvirt] [v2] storage: Do not use comma as seperator for lvs output

Osier Yang jyang at redhat.com
Mon Oct 10 12:37:44 UTC 2011


于 2011年10月10日 18:05, Daniel P. Berrange 写道:
> On Mon, Oct 10, 2011 at 12:28:04PM +0800, Osier Yang wrote:
>> * 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
>>
>> v1 - v2:
>>     v1 just simply changes the seperator into "#".
>>
>> ---
>>   src/storage/storage_backend_logical.c |  156 +++++++++++++++++++++++++--------
>>   1 files changed, 121 insertions(+), 35 deletions(-)
>>
>> diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
>> index 589f486..dda68fd 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
>> @@ -193,15 +279,15 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool,
>>        *    not a suitable separator (rhbz 470693).
>>        */
>>       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
>>       };
>>
> ACK, based on testing this patch with a couple of vol groups, some with
> and some without stripped volumes
>
> Regards,
> Daniel

Thanks, I also did the testing before posted the patch, pushed with the 
comments
added.

Osier




More information about the libvir-list mailing list