[libvirt] storage pool that contains thin LVs
Hey guys, Just looking at https://bugzilla.redhat.com/show_bug.cgi?id=924672 and looks like all we need to do is ignore thin pools and thin pool data devices. With some trivial testing this seems to work fine (i.e. failed before and works now). diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index a1a37a1..0154256 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -85,6 +85,10 @@ virStorageBackendLogicalMakeVol(virStoragePoolObjPtr pool, if (attrs[4] != 'a') return 0; + /* Skip thin pools(t) and thin pool data(T) */ + if (attrs[0] == 't' || attrs[0] == 'T') + return 0; + /* See if we're only looking for a specific volume */ if (data != NULL) { vol = data; I'm sure the fix isn't this trivial but i can finish it up and submit an official patch (through git) if this is close. Dusty
On Thu, Sep 19, 2013 at 10:34 PM, Dusty Mabe <dustymabe@gmail.com> wrote:
Hey guys,
Just looking at https://bugzilla.redhat.com/show_bug.cgi?id=924672 and looks like all we need to do is ignore thin pools and thin pool data devices. With some trivial testing this seems to work fine (i.e. failed before and works now).
diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index a1a37a1..0154256 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -85,6 +85,10 @@ virStorageBackendLogicalMakeVol(virStoragePoolObjPtr pool, if (attrs[4] != 'a') return 0;
+ /* Skip thin pools(t) and thin pool data(T) */ + if (attrs[0] == 't' || attrs[0] == 'T') + return 0; + /* See if we're only looking for a specific volume */ if (data != NULL) { vol = data;
I'm sure the fix isn't this trivial but i can finish it up and submit an official patch (through git) if this is close.
Dusty
Not yet sure of the correctness / completeness but I would go ahead and submit a git commit and we can go from there. -- Doug Goldstein
On Thu, Sep 19, 2013 at 11:34:15PM -0400, Dusty Mabe wrote:
Hey guys,
Just looking at https://bugzilla.redhat.com/show_bug.cgi?id=924672 and looks like all we need to do is ignore thin pools and thin pool data devices. With some trivial testing this seems to work fine (i.e. failed before and works now).
diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index a1a37a1..0154256 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -85,6 +85,10 @@ virStorageBackendLogicalMakeVol(virStoragePoolObjPtr pool, if (attrs[4] != 'a') return 0;
+ /* Skip thin pools(t) and thin pool data(T) */ + if (attrs[0] == 't' || attrs[0] == 'T') + return 0; + /* See if we're only looking for a specific volume */ if (data != NULL) { vol = data;
I'm sure the fix isn't this trivial but i can finish it up and submit an official patch (through git) if this is close.
Do we really want to hide thin volumes. Unless i'm missing something in my understanding of LVM, we should expose these as storage pool volumes in the same way as thick provisioned volumes. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Fri, Sep 20, 2013 at 5:18 AM, Daniel P. Berrange <berrange@redhat.com> wrote:
On Thu, Sep 19, 2013 at 11:34:15PM -0400, Dusty Mabe wrote:
I'm sure the fix isn't this trivial but i can finish it up and submit an official patch (through git) if this is close.
Do we really want to hide thin volumes. Unless i'm missing something in my understanding of LVM, we should expose these as storage pool volumes in the same way as thick provisioned volumes.
Good question. This patch would only filter out (t)hin pools, and (T)hin pool data volumes (these are the backing stores). It would not filter out Thin (V)olumes. "man lvs" lists all of the different types. By the way if you wanted thin volumes to show up in libvirt they already don't. The regular expression we are using requires items to be listed within the "devices" column of lvs and these volumes don't have anything in that column. I can work with you guys to fix that if you like. One other thing.. has anyone considered making an LVM thin pool a storage type? I assume so but just wanted to check. Dusty
participants (3)
-
Daniel P. Berrange -
Doug Goldstein -
Dusty Mabe