A thin lv doesn't have any extents defined - rather it uses a concept of a "thin-pool" in order to describe it's source. In order to allow that to be displayed properly in a future patch, add a new 'thin_pool' name that can be used by a future patch to store the name of the source thin_pool name used by a thin logical volume. A thin_pool has a specific size, so add a thin_capacity field to store that. For now, these are "output only" fields. That is - if a thin lv is found in a volume group, we can display the thinpool data. The output will appear in a vol-dumpxml (similar to how extents are handled) as: <source> <thinpool name='thinmints'> <capacity unit='bytes'>20971520</capacity> </thinpool> </source> The new tests add the "bones" in order to at some point in a future patch add the ability to parse the input XML with a <thinpool> and generate a thinpool within the volume group. Although the storagevolxml2xmltest added valid input (so that storagevolschematest can validate the format), the output will not be listed, thus the storagevolxml2xmlout does not have the source data (similar to vol-logical.xml). Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- docs/schemas/storagevol.rng | 14 ++++++++++++++ src/conf/storage_conf.c | 11 +++++++++++ src/conf/storage_conf.h | 3 +++ tests/storagevolxml2xmlin/vol-logical-thin.xml | 20 ++++++++++++++++++++ tests/storagevolxml2xmlout/vol-logical-thin.xml | 17 +++++++++++++++++ tests/storagevolxml2xmltest.c | 1 + 6 files changed, 66 insertions(+) create mode 100644 tests/storagevolxml2xmlin/vol-logical-thin.xml create mode 100644 tests/storagevolxml2xmlout/vol-logical-thin.xml diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng index 7450547..25e2db7 100644 --- a/docs/schemas/storagevol.rng +++ b/docs/schemas/storagevol.rng @@ -144,6 +144,9 @@ <zeroOrMore> <ref name='sourcedev'/> </zeroOrMore> + <optional> + <ref name='sourcethinpool'/> + </optional> </element> </define> @@ -172,6 +175,17 @@ </oneOrMore> </define> + <define name='sourcethinpool'> + <element name='thinpool'> + <attribute name='name'> + <text/> + </attribute> + <element name='capacity'> + <ref name='scaledInteger'/> + </element> + </element> + </define> + <define name='formatdev'> <choice> <value>none</value> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 3657dfd..8eef399 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -322,6 +322,7 @@ virStorageVolDefFree(virStorageVolDefPtr def) for (i = 0; i < def->source.nextent; i++) VIR_FREE(def->source.extents[i].path); VIR_FREE(def->source.extents); + VIR_FREE(def->source.thin_pool); virStorageSourceClear(&def->target); VIR_FREE(def); @@ -1655,6 +1656,16 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool, virBufferAddLit(&buf, "</device>\n"); } + if (def->source.thin_pool) { + virBufferEscapeString(&buf, "<thinpool name='%s'>\n", + def->source.thin_pool); + virBufferAdjustIndent(&buf, 2); + virBufferAsprintf(&buf, "<capacity unit='bytes'>%llu</capacity>\n", + def->source.thin_capacity); + virBufferAdjustIndent(&buf, -2); + virBufferAddLit(&buf, "</thinpool>\n"); + } + virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</source>\n"); diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 31b45be..f19cb59 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -55,6 +55,9 @@ struct _virStorageVolSource { int partType; /* virStorageVolTypeDisk, only used by disk * backend for partition type creation */ + + char *thin_pool; /* For a thin volume, the thin-pool and capacity */ + unsigned long long thin_capacity; /* bytes */ }; diff --git a/tests/storagevolxml2xmlin/vol-logical-thin.xml b/tests/storagevolxml2xmlin/vol-logical-thin.xml new file mode 100644 index 0000000..7d97627 --- /dev/null +++ b/tests/storagevolxml2xmlin/vol-logical-thin.xml @@ -0,0 +1,20 @@ +<volume type='block'> + <name>thinmint1</name> + <key>r4xkCv-MQhr-WKIT-R66x-Epn2-e8hG-1Z5gY0</key> + <source> + <thinpool name='thinmints'> + <capacity unit='bytes'>20971520</capacity> + </thinpool> + </source> + <capacity unit='bytes'>2080374784</capacity> + <allocation unit='bytes'>2080374784</allocation> + <target> + <path>/dev/HostVG/thinmints</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> diff --git a/tests/storagevolxml2xmlout/vol-logical-thin.xml b/tests/storagevolxml2xmlout/vol-logical-thin.xml new file mode 100644 index 0000000..198d3ea --- /dev/null +++ b/tests/storagevolxml2xmlout/vol-logical-thin.xml @@ -0,0 +1,17 @@ +<volume type='block'> + <name>thinmint1</name> + <key>r4xkCv-MQhr-WKIT-R66x-Epn2-e8hG-1Z5gY0</key> + <source> + </source> + <capacity unit='bytes'>2080374784</capacity> + <allocation unit='bytes'>2080374784</allocation> + <target> + <path>/dev/HostVG/thinmints</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> diff --git a/tests/storagevolxml2xmltest.c b/tests/storagevolxml2xmltest.c index 148d1e6..197d308 100644 --- a/tests/storagevolxml2xmltest.c +++ b/tests/storagevolxml2xmltest.c @@ -107,6 +107,7 @@ mymain(void) DO_TEST("pool-dir", "vol-qcow2-nobacking"); DO_TEST("pool-disk", "vol-partition"); DO_TEST("pool-logical", "vol-logical"); + DO_TEST("pool-logical", "vol-logical-thin"); DO_TEST("pool-logical", "vol-logical-backing"); DO_TEST("pool-sheepdog", "vol-sheepdog"); DO_TEST("pool-gluster", "vol-gluster-dir"); -- 2.5.0

