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 <os(a)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_XXXX 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 8bc5907..8814973 100644
--- a/docs/schemas/storagevol.rng
+++ b/docs/schemas/storagevol.rng
@@ -201,6 +201,7 @@
<value>qed</value>
<value>vmdk</value>
<value>vpc</value>
+ <value>volume</value>
</choice>
</define>
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 975e662..b349c3f 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -314,11 +314,14 @@ virStorageVolDefFree(virStorageVolDefPtr def)
VIR_FREE(def->source.extents);
VIR_FREE(def->target.compat);
+ VIR_FREE(def->target.uuid);
virBitmapFree(def->target.features);
VIR_FREE(def->target.path);
VIR_FREE(def->target.perms.label);
VIR_FREE(def->target.timestamps);
virStorageEncryptionFree(def->target.encryption);
+ VIR_FREE(def->backingStore.compat);
+ VIR_FREE(def->backingStore.uuid);
VIR_FREE(def->backingStore.path);
VIR_FREE(def->backingStore.perms.label);
VIR_FREE(def->backingStore.timestamps);
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 62ff1fd..6414c74 100644
--- 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.
diff --git a/src/storage/storage_backend_fs.c
b/src/storage/storage_backend_fs.c
index f9b0326..2cf300f 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -51,6 +51,7 @@
#include "virfile.h"
#include "virlog.h"
#include "virstring.h"
+#include "dirname.h"
#define VIR_FROM_THIS VIR_FROM_STORAGE
@@ -801,6 +802,33 @@ error:
}
+#if WITH_STORAGE_BTRFS
+static int
+btrfsVolInfo(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
+ char **groups,
+ void *data)
+{
+ virStorageVolDefPtr vol = data;
+
+ if (STREQ(groups[0], "uuid")) {
+ vol->type = VIR_STORAGE_VOL_DIR;
+ vol->target.format = VIR_STORAGE_FILE_VOLUME;
+ VIR_FREE(vol->target.uuid);
+ if (VIR_STRDUP(vol->target.uuid, groups[1]) < 0)
+ return -1;
+ } else if (STREQ(groups[0], "Parent uuid") && STRNEQ(groups[1],
"-")) {
+ vol->backingStore.format = VIR_STORAGE_FILE_VOLUME;
+ VIR_FREE(vol->backingStore.path);
+ vol->backingStore.path = NULL;
+ VIR_FREE(vol->backingStore.uuid);
+ if (VIR_STRDUP(vol->backingStore.uuid, groups[1]) < 0)
+ return -1;
+ }
+
+ return 0;
+}
+#endif
+
/**
* Iterate over the pool's directory and enumerate all disk images
* within it. This is non-recursive.
@@ -809,10 +837,17 @@ static int
virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED,
virStoragePoolObjPtr pool)
{
- DIR *dir;
+ int ret = -1;
+ DIR *dir = NULL;
struct dirent *ent;
struct statvfs sb;
virStorageVolDefPtr vol = NULL;
+#if WITH_STORAGE_BTRFS
+ int missing_subvolume_info = 0;
+ char *fstype = NULL;
+
+ fstype = virFileFsType(pool->def->target.path);
+#endif
if (!(dir = opendir(pool->def->target.path))) {
virReportSystemError(errno,
@@ -822,7 +857,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
ATTRIBUTE_UNUSED,
}
while ((ent = readdir(dir)) != NULL) {
- int ret;
+ int probe;
char *backingStore;
int backingStoreFormat;
@@ -842,19 +877,19 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
ATTRIBUTE_UNUSED,
if (VIR_STRDUP(vol->key, vol->target.path) < 0)
goto cleanup;
- if ((ret = virStorageBackendProbeTarget(&vol->target,
- &backingStore,
- &backingStoreFormat,
- &vol->allocation,
- &vol->capacity,
- &vol->target.encryption)) < 0)
{
- if (ret == -2) {
+ if ((probe = virStorageBackendProbeTarget(&vol->target,
+ &backingStore,
+ &backingStoreFormat,
+ &vol->allocation,
+ &vol->capacity,
+ &vol->target.encryption)) <
0) {
+ if (probe == -2) {
/* Silently ignore non-regular files,
* eg '.' '..', 'lost+found', dangling symbolic
link */
virStorageVolDefFree(vol);
vol = NULL;
continue;
- } else if (ret == -3) {
+ } else if (probe == -3) {
/* The backing file is currently unavailable, its format is not
* explicitly specified, the probe to auto detect the format
* failed: continue with faked RAW format, since AUTO will
@@ -865,6 +900,23 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
ATTRIBUTE_UNUSED,
goto cleanup;
}
+#if WITH_STORAGE_BTRFS
+ /* check for subvolumes */
+ if (vol->target.format == VIR_STORAGE_FILE_DIR &&
+ fstype != NULL && STREQ(fstype, "btrfs")) {
+ int vars[] = {2};
+ const char *regexes[] = {"^\\s*([A-Za-z ]+):\\s*(.+)\\s*$"};
+ virCommandPtr cmd = virCommandNewArgList(
+ "btrfs", "subvolume", "show",
vol->target.path, NULL);
+ if (cmd == NULL)
+ goto cleanup;
+ virStorageBackendRunProgRegex(NULL, cmd, 1, regexes, vars,
+ btrfsVolInfo, vol, NULL);
+ if (vol->backingStore.format == VIR_STORAGE_FILE_VOLUME)
+ missing_subvolume_info ++;
You're incrementing this multiple times
+ }
+#endif
+
/* directory based volume */
if (vol->target.format == VIR_STORAGE_FILE_DIR)
vol->type = VIR_STORAGE_VOL_DIR;
@@ -896,13 +948,44 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn
ATTRIBUTE_UNUSED,
vol = NULL;
}
closedir(dir);
-
+ dir = NULL;
+
+#if WITH_STORAGE_BTRFS
+ if (missing_subvolume_info) {
But only checking for non-zero, so this should be a 'bool' not 'int'
+ /* update snapshot information */
+ virStorageVolDefPtr volv, volb;
+ int idxv, idxb;
Use 'i' and 'j' for loop iterators and make them size_t.
+
+ for (idxv = 0; idxv < pool->volumes.count; idxv ++) {
No whitespace before '++' operator
+ volv = pool->volumes.objs[idxv];
+ if (volv->backingStore.format == VIR_STORAGE_FILE_VOLUME &&
+ volv->backingStore.uuid != NULL) {
+ for (idxb = 0; idxb < pool->volumes.count; idxb ++) {
Again whitespace.
+ volb = pool->volumes.objs[idxb];
+ if (volb->target.format == VIR_STORAGE_FILE_VOLUME &&
+ volb->target.uuid != NULL &&
+ STREQ(volb->target.uuid, volv->backingStore.uuid)) {
+ if (VIR_STRDUP(volv->backingStore.path, volb->target.path)
< 0)
+ goto cleanup;
+ break;
+ }
+ }
+ 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.
+static int createVolumeDir(virConnectPtr conn ATTRIBUTE_UNUSED,
+ virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
+ virStorageVolDefPtr vol,
+ virStorageVolDefPtr inputvol,
+ unsigned int flags)
+{
+ int ret = -1;
+ char *vol_dir_name = NULL;
+ char *fstype = NULL;
+ virCommandPtr cmd = NULL;
+ struct stat st;
+
+ virCheckFlags(0, -1);
+
+ if (inputvol) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("cannot copy from volume to a subvolume"));
+ return -1;
+ }
+
+ if ((vol_dir_name = mdir_name(vol->target.path)) == NULL)
+ goto cleanup;
+
+ fstype = virFileFsType(vol_dir_name);
+ if (fstype == NULL) {
+ virReportSystemError(errno,
+ _("cannot get filesystem type for %s"),
+ vol->target.path);
+ goto cleanup;
+ }
+#if WITH_STORAGE_BTRFS
+ else if (STREQ(fstype, "btrfs")) {
+ cmd = virCommandNew("btrfs");
+ if (!cmd)
+ goto cleanup;
+
+ if (vol->backingStore.path == NULL) {
+ virCommandAddArgList(cmd, "subvolume", "create",
vol->target.path, NULL);
+ } else {
+ if (!virFileIsDir(vol->backingStore.path)) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ _("backing store %s is not a directory"),
+ vol->backingStore.path);
+ goto cleanup;
+ }
+
+ virCommandAddArgList(cmd, "subvolume", "snapshot",
vol->backingStore.path,
+ vol->target.path, NULL);
+ }
+ if (virCommandRun(cmd, NULL) < 0)
+ goto cleanup;
+ }
+#endif
+ else {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("subvolumes are not supported in %s"),
+ fstype);
+ goto cleanup;
+ }
+
+ if (stat(vol->target.path, &st) < 0) {
+ virReportSystemError(errno,
+ _("failed to create %s"), vol->target.path);
+ goto cleanup;
+ }
+ ret = 0;
+
+cleanup:
+ VIR_FREE(vol_dir_name);
+ VIR_FREE(fstype);
+ virCommandFree(cmd);
+ return ret;
+}
+
static int createFileDir(virConnectPtr conn ATTRIBUTE_UNUSED,
virStoragePoolObjPtr pool,
virStorageVolDefPtr vol,
@@ -1061,6 +1229,8 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn,
create_func = virStorageBackendCreateRaw;
} else if (vol->target.format == VIR_STORAGE_FILE_DIR) {
create_func = createFileDir;
+ } else if (vol->target.format == VIR_STORAGE_FILE_VOLUME) {
+ create_func = createVolumeDir;
} else if ((tool_type = virStorageBackendFindFSImageTool(NULL)) != -1) {
create_func = virStorageBackendFSImageToolTypeToFunc(tool_type);
@@ -1133,6 +1303,23 @@ virStorageBackendFileSystemVolDelete(virConnectPtr conn
ATTRIBUTE_UNUSED,
}
break;
case VIR_STORAGE_VOL_DIR:
+#if WITH_STORAGE_BTRFS
+ if (vol->target.format == VIR_STORAGE_FILE_VOLUME) {
+ char *fstype = virFileFsType(vol->target.path);
+ if (fstype != NULL && STREQ(fstype, "btrfs")) {
+ virCommandPtr cmd = NULL;
+ int ret;
+
+ cmd = virCommandNewArgList("btrfs", "subvolume",
"delete",
+ vol->target.path, NULL);
+ ret = (virCommandRun(cmd, NULL) < 0) ? -1 : 0;
+ virCommandFree(cmd);
+ VIR_FREE(fstype);
+ return ret;
+ }
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.
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 48d5fbb..d0231b6 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -57,7 +57,7 @@ VIR_ENUM_IMPL(virStorageFileFormat,
"raw", "dir", "bochs",
"cloop", "cow", "dmg", "iso",
"qcow", "qcow2", "qed", "vmdk",
"vpc",
- "fat", "vhd", "vdi")
+ "fat", "vhd", "vdi", "volume")
VIR_ENUM_IMPL(virStorageFileFeature,
VIR_STORAGE_FILE_FEATURE_LAST,
@@ -232,6 +232,8 @@ static struct FileTypeInfo const fileTypeInfo[] = {
-1, {0}, 0, 0, 0, 0, NULL, NULL },
[VIR_STORAGE_FILE_VHD] = { 0, NULL, NULL, LV_LITTLE_ENDIAN,
-1, {0}, 0, 0, 0, 0, NULL, NULL },
+ [VIR_STORAGE_FILE_VOLUME] = { 0, NULL, NULL, LV_LITTLE_ENDIAN,
+ -1, {0}, 0, 0, 0, 0, NULL, NULL },
};
verify(ARRAY_CARDINALITY(fileTypeInfo) == VIR_STORAGE_FILE_LAST);
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.
VIR_STORAGE_FILE_LAST,
};
diff --git a/tests/storagevolxml2xmlin/vol-btrfs-snapshot.xml
b/tests/storagevolxml2xmlin/vol-btrfs-snapshot.xml
new file mode 100644
index 0000000..5a58b56
--- /dev/null
+++ b/tests/storagevolxml2xmlin/vol-btrfs-snapshot.xml
@@ -0,0 +1,13 @@
+<volume>
+ <name>clone</name>
+ <capacity unit="bytes">0</capacity>
+ <allocation unit="bytes">0</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>
diff --git a/tests/storagevolxml2xmlout/vol-btrfs-snapshot.xml
b/tests/storagevolxml2xmlout/vol-btrfs-snapshot.xml
new file mode 100644
index 0000000..75830d3
--- /dev/null
+++ b/tests/storagevolxml2xmlout/vol-btrfs-snapshot.xml
@@ -0,0 +1,26 @@
+<volume>
+ <name>clone</name>
+ <key>(null)</key>
If you're getting (null) here, then something has gone wrong
+ <source>
+ </source>
+ <capacity unit='bytes'>0</capacity>
+ <allocation unit='bytes'>0</allocation>
+ <target>
+ <path>/lxc/clone</path>
+ <format type='volume'/>
+ <permissions>
+ <mode>0600</mode>
+ <owner>4294967295</owner>
+ <group>4294967295</group>
+ </permissions>
+ </target>
+ <backingStore>
+ <path>/lxc/vanilla</path>
+ <format type='volume'/>
+ <permissions>
+ <mode>0600</mode>
+ <owner>4294967295</owner>
+ <group>4294967295</group>
+ </permissions>
+ </backingStore>
+</volume>
diff --git a/tests/storagevolxml2xmlout/vol-btrfs.xml
b/tests/storagevolxml2xmlout/vol-btrfs.xml
new file mode 100644
index 0000000..fbad915
--- /dev/null
+++ b/tests/storagevolxml2xmlout/vol-btrfs.xml
@@ -0,0 +1,17 @@
+<volume>
+ <name>vanilla</name>
+ <key>(null)</key>
Again.
+ <source>
+ </source>
+ <capacity unit='bytes'>0</capacity>
+ <allocation unit='bytes'>0</allocation>
+ <target>
+ <path>/lxc/vanilla</path>
+ <format type='volume'/>
+ <permissions>
+ <mode>0600</mode>
+ <owner>4294967295</owner>
+ <group>4294967295</group>
+ </permissions>
+ </target>
+</volume>
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 :|