Re: [libvirt] [PATCH v2 1/6] logical: Create helper virStorageBackendLogicalParseVolExtents

[Pavel Hrdina]

On Thu, Jan 28, 2016 at 05:44:04PM -0500, John Ferlan wrote:
> 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 
> ---
>  src/storage/storage_backend_logical.c | 208 
> ++
>  1 file changed, 113 insertions(+), 95 deletions(-)

ACK

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2 1/6] logical: Create helper virStorageBackendLogicalParseVolExtents

[John Ferlan]

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 
---
 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(>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(>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, ) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("malformed volume extent size value"));
 goto cleanup;
 }
-if (virStrToLong_ull(groups[8], NULL, 10, >target.allocation) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("malformed volume allocation