1:10 p.m.
The first patch just changes 'building' into a bool as that's how I
found it used when going to add a new bool for patch 2
Patch 2 addresses the following bz:
https://bugzilla.redhat.com/show_bug.cgi?id=1091866
Essentially, wiping the lv caused it to disappear after a vol-refresh.
This was because the lv that was created was a sparse (or thin or
snapshot via --virtualsize of -V) logical volume. The multiple names
are the ways I found the feature described. When the sparse volume
was written to by the wipe algorithms it actually filled the volume
beyond it's capacity rendering it INACTIVE which causes pool-refresh
to ignore it. As it turns out a sparse lv created of size (for example)
4 MiB has metadata contained within the size created. While we could
adjust the size of the metadata, control over it's location becomes
the issue. It seems there is also some guard data at each end of the
sparse lv as when the wipe was done lost I/O messages were sent to
the system log.
After much searching it just seems that writing a sparse lv with some
sort of wipe algorithm will not work. Although if anyone has suggestions
I'm willing to try. I have found writing '0x00''s, the scrub utility,
and a "dd if=/dev/zero of=/dev/lv_pool/lv_test bs=4096 count=1024"
doesn't work.
It's also of interest to note that the 'scrub' utility doesn't seem
to recognize the logical volume format properly as none of the nonzero
algorithms are run - each fails to open the file - for example:
$ /usr/sbin/lvcreate --name lv_test -L 4096K lv_pool
$ /usr/bin/scrub -f -p bsi /dev/lv_pool/lv_test
scrub: using BSI patterns
scrub: warning: /dev/lv_pool/lv_test is zero length
$
John Ferlan (2):
storage: Convert 'building' into a bool
storage: Disallow vol_wipe for sparse logical volumes
src/conf/storage_conf.h | 2 +-
src/storage/storage_backend_logical.c | 39 ++++++++++++++++++++++++++++++++++-
src/storage/storage_driver.c | 8 +++----
src/util/virstoragefile.h | 1 +
4 files changed, 44 insertions(+), 6 deletions(-)
--
1.9.3
1:10 p.m.
New subject: [libvirt] [PATCH 1/2] storage: Convert 'building' into a bool
Rather than a unsigned int, use a 'bool' since that's how it was used.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/storage_conf.h | 2 +-
src/storage/storage_driver.c | 8 ++++----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index badf7a3..54c978d 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -63,7 +63,7 @@ struct _virStorageVolDef {
char *key;
int type; /* virStorageVolType */
- unsigned int building;
+ bool building;
unsigned int in_use;
virStorageVolSource source;
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 441da21..3830867 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1691,7 +1691,7 @@ storageVolCreateXML(virStoragePoolPtr obj,
/* Drop the pool lock during volume allocation */
pool->asyncjobs++;
- voldef->building = 1;
+ voldef->building = true;
virStoragePoolObjUnlock(pool);
buildret = backend->buildVol(obj->conn, pool, buildvoldef, flags);
@@ -1700,7 +1700,7 @@ storageVolCreateXML(virStoragePoolPtr obj,
virStoragePoolObjLock(pool);
storageDriverUnlock(driver);
- voldef->building = 0;
+ voldef->building = false;
pool->asyncjobs--;
if (buildret < 0) {
@@ -1858,7 +1858,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj,
/* Drop the pool lock during volume allocation */
pool->asyncjobs++;
- newvol->building = 1;
+ newvol->building = true;
origvol->in_use++;
virStoragePoolObjUnlock(pool);
@@ -1876,7 +1876,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj,
storageDriverUnlock(driver);
origvol->in_use--;
- newvol->building = 0;
+ newvol->building = false;
allocation = newvol->target.allocation;
pool->asyncjobs--;
--
1.9.3
1:59 p.m.
New subject: [libvirt] [PATCH 1/2] storage: Convert 'building' into a bool
On 07/17/2014 12:10 PM, John Ferlan wrote:
Rather than a unsigned int, use a 'bool' since that's how
it was used.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/storage_conf.h | 2 +-
src/storage/storage_driver.c | 8 ++++----
2 files changed, 5 insertions(+), 5 deletions(-)
ACK
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
1:10 p.m.
New subject: [libvirt] [PATCH 2/2] storage: Disallow vol_wipe for sparse logical volumes
https://bugzilla.redhat.com/show_bug.cgi?id=1091866
Add a new boolean 'sparse'. This will be used by the logical backend
storage driver to determine whether the target volume is sparse or not
(also known by a snapshot or thin logical volume). Although setting sparse
to true at creation could be seen as duplicitous to setting during
virStorageBackendLogicalMakeVol() in case there are ever other code paths
between Create and FindLVs that need to know about the volume be sparse.
Use the 'sparse' in a new virStorageBackendLogicalVolWipe() to decide whether
to attempt to wipe the logical volume or not. For now, I have found no
means to wipe the volume without writing to it. Writing to the sparse
volume causes it to be filled. A sparse logical volume is not complely
writeable as there exists metadata which if overwritten will cause the
sparse lv to go INACTIVE which means pool-refresh will not find it.
Access to whatever lvm uses to manage data blocks is not provided by
any API I could find.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_backend_logical.c | 39 ++++++++++++++++++++++++++++++++++-
src/util/virstoragefile.h | 1 +
2 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/src/storage/storage_backend_logical.c
b/src/storage/storage_backend_logical.c
index ab1e64b..ed62c2f 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -131,6 +131,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).
+ */
+ 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.
@@ -752,6 +761,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
VIR_DIV_UP(vol->target.allocation
? vol->target.allocation : 1, 1024));
virCommandAddArg(cmd, "--virtualsize");
+ vol->target.sparse = true;
}
virCommandAddArgFormat(cmd, "%lluK", VIR_DIV_UP(vol->target.capacity,
1024));
@@ -829,6 +839,33 @@ virStorageBackendLogicalBuildVolFrom(virConnectPtr conn,
return build_func(conn, pool, vol, inputvol, flags);
}
+static int
+virStorageBackendLogicalVolWipe(virConnectPtr conn,
+ virStoragePoolObjPtr pool,
+ virStorageVolDefPtr vol,
+ unsigned int algorithm,
+ unsigned int flags)
+{
+ if (!vol->target.sparse)
+ return virStorageBackendVolWipeLocal(conn, pool, vol, algorithm, flags);
+
+ /* The wiping algorithms will write something to the logical volume.
+ * Writing to a sparse logical volume causes it to be filled resulting
+ * in the volume becoming INACTIVE because there is some amount of
+ * metadata contained within the sparse lv. Choosing to only write
+ * a wipe pattern to the already written portion lv based on what
+ * 'lvs' shows in the "Data%" column/field for the sparse lv was
+ * considered. However, there is no guarantee that sparse lv could
+ * grow or shrink outside of libvirt's knowledge and thus still render
+ * the volume INACTIVE. Until there is some sort of wipe function
+ * implemented by lvm for one of these sparse lv, we'll just return
+ * unsupported.
+ */
+ virReportError(VIR_ERR_NO_SUPPORT,
+ _("logical volue '%s' is sparse, volume wipe not
supported"),
+ vol->target.path);
+ return -1;
+}
virStorageBackend virStorageBackendLogical = {
.type = VIR_STORAGE_POOL_LOGICAL,
@@ -846,5 +883,5 @@ virStorageBackend virStorageBackendLogical = {
.deleteVol = virStorageBackendLogicalDeleteVol,
.uploadVol = virStorageBackendVolUploadLocal,
.downloadVol = virStorageBackendVolDownloadLocal,
- .wipeVol = virStorageBackendVolWipeLocal,
+ .wipeVol = virStorageBackendLogicalVolWipe,
};
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index c840aa0..eccbf4e 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -248,6 +248,7 @@ struct _virStorageSource {
virBitmapPtr features;
char *compat;
bool nocow;
+ bool sparse;
virStoragePermsPtr perms;
virStorageTimestampsPtr timestamps;
--
1.9.3
2:07 p.m.
New subject: [libvirt] [PATCH 2/2] storage: Disallow vol_wipe for sparse logical volumes
On 07/17/2014 12:10 PM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1091866
Add a new boolean 'sparse'. This will be used by the logical backend
storage driver to determine whether the target volume is sparse or not
(also known by a snapshot or thin logical volume). Although setting sparse
to true at creation could be seen as duplicitous to setting during
virStorageBackendLogicalMakeVol() in case there are ever other code paths
between Create and FindLVs that need to know about the volume be sparse.
Use the 'sparse' in a new virStorageBackendLogicalVolWipe() to decide whether
to attempt to wipe the logical volume or not. For now, I have found no
means to wipe the volume without writing to it. Writing to the sparse
volume causes it to be filled. A sparse logical volume is not complely
s/complely/completely/
writeable as there exists metadata which if overwritten will cause
the
sparse lv to go INACTIVE which means pool-refresh will not find it.
Access to whatever lvm uses to manage data blocks is not provided by
any API I could find.
I wonder if lvm developers could point us to an alternative; maybe even
lvreduce (shrink the allocated size) followed by lvresize (regrow it,
but the new growth starts life empty) might behave semantically like a
wipe. But yeah, we don't need to solve that today.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_backend_logical.c | 39 ++++++++++++++++++++++++++++++++++-
src/util/virstoragefile.h | 1 +
2 files changed, 39 insertions(+), 1 deletion(-)
ACK
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4:20 p.m.
On 07/17/2014 02:10 PM, John Ferlan wrote:
John Ferlan (2):
storage: Convert 'building' into a bool
storage: Disallow vol_wipe for sparse logical volumes
src/conf/storage_conf.h | 2 +-
src/storage/storage_backend_logical.c | 39 ++++++++++++++++++++++++++++++++++-
src/storage/storage_driver.c | 8 +++----
src/util/virstoragefile.h | 1 +
4 files changed, 44 insertions(+), 6 deletions(-)
Thanks for the review - I adjusted the completely... funny when I typed
it and looked at it I thought it looked funny but my fingers were more
insistent that they had done the right thing :-)
Not sure lvremove/lvresize will do the trick here - read the 'lvmthin'
man page... Also online I found here was a reading or two online that
suggested one cannot reduce the size of a thin pool due to some lvm bug.
There's another doc that says in order to reduce the size use trim, but
I'm not quite sure how to access that.
In my "example" case - my extent size is 4096 and my thin/sparse size is
4096. Attemping to lvreduce results in a "New size given (xxx) not less
than existing size (1 extents)." So even though I've filled half of it
with 0's, there doesn't seem to be a way to do what we want.
Maybe someone has more "experience" dealing with these
In any case, I pushed what I have...
John
3811
days inactive
3811
days old
5 comments
2 participants
participants (2)
-
Eric Blake -
John Ferlan