[libvirt] [PATCH v2 2/2] storage: btrfs subvolumes for directory pools
This commit adds support for btrfs subvolumes as directory volumes. The directory storage pool can be used to manage btrfs subvolumes on an existing btrfs filesystem. The driver can create new blank subvolumes and snapshots of existing subvolumes as well as delete existing subvolumes. The subvolumes created are automatically made visible on the host side and can be attached to domains using the filesystem tags as defined in 'format domain' documentation. Subvolumes do not implement quotas at the moment because the current (btrfs-progs-0.20.rc1.20130501git7854c8b-4.fc20.x86_64) support for quota management in btrfs-progs is lacking the necessary features, for example it's not possible to see the quota assigned to a certain subvolume and usage information is only updated on syncfs(2). Quota support will be implemented once the tools gain the necessary features. Signed-off-by: Oskari Saarenmaa o...@ohmu.fi --- I did not add a new virDirIsExecutable function as it wasn't really needed by this patch set, it's sufficient to just verify that the backing volume is a directory before running btrfs subvolume snapshot. Part 1/2 still applies cleanly on today's git master so not resending it. configure.ac | 25 ++- docs/schemas/storagevol.rng | 1 + docs/storage.html.in | 30 ++- libvirt.spec.in | 4 + src/conf/storage_conf.c | 3 + src/conf/storage_conf.h | 1 + src/storage/storage_backend.c | 15 +- src/storage/storage_backend_fs.c | 219 -- src/util/virstoragefile.c | 4 +- src/util/virstoragefile.h | 1 + tests/storagevolxml2xmlin/vol-btrfs-snapshot.xml | 13 ++ tests/storagevolxml2xmlin/vol-btrfs.xml | 9 + tests/storagevolxml2xmlout/vol-btrfs-snapshot.xml | 26 +++ tests/storagevolxml2xmlout/vol-btrfs.xml | 17 ++ tests/storagevolxml2xmltest.c | 2 + 15 files changed, 340 insertions(+), 30 deletions(-) create mode 100644 tests/storagevolxml2xmlin/vol-btrfs-snapshot.xml create mode 100644 tests/storagevolxml2xmlin/vol-btrfs.xml create mode 100644 tests/storagevolxml2xmlout/vol-btrfs-snapshot.xml create mode 100644 tests/storagevolxml2xmlout/vol-btrfs.xml diff --git a/configure.ac b/configure.ac index 1993fab..a82640c 100644 --- a/configure.ac +++ b/configure.ac @@ -1646,6 +1646,10 @@ AC_ARG_WITH([storage-sheepdog], [AS_HELP_STRING([--with-storage-sheepdog], [with Sheepdog backend for the storage driver @:@default=check@:@])], [],[with_storage_sheepdog=check]) +AC_ARG_WITH([storage-btrfs], + [AS_HELP_STRING([--with-storage-btrfs], +[with Btrfs backend for the storage driver @:@default=check@:@])], + [],[with_storage_btrfs=check]) if test $with_libvirtd = no; then with_storage_dir=no @@ -1858,6 +1862,24 @@ fi AM_CONDITIONAL([WITH_STORAGE_SHEEPDOG], [test $with_storage_sheepdog = yes]) +if test $with_storage_btrfs = yes || test $with_storage_btrfs = check; then + AC_PATH_PROG([BTRFS], [btrfs], [], [$PATH:/sbin:/usr/sbin]) + + if test $with_storage_btrfs = yes ; then +if test -z $BTRFS ; then AC_MSG_ERROR([We need btrfs -command for btrfs storage driver]) ; fi + else +if test -z $BTRFS ; then with_storage_btrfs=no ; fi +if test $with_storage_btrfs = check ; then with_storage_btrfs=yes ; fi + fi + + if test $with_storage_btrfs = yes ; then +AC_DEFINE_UNQUOTED([WITH_STORAGE_BTRFS], 1, [whether Btrfs backend for storage driver is enabled]) +AC_DEFINE_UNQUOTED([BTRFS],[$BTRFS],[Location of btrfs program]) + fi +fi +AM_CONDITIONAL([WITH_STORAGE_BTRFS], [test $with_storage_btrfs = yes]) + + LIBPARTED_CFLAGS= LIBPARTED_LIBS= @@ -1945,7 +1967,7 @@ AC_SUBST([DEVMAPPER_CFLAGS]) AC_SUBST([DEVMAPPER_LIBS]) with_storage=no -for backend in dir fs lvm iscsi scsi mpath rbd disk; do +for backend in dir fs lvm iscsi scsi mpath rbd disk btrfs; do if eval test \$with_storage_$backend = yes; then with_storage=yes break @@ -2670,6 +2692,7 @@ AC_MSG_NOTICE([ mpath: $with_storage_mpath]) AC_MSG_NOTICE([Disk: $with_storage_disk]) AC_MSG_NOTICE([ RBD: $with_storage_rbd]) AC_MSG_NOTICE([Sheepdog: $with_storage_sheepdog]) +AC_MSG_NOTICE([ Btrfs: $with_storage_btrfs]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Security Drivers]) AC_MSG_NOTICE([]) diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng index 8bc5907..8814973 100644 --- a/docs/schemas/storagevol.rng +++ b/docs/schemas/storagevol.rng @@ -201,6 +201,7 @@ valueqed/value valuevmdk/value valuevpc/value + valuevolume/value /choice /define diff --git a/docs/storage.html.in b/docs/storage.html.in index 1181444..03018ab 100644 --- a/docs/storage.html.in +++ b/docs/storage.html.in @@ -119,12
Re: [libvirt] [PATCH v2 2/2] storage: btrfs subvolumes for directory pools
On Thu, Oct 10, 2013 at 05:52:22PM +0300, Oskari Saarenmaa wrote: This commit adds support for btrfs subvolumes as directory volumes. The directory storage pool can be used to manage btrfs subvolumes on an existing btrfs filesystem. The driver can create new blank subvolumes and snapshots of existing subvolumes as well as delete existing subvolumes. The subvolumes created are automatically made visible on the host side and can be attached to domains using the filesystem tags as defined in 'format domain' documentation. Subvolumes do not implement quotas at the moment because the current (btrfs-progs-0.20.rc1.20130501git7854c8b-4.fc20.x86_64) support for quota management in btrfs-progs is lacking the necessary features, for example it's not possible to see the quota assigned to a certain subvolume and usage information is only updated on syncfs(2). Quota support will be implemented once the tools gain the necessary features. Signed-off-by: Oskari Saarenmaa o...@ohmu.fi --- I did not add a new virDirIsExecutable function as it wasn't really needed by this patch set, it's sufficient to just verify that the backing volume is a directory before running btrfs subvolume snapshot. Part 1/2 still applies cleanly on today's git master so not resending it. configure.ac | 25 ++- docs/schemas/storagevol.rng | 1 + docs/storage.html.in | 30 ++- libvirt.spec.in | 4 + src/conf/storage_conf.c | 3 + src/conf/storage_conf.h | 1 + src/storage/storage_backend.c | 15 +- src/storage/storage_backend_fs.c | 219 -- src/util/virstoragefile.c | 4 +- src/util/virstoragefile.h | 1 + tests/storagevolxml2xmlin/vol-btrfs-snapshot.xml | 13 ++ tests/storagevolxml2xmlin/vol-btrfs.xml | 9 + tests/storagevolxml2xmlout/vol-btrfs-snapshot.xml | 26 +++ tests/storagevolxml2xmlout/vol-btrfs.xml | 17 ++ tests/storagevolxml2xmltest.c | 2 + 15 files changed, 340 insertions(+), 30 deletions(-) create mode 100644 tests/storagevolxml2xmlin/vol-btrfs-snapshot.xml create mode 100644 tests/storagevolxml2xmlin/vol-btrfs.xml create mode 100644 tests/storagevolxml2xmlout/vol-btrfs-snapshot.xml create mode 100644 tests/storagevolxml2xmlout/vol-btrfs.xml diff --git a/configure.ac b/configure.ac index 1993fab..a82640c 100644 --- a/configure.ac +++ b/configure.ac @@ -1646,6 +1646,10 @@ AC_ARG_WITH([storage-sheepdog], [AS_HELP_STRING([--with-storage-sheepdog], [with Sheepdog backend for the storage driver @:@default=check@:@])], [],[with_storage_sheepdog=check]) +AC_ARG_WITH([storage-btrfs], + [AS_HELP_STRING([--with-storage-btrfs], +[with Btrfs backend for the storage driver @:@default=check@:@])], + [],[with_storage_btrfs=check]) We no longer have a backend for btrfs - it is just a part of the fs driver. if test $with_libvirtd = no; then with_storage_dir=no @@ -1858,6 +1862,24 @@ fi AM_CONDITIONAL([WITH_STORAGE_SHEEPDOG], [test $with_storage_sheepdog = yes]) +if test $with_storage_btrfs = yes || test $with_storage_btrfs = check; then + AC_PATH_PROG([BTRFS], [btrfs], [], [$PATH:/sbin:/usr/sbin]) + + if test $with_storage_btrfs = yes ; then +if test -z $BTRFS ; then AC_MSG_ERROR([We need btrfs -command for btrfs storage driver]) ; fi + else +if test -z $BTRFS ; then with_storage_btrfs=no ; fi +if test $with_storage_btrfs = check ; then with_storage_btrfs=yes ; fi + fi + + if test $with_storage_btrfs = yes ; then +AC_DEFINE_UNQUOTED([WITH_STORAGE_BTRFS], 1, [whether Btrfs backend for storage driver is enabled]) +AC_DEFINE_UNQUOTED([BTRFS],[$BTRFS],[Location of btrfs program]) + fi +fi +AM_CONDITIONAL([WITH_STORAGE_BTRFS], [test $with_storage_btrfs = yes]) Just use 'WITH_BTRFS' here - the WITH_STORAGE_ is for actual storage backends. + + LIBPARTED_CFLAGS= LIBPARTED_LIBS= @@ -1945,7 +1967,7 @@ AC_SUBST([DEVMAPPER_CFLAGS]) AC_SUBST([DEVMAPPER_LIBS]) with_storage=no -for backend in dir fs lvm iscsi scsi mpath rbd disk; do +for backend in dir fs lvm iscsi scsi mpath rbd disk btrfs; do if eval test \$with_storage_$backend = yes; then with_storage=yes break @@ -2670,6 +2692,7 @@ AC_MSG_NOTICE([ mpath: $with_storage_mpath]) AC_MSG_NOTICE([Disk: $with_storage_disk]) AC_MSG_NOTICE([ RBD: $with_storage_rbd]) AC_MSG_NOTICE([Sheepdog: $with_storage_sheepdog]) +AC_MSG_NOTICE([ Btrfs: $with_storage_btrfs]) Again not needed here. AC_MSG_NOTICE([]) AC_MSG_NOTICE([Security Drivers]) AC_MSG_NOTICE([]) diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng index
Re: [libvirt] [PATCH v2 2/2] storage: btrfs subvolumes for directory pools
Thanks for the review,
10.10.2013 18:57, Daniel P. Berrange kirjoitti:
On Thu, Oct 10, 2013 at 05:52:22PM +0300, Oskari Saarenmaa wrote:
+if test $with_storage_btrfs = yes || test $with_storage_btrfs = check;
then
+ AC_PATH_PROG([BTRFS], [btrfs], [], [$PATH:/sbin:/usr/sbin])
[...]
+AM_CONDITIONAL([WITH_STORAGE_BTRFS], [test $with_storage_btrfs = yes])
Just use 'WITH_BTRFS' here - the WITH_STORAGE_ is for actual
storage backends.
Do we actually need a flag for enabling / disabling btrfs usage as it's
not a separate storage pool? I'm thinking about just calling
AC_PATH_PROG(btrfs) if FS pool is enabled, and using #ifdef BTRFS in the
fs code to use btrfs if available.
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -90,6 +90,7 @@ struct _virStorageVolTarget {
virStorageEncryptionPtr encryption;
virBitmapPtr features;
char *compat;
+char *uuid;
};
This struct represents the public XML data format - adding random fields
to it for individual driver use is not allowed.
Ok, I'll put this thing somewhere else.
+if (volv-backingStore.path == NULL) {
+/* backing store not in the pool, ignore it */
Backing stores for volumes are not required to be in the same pool as
the source. For example it is valid to have a qcow2 file backed by
a lvm volume.
In btrfs's case the note about backing store in an existing subvolume is
informational only; all subvolumes, snapshots or not, are independent
and in case we can't find a symbolic name for the parent volume I think
it's safe to just ignore it. I'll add this note to the comment.
I'm thinking it would be nice to have a dedicate file with APIs for btrfs
eg src/util/virbtrfs.{h,c} and have it contain things like
virBtrFSCreateVolume()
virBtrFSCreateVolume()
virBtrFS...()
and so on, so we don't have all these virCommand calls littering this
file.
I'm not sure it makes sense to do this until there's another user for
btrfs commands in libvirt, but I can do it if you like. I think it'd be
more useful to have a module which implements copy-on-write operations
for different filesystems (btrfs, zfs, something else?) with a common api.
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index d5effa4..4dc6c81 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -46,6 +46,7 @@ enum virStorageFileFormat {
VIR_STORAGE_FILE_FAT,
VIR_STORAGE_FILE_VHD,
VIR_STORAGE_FILE_VDI,
+VIR_STORAGE_FILE_VOLUME,
This isn't right - volumes already have a type={file,block,dir,volume}
and we shouldn't duplicate this in the format attribute too.
Hmm.. I don't see such a field in virStorageVolType - am I looking at a
wrong enum?
diff --git a/tests/storagevolxml2xmlin/vol-btrfs-snapshot.xml
b/tests/storagevolxml2xmlin/vol-btrfs-snapshot.xml
new file mode 100644
index 000..5a58b56
--- /dev/null
+++ b/tests/storagevolxml2xmlin/vol-btrfs-snapshot.xml
@@ -0,0 +1,13 @@
+volume
+ nameclone/name
+ capacity unit=bytes0/capacity
+ allocation unit=bytes0/allocation
+ target
+path/lxc/clone/path
+format type='volume'/
+ /target
+ backingStore
+path/lxc/vanilla/path
+format type='volume'/
+ /backingStore
+/volume
Using 'format' in this way is wrong. What we should be doing
is exposing the volume 'type' as an attribute eg
volume type='volume'
...
/volume
I suppose that makes sense, but it would make this incompatible with
existing volume creation. Right now (with the current patch) I can use
existing virsh tooling to run, for example 'virsh vol-create-as default
guest-1 0 --format volume --backing-store guest-base'
diff --git a/tests/storagevolxml2xmlout/vol-btrfs-snapshot.xml
b/tests/storagevolxml2xmlout/vol-btrfs-snapshot.xml
new file mode 100644
index 000..75830d3
--- /dev/null
+++ b/tests/storagevolxml2xmlout/vol-btrfs-snapshot.xml
@@ -0,0 +1,26 @@
+volume
+ nameclone/name
+ key(null)/key
If you're getting (null) here, then something has gone wrong
I'll look into this.
Thanks,
Oskari
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/2] storage: btrfs subvolumes for directory pools
On 10/10/2013 09:57 AM, Daniel P. Berrange wrote:
+}
+if (volv-backingStore.path == NULL) {
+/* backing store not in the pool, ignore it */
Backing stores for volumes are not required to be in the same pool as
the source. For example it is valid to have a qcow2 file backed by
a lvm volume.
What's more, qemu allows one to have qcow2 data atop a network device;
but right now libvirt forcefully assumes that all network devices (nbd,
ceph, sheepdog, gluster) contain only raw data rather than other
formats. I'm trying to investigate what implications this has, but
among other things, I think we need to use more polymorphic callbacks
where for a given backing store string, we determine which storage
driver to use, then that storage driver tells us how to manage data from
that storage (including having qcow2 data atop network storage).
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
