[libvirt] [PATCH v2 1/6] 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 <jferlan(a)redhat.com&gt; --- src/storage/storage_backend_logical.c | 208 ++++++++++++++++++---------------- 1 file changed, 113 insertions(+), 95 deletions(-) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 944be40..7c05b6a 100644 --- a/src/storage/storage_backend_logical.c +++ b/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,6 +184,112 @@ 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; @@ -276,9 +297,6 @@ virStorageBackendLogicalMakeVol(char **const groups, ret = 0; cleanup: - VIR_FREE(regex); - VIR_FREE(reg); - VIR_FREE(vars); if (is_new_vol && (ret == -1)) virStorageVolDefFree(vol); return ret; -- 2.5.0