During processing of the extents found in a pool, we have historically ignored the thin-pool which means any thin lv found in the pool would also be ignored. This can start to change now - we can save aside the name and capacity of any thin-pool's found so that we can use that when we find a thin lv and fill in the thin-pool capacity value on output. The result will end up being the following for a vol-dumpxml: <source> <thinpool name='thinmints'/> <capacity unit='bytes'>20971520</capacity> </thinpool> </source> instead of an empty <source> </source> pair. An upcoming patch will allow a thin lv to be seen Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/conf/storage_conf.h | 12 +++++++++ src/storage/storage_backend_logical.c | 51 +++++++++++++++++++++++++++++++++-- 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index f19cb59..a9a1288 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -47,6 +47,18 @@ struct _virStorageVolSourceExtent { unsigned long long end; }; +/* + * How to represent thin-pool's for a logical volume. Used by the + * logical parsing code + */ +typedef struct _virStorageVolSourceThinPool virStorageVolSourceThinPool; +typedef virStorageVolSourceThinPool *virStorageVolSourceThinPoolPtr; +struct _virStorageVolSourceThinPool { + char *name; + unsigned long long capacity; +}; + + typedef struct _virStorageVolSource virStorageVolSource; typedef virStorageVolSource *virStorageVolSourcePtr; struct _virStorageVolSource { diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 3044853..d7990e2 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -72,6 +72,8 @@ virStorageBackendLogicalSetActive(virStoragePoolObjPtr pool, struct virStorageBackendLogicalPoolVolData { virStoragePoolObjPtr pool; virStorageVolDefPtr vol; + size_t nthinpools; + virStorageVolSourceThinPoolPtr thinpools; }; static int @@ -221,12 +223,38 @@ virStorageBackendLogicalMakeVol(char **const groups, return 0; /* - * Skip thin pools(t). These show up in normal lvs output + * Save the 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. + * The thin pools are the "capacity container" for all the thin + * lv's found in the pool. A thin lv provides a virtual size upon + * creation and can appear to over subscribe the pool capacity. + * Although usually thin pools are listed before thin lv's in the + * output, we'll just wait until all volumes are processed and + * then match the name saved here with thin_pool name we saved + * earlier to get the capacity value of thin-pool into the volume. + * It's expected that there is more than 1 thin lv per thin-pool. + * There can be more than 1 thin-pool per volume group. */ - if (attrs[0] == 't') + if (attrs[0] == 't') { + virStorageVolSourceThinPool thin; + + memset(&thin, 0, sizeof(thin)); + if (VIR_STRDUP(thin.name, groups[0]) < 0) + goto cleanup; + if (virStrToLong_ull(groups[8], NULL, 10, &thin.capacity) < 0) { + VIR_FREE(thin.name); + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("malformed thin pool capacity value")); + goto cleanup; + } + if (VIR_APPEND_ELEMENT(data->thinpools, data->nthinpools, thin) < 0) { + VIR_FREE(thin.name); + goto cleanup; + } + return 0; + } /* See if we're only looking for a specific volume */ if (data->vol != NULL) { @@ -389,7 +417,10 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, struct virStorageBackendLogicalPoolVolData cbdata = { .pool = pool, .vol = vol, + .nthinpools = 0, + .thinpools = NULL, }; + size_t i, j; cmd = virCommandNewArgList(LVS, "--separator", "#", @@ -410,9 +441,25 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, "lvs") < 0) goto cleanup; + /* If we find some thin-pools during processing, let's see if we + * need information from them for any thin lv's in the pool + */ + for (i = 0; i < cbdata.nthinpools; i++) { + for (j = 0; j < pool->volumes.count; j++) { + if (STREQ_NULLABLE(pool->volumes.objs[j]->source.thin_pool, + cbdata.thinpools[i].name)) { + pool->volumes.objs[j]->source.thin_capacity = + cbdata.thinpools[i].capacity; + } + } + } + ret = 0; cleanup: virCommandFree(cmd); + for (i = 0; i < cbdata.nthinpools; i++) + VIR_FREE(cbdata.thinpools[i].name); + VIR_FREE(cbdata.thinpools); return ret; } -- 2.5.0

Modify the regex for the 'devices' (a/k/a 'extents') from "(\\S+)" (e.g., 1 or more) to "(\\S*)" (e.g., zero or more). Then for any "thin" lv's found, mark the volume as a sparse volume so that the volume wipe algorithm doesn't work. Since a "thin" segtype has no devices, this will result in any "thin" lv part of some thin-pool within a volume group used as a libvirt pool to be displayed as a possible volume to use. NB: Based on a proposal authored by Joe Harvell <joe.harvell(a)tekcomms.com&gt;, but with much intervening rework, the resulting patch is changed from the original concept. About all that remains is changing the regex and checking for NULL/empty field during parse. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- docs/formatstorage.html.in | 9 +++++++-- src/storage/storage_backend_logical.c | 22 ++++++++++++++-------- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index 4965a4c..2f4662c 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -566,8 +566,13 @@ <span class="since">Since 0.4.1</span></dd> <dt><code>source</code></dt> <dd>Provides information about the underlying storage allocation - of the volume. This may not be available for some pool types. - <span class="since">Since 0.4.1</span></dd> + of the volume. This is available logical pool types to display + details of logical volume extent information + (<span class="since">Since 0.4.1</span>) + or the name of the name of the thin-pool used by the thin + logical volume + (<span class="since">Since 1.3.2</span>). + </dd> <dt><code>target</code></dt> <dd>Provides information about the representation of the volume on the local host. <span class="since">Since 0.4.1</span></dd> diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index d7990e2..15fa228 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -91,9 +91,13 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDefPtr vol, unsigned long long offset, size, length; virStorageVolSourceExtent extent; + /* If the devices field is NULL or empty, then there's nothing to do */ + if (!groups[3] || !*groups[3]) + return 0; + memset(&extent, 0, sizeof(extent)); - /* Assume 1 extent (the regex for 'devices' is "(\\S+)") and only + /* Assume 1 extent (since we checked for NULL or empty above) and only * check the 'stripes' field if we have a striped, mirror, or one of * the raid (raid1, raid4, raid5*, raid6*, or raid10) segtypes in which * case the stripes field will denote the number of lv's within the @@ -286,13 +290,15 @@ virStorageBackendLogicalMakeVol(char **const groups, 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). + /* Mark the (s) sparse/snapshot or the (V) thin/thin-pool member lv, + * e.g. the lv created using the --virtualsize/-V option to ensure + * the volume wipe algorithm doesn't overwrite sparse/thin volumes. + * 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') + if (attrs[0] == 's' || attrs[0] == 'V') vol->target.sparse = true; /* Skips the backingStore of lv created with "--virtualsize", @@ -350,7 +356,7 @@ virStorageBackendLogicalMakeVol(char **const groups, #define VIR_STORAGE_VOL_LOGICAL_LV_NAME_REGEX "(\\S+)#" #define VIR_STORAGE_VOL_LOGICAL_ORIGIN_REGEX "(\\S*)#" #define VIR_STORAGE_VOL_LOGICAL_UUID_REGEX "(\\S+)#" -#define VIR_STORAGE_VOL_LOGICAL_DEVICES_REGEX "(\\S+)#" +#define VIR_STORAGE_VOL_LOGICAL_DEVICES_REGEX "(\\S*)#" #define VIR_STORAGE_VOL_LOGICAL_SEGTYPE_REGEX "(\\S+)#" #define VIR_STORAGE_VOL_LOGICAL_STRIPES_REGEX "([0-9]+)#" #define VIR_STORAGE_VOL_LOGICAL_SEG_SIZE_REGEX "(\\S+)#" -- 2.5.0