[libvirt] [PATCH 0/2] btrfs subvolume management

Moved btrfs subvolume management to storage_backend_fs.c instead of implementing it as a separate pool as suggested by Daniel P. Berrange in https://www.redhat.com/archives/libvir-list/2013-September/msg00316.html Oskari Saarenmaa (2): virFileFsType: get filesystem type of a given path storage: btrfs subvolumes for directory pools 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/libvirt_private.syms | 1 + src/storage/storage_backend.c | 15 +- src/storage/storage_backend_fs.c | 222 ++++++++++++++++++++-- src/util/virfile.c | 47 +++++ src/util/virfile.h | 1 + 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 + 18 files changed, 392 insertions(+), 30 deletions(-)

This can be used by storage pools to figure out which actions are available on various paths (for example subvolumes when running on btrfs.) Signed-off-by: Oskari Saarenmaa <os@ohmu.fi> --- src/libvirt_private.syms | 1 + src/util/virfile.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ src/util/virfile.h | 1 + 3 files changed, 49 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 35f0f1b..d0238cf 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1370,6 +1370,7 @@ virFileExists; virFileFclose; virFileFdopen; virFileFindMountPoint; +virFileFsType; virFileHasSuffix; virFileIsAbsPath; virFileIsDir; diff --git a/src/util/virfile.c b/src/util/virfile.c index feac3c9..44871d6 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -1133,6 +1133,45 @@ cleanup: return ret; } +/* search /proc/mounts for the filesystem type of the given path; + * return pointer to malloc'ed string of type if found, otherwise + * return NULL. + */ +char * +virFileFsType(const char *path) +{ + FILE *f; + struct mntent mb; + char mntbuf[1024]; + char *real = NULL, *ret = NULL; + size_t lookup_len, longest = 0; + + if ((real = realpath(path, NULL)) == NULL) + return NULL; + lookup_len = strlen(real); + + f = setmntent("/proc/mounts", "r"); + if (!f) { + VIR_FREE(real); + return NULL; + } + + while (getmntent_r(f, &mb, mntbuf, sizeof(mntbuf))) { + size_t mnt_dir_len = strlen(mb.mnt_dir); + if (lookup_len >= mnt_dir_len && mnt_dir_len >= longest) { + if (memcmp(mb.mnt_dir, real, mnt_dir_len) == 0) { + longest = mnt_dir_len; + VIR_FREE(ret); + ignore_value(VIR_STRDUP_QUIET(ret, mb.mnt_type)); + } + } + } + endmntent(f); + VIR_FREE(real); + + return ret; +} + #else /* defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R */ char * @@ -1143,6 +1182,14 @@ virFileFindMountPoint(const char *type ATTRIBUTE_UNUSED) return NULL; } +char * +virFileFsType(const char *path) +{ + errno = ENOSYS; + + return NULL; +} + #endif /* defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R */ int diff --git a/src/util/virfile.h b/src/util/virfile.h index 72d35ce..3c01247 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -221,6 +221,7 @@ int virFileOpenTty(int *ttymaster, int rawmode); char *virFileFindMountPoint(const char *type); +char *virFileFsType(const char *path); void virFileWaitForDevices(void); -- 1.8.3.1

On 10.09.2013 20:57, Oskari Saarenmaa wrote:
This can be used by storage pools to figure out which actions are available on various paths (for example subvolumes when running on btrfs.)
Signed-off-by: Oskari Saarenmaa <os@ohmu.fi> --- src/libvirt_private.syms | 1 + src/util/virfile.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ src/util/virfile.h | 1 + 3 files changed, 49 insertions(+)
ACK Michal

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@ohmu.fi> --- 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 | 222 ++++++++++++++++++++-- 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, 343 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 cdbe6cc..91b0a8d 100644 --- a/configure.ac +++ b/configure.ac @@ -1642,6 +1642,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 @@ -1854,6 +1858,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= @@ -1941,7 +1963,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 @@ -2663,6 +2685,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 @@ <value>qed</value> <value>vmdk</value> <value>vpc</value> + <value>volume</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 +119,15 @@ <h2><a name="StorageBackendDir">Directory pool</a></h2> <p> A pool with a type of <code>dir</code> provides the means to manage - files within a directory. The files can be fully allocated raw files, - sparsely allocated raw files, or one of the special disk formats - such as <code>qcow</code>,<code>qcow2</code>,<code>vmdk</code>, - <code>cow</code>, etc as supported by the <code>qemu-img</code> - program. If the directory does not exist at the time the pool is - defined, the <code>build</code> operation can be used to create it. + files and filesystem subvolumes within a directory. The files can + be fully allocated raw files, sparsely allocated raw files, or one + of the special disk formats such as <code>qcow</code>, + <code>qcow2</code>, <code>vmdk</code>, <code>cow</code>, etc as + supported by the <code>qemu-img</code> program. The directory can + also contain filesystem subvolumes which show up as + <code>volume</code> format volumes. If the directory does not exist + at the time the pool is defined, the <code>build</code> operation + can be used to create it. </p> <h3>Example pool input definition</h3> @@ -157,6 +160,9 @@ <li><code>qed</code>: QEMU Enhanced Disk image format</li> <li><code>vmdk</code>: VMWare disk image format</li> <li><code>vpc</code>: VirtualPC disk image format</li> + <li><code>volume</code>: Filesystem subvolume (only available on + btrfs <span class="since">since + 1.1.3</span></li> </ul> <p> When listing existing volumes all these formats are supported @@ -166,7 +172,19 @@ either <code>qemu-img</code> or <code>qcow-create</code> tools are present. The others are dependent on support of the <code>qemu-img</code> tool. + </p> + + <h3>Btrfs subvolume management</h3> + <p> + Libvirt can manage btrfs subvolumes and their snapshots in a btrfs + backed directory pool. Such a pool can be used to create subvolumes + and snapshots on the host side and the created volumes can be + attached as filesystems to LXC domains. Information about attaching + a filesystem to a guest can be found at the <a + href="formatdomain.html#elementsFilesystems">format domain</a> page. + Subvolume quotas are not supported at the moment. + <span class="since">Since 1.1.3</span> </p> <h2><a name="StorageBackendFS">Filesystem pool</a></h2> diff --git a/libvirt.spec.in b/libvirt.spec.in index a55e87c..1f57678 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -518,6 +518,8 @@ BuildRequires: PolicyKit-devel >= 0.6 %if %{with_storage_fs} # For mount/umount in FS driver BuildRequires: util-linux +# For btrfs +BuildRequires: btrfs-progs %endif %if %{with_qemu} # From QEMU RPMs @@ -731,6 +733,8 @@ Requires: device-mapper Requires: device-mapper %endif %if %{with_storage_sheepdog} +# For Btrfs storage +Requires: btrfs-progs # For Sheepdog support Requires: sheepdog %endif diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 002663f..b3d6a4a 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; }; typedef struct _virStorageVolDef virStorageVolDef; diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index b7edf85..668a27b 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1240,13 +1240,16 @@ virStorageBackendUpdateVolInfoFlags(virStorageVolDefPtr vol, openflags)) < 0) return ret; - if (vol->backingStore.path && - (ret = virStorageBackendUpdateVolTargetInfo(&vol->backingStore, - NULL, NULL, - VIR_STORAGE_VOL_OPEN_DEFAULT)) < 0) - return ret; + if (vol->backingStore.path) { + int flags = VIR_STORAGE_VOL_OPEN_DEFAULT; - return 0; + if (vol->backingStore.format == VIR_STORAGE_FILE_VOLUME) + flags |= VIR_STORAGE_VOL_OPEN_DIR; + ret = virStorageBackendUpdateVolTargetInfo(&vol->backingStore, + NULL, NULL, flags); + } + + return ret; } int virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index d305b06..7b4c0b6 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 @@ -802,6 +803,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. @@ -810,10 +838,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, @@ -823,7 +858,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED, } while ((ent = readdir(dir)) != NULL) { - int ret; + int probe; char *backingStore; int backingStoreFormat; @@ -843,19 +878,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 @@ -866,6 +901,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 ++; + } +#endif + /* directory based volume */ if (vol->target.format == VIR_STORAGE_FILE_DIR) vol->type = VIR_STORAGE_VOL_DIR; @@ -897,13 +949,44 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED, vol = NULL; } closedir(dir); - + dir = NULL; + +#if WITH_STORAGE_BTRFS + if (missing_subvolume_info) { + /* update snapshot information */ + virStorageVolDefPtr volv, volb; + int idxv, idxb; + + for (idxv = 0; idxv < pool->volumes.count; idxv ++) { + 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 ++) { + 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 */ + VIR_FREE(volv->backingStore.uuid); + volv->backingStore.uuid = NULL; + volv->backingStore.format = VIR_STORAGE_FILE_NONE; + } + } + } + } +#endif if (statvfs(pool->def->target.path, &sb) < 0) { virReportSystemError(errno, _("cannot statvfs path '%s'"), pool->def->target.path); - return -1; + goto cleanup; } pool->def->capacity = ((unsigned long long)sb.f_frsize * (unsigned long long)sb.f_blocks); @@ -911,14 +994,19 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED, (unsigned long long)sb.f_frsize); pool->def->allocation = pool->def->capacity - pool->def->available; - return 0; + ret = 0; - cleanup: +cleanup: +#if WITH_STORAGE_BTRFS + VIR_FREE(fstype); +#endif if (dir) closedir(dir); virStorageVolDefFree(vol); - virStoragePoolObjClearVols(pool); - return -1; + if (ret < 0) + virStoragePoolObjClearVols(pool); + + return ret; } @@ -1001,10 +1089,93 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn ATTRIBUTE_UNUSED, return -1; } + if (vol->target.format == VIR_STORAGE_FILE_VOLUME) { + vol->type = VIR_STORAGE_VOL_DIR; + if (vol->backingStore.path != NULL) + vol->backingStore.format = VIR_STORAGE_FILE_VOLUME; + } + VIR_FREE(vol->key); return VIR_STRDUP(vol->key, vol->target.path); } +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 { + int accessRetCode = -1; + + accessRetCode = access(vol->backingStore.path, R_OK | X_OK); + if (accessRetCode != 0) { + virReportSystemError(errno, + _("inaccessible backing store volume %s"), + 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, @@ -1062,6 +1233,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); @@ -1134,6 +1307,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; + } + VIR_FREE(fstype); + } +#endif if (rmdir(vol->target.path) < 0) { virReportSystemError(errno, _("cannot remove directory '%s'"), diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 0b9cec3..7872340 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 1f89839..eaaffb0 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, 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> diff --git a/tests/storagevolxml2xmlin/vol-btrfs.xml b/tests/storagevolxml2xmlin/vol-btrfs.xml new file mode 100644 index 0000000..e53bc8d --- /dev/null +++ b/tests/storagevolxml2xmlin/vol-btrfs.xml @@ -0,0 +1,9 @@ +<volume> + <name>vanilla</name> + <capacity unit="bytes">0</capacity> + <allocation unit="bytes">0</allocation> + <target> + <path>/lxc/vanilla</path> + <format type='volume'/> + </target> +</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> + <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> + <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> diff --git a/tests/storagevolxml2xmltest.c b/tests/storagevolxml2xmltest.c index 5b0a60b..6be34d6 100644 --- a/tests/storagevolxml2xmltest.c +++ b/tests/storagevolxml2xmltest.c @@ -116,6 +116,8 @@ mymain(void) DO_TEST("pool-dir", "vol-qcow2-lazy"); DO_TEST("pool-dir", "vol-qcow2-0.10-lazy"); DO_TEST("pool-dir", "vol-qcow2-nobacking"); + DO_TEST("pool-dir", "vol-btrfs"); + DO_TEST("pool-dir", "vol-btrfs-snapshot"); DO_TEST("pool-disk", "vol-partition"); DO_TEST("pool-logical", "vol-logical"); DO_TEST("pool-logical", "vol-logical-backing"); -- 1.8.3.1

On 10.09.2013 20:59, 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@ohmu.fi> --- 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 | 222 ++++++++++++++++++++-- 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, 343 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
@@ -866,6 +901,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")) {
or STREQ_NULLABLE
+ 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 ++; + } +#endif +
+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 { + int accessRetCode = -1; + + accessRetCode = access(vol->backingStore.path, R_OK | X_OK);
We need virDirIsExecutable or something like that. make syntax-check is objecting to this line.
+ if (accessRetCode != 0) { + virReportSystemError(errno, + _("inaccessible backing store volume %s"), + 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; +} +
Michal

On 09.10.2013 17:15, Michal Privoznik wrote:
On 10.09.2013 20:59, 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@ohmu.fi> --- 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 | 222 ++++++++++++++++++++-- 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, 343 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
Just to let you and everybody else's know - I'll wait for you to post v2 to this patch (probably worth resending the set) and push the whole new patch set even though the 1/2 is ACKed. The rationale is, the virDirIsExecutable is worth a separate patch - that is the new set will have 3 patches (at least). Michal

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@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 @@ <value>qed</value> <value>vmdk</value> <value>vpc</value> + <value>volume</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 +119,15 @@ <h2><a name="StorageBackendDir">Directory pool</a></h2> <p> A pool with a type of <code>dir</code> provides the means to manage - files within a directory. The files can be fully allocated raw files, - sparsely allocated raw files, or one of the special disk formats - such as <code>qcow</code>,<code>qcow2</code>,<code>vmdk</code>, - <code>cow</code>, etc as supported by the <code>qemu-img</code> - program. If the directory does not exist at the time the pool is - defined, the <code>build</code> operation can be used to create it. + files and filesystem subvolumes within a directory. The files can + be fully allocated raw files, sparsely allocated raw files, or one + of the special disk formats such as <code>qcow</code>, + <code>qcow2</code>, <code>vmdk</code>, <code>cow</code>, etc as + supported by the <code>qemu-img</code> program. The directory can + also contain filesystem subvolumes which show up as + <code>volume</code> format volumes. If the directory does not exist + at the time the pool is defined, the <code>build</code> operation + can be used to create it. </p> <h3>Example pool input definition</h3> @@ -157,6 +160,9 @@ <li><code>qed</code>: QEMU Enhanced Disk image format</li> <li><code>vmdk</code>: VMWare disk image format</li> <li><code>vpc</code>: VirtualPC disk image format</li> + <li><code>volume</code>: Filesystem subvolume (only available on + btrfs <span class="since">since + 1.1.3</span></li> </ul> <p> When listing existing volumes all these formats are supported @@ -166,7 +172,19 @@ either <code>qemu-img</code> or <code>qcow-create</code> tools are present. The others are dependent on support of the <code>qemu-img</code> tool. + </p> + + <h3>Btrfs subvolume management</h3> + <p> + Libvirt can manage btrfs subvolumes and their snapshots in a btrfs + backed directory pool. Such a pool can be used to create subvolumes + and snapshots on the host side and the created volumes can be + attached as filesystems to LXC domains. Information about attaching + a filesystem to a guest can be found at the <a + href="formatdomain.html#elementsFilesystems">format domain</a> page. + Subvolume quotas are not supported at the moment. + <span class="since">Since 1.1.3</span> </p> <h2><a name="StorageBackendFS">Filesystem pool</a></h2> diff --git a/libvirt.spec.in b/libvirt.spec.in index 7abc315..05fedfe 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -517,6 +517,8 @@ BuildRequires: PolicyKit-devel >= 0.6 %if %{with_storage_fs} # For mount/umount in FS driver BuildRequires: util-linux +# For btrfs +BuildRequires: btrfs-progs %endif %if %{with_qemu} # From QEMU RPMs @@ -730,6 +732,8 @@ Requires: device-mapper Requires: device-mapper %endif %if %{with_storage_sheepdog} +# For Btrfs storage +Requires: btrfs-progs # For Sheepdog support Requires: sheepdog %endif 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; }; typedef struct _virStorageVolDef virStorageVolDef; diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index c21e6ea..9f12d16 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1269,13 +1269,16 @@ virStorageBackendUpdateVolInfoFlags(virStorageVolDefPtr vol, openflags)) < 0) return ret; - if (vol->backingStore.path && - (ret = virStorageBackendUpdateVolTargetInfo(&vol->backingStore, - NULL, NULL, - VIR_STORAGE_VOL_OPEN_DEFAULT)) < 0) - return ret; + if (vol->backingStore.path) { + int flags = VIR_STORAGE_VOL_OPEN_DEFAULT; - return 0; + if (vol->backingStore.format == VIR_STORAGE_FILE_VOLUME) + flags |= VIR_STORAGE_VOL_OPEN_DIR; + ret = virStorageBackendUpdateVolTargetInfo(&vol->backingStore, + NULL, NULL, flags); + } + + return ret; } int virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, 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 ++; + } +#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) { + /* update snapshot information */ + virStorageVolDefPtr volv, volb; + int idxv, idxb; + + for (idxv = 0; idxv < pool->volumes.count; idxv ++) { + 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 ++) { + 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 */ + VIR_FREE(volv->backingStore.uuid); + volv->backingStore.uuid = NULL; + volv->backingStore.format = VIR_STORAGE_FILE_NONE; + } + } + } + } +#endif if (statvfs(pool->def->target.path, &sb) < 0) { virReportSystemError(errno, _("cannot statvfs path '%s'"), pool->def->target.path); - return -1; + goto cleanup; } pool->def->capacity = ((unsigned long long)sb.f_frsize * (unsigned long long)sb.f_blocks); @@ -910,14 +993,19 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED, (unsigned long long)sb.f_frsize); pool->def->allocation = pool->def->capacity - pool->def->available; - return 0; + ret = 0; - cleanup: +cleanup: +#if WITH_STORAGE_BTRFS + VIR_FREE(fstype); +#endif if (dir) closedir(dir); virStorageVolDefFree(vol); - virStoragePoolObjClearVols(pool); - return -1; + if (ret < 0) + virStoragePoolObjClearVols(pool); + + return ret; } @@ -1000,10 +1088,90 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn ATTRIBUTE_UNUSED, return -1; } + if (vol->target.format == VIR_STORAGE_FILE_VOLUME) { + vol->type = VIR_STORAGE_VOL_DIR; + if (vol->backingStore.path != NULL) + vol->backingStore.format = VIR_STORAGE_FILE_VOLUME; + } + VIR_FREE(vol->key); return VIR_STRDUP(vol->key, vol->target.path); } +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; + } + VIR_FREE(fstype); + } +#endif if (rmdir(vol->target.path) < 0) { virReportSystemError(errno, _("cannot remove directory '%s'"), 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, 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> diff --git a/tests/storagevolxml2xmlin/vol-btrfs.xml b/tests/storagevolxml2xmlin/vol-btrfs.xml new file mode 100644 index 0000000..e53bc8d --- /dev/null +++ b/tests/storagevolxml2xmlin/vol-btrfs.xml @@ -0,0 +1,9 @@ +<volume> + <name>vanilla</name> + <capacity unit="bytes">0</capacity> + <allocation unit="bytes">0</allocation> + <target> + <path>/lxc/vanilla</path> + <format type='volume'/> + </target> +</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> + <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> + <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> diff --git a/tests/storagevolxml2xmltest.c b/tests/storagevolxml2xmltest.c index 1fd01f1..e84e6a2 100644 --- a/tests/storagevolxml2xmltest.c +++ b/tests/storagevolxml2xmltest.c @@ -116,6 +116,8 @@ mymain(void) DO_TEST("pool-dir", "vol-qcow2-lazy"); DO_TEST("pool-dir", "vol-qcow2-0.10-lazy"); DO_TEST("pool-dir", "vol-qcow2-nobacking"); + DO_TEST("pool-dir", "vol-btrfs"); + DO_TEST("pool-dir", "vol-btrfs-snapshot"); DO_TEST("pool-disk", "vol-partition"); DO_TEST("pool-logical", "vol-logical"); DO_TEST("pool-logical", "vol-logical-backing"); -- 1.8.3.1

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@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 :|

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_XXXX 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 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>
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 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
I'll look into this. Thanks, Oskari

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

On Tue, Sep 10, 2013 at 09:56:48PM +0300, Oskari Saarenmaa wrote:
Date: Tue, 10 Sep 2013 21:56:48 +0300 From: Oskari Saarenmaa <os@ohmu.fi> To: libvir-list@redhat.com Subject: [libvirt] [PATCH 0/2] btrfs subvolume management
Moved btrfs subvolume management to storage_backend_fs.c instead of implementing it as a separate pool as suggested by Daniel P. Berrange in https://www.redhat.com/archives/libvir-list/2013-September/msg00316.html
Ping, anyone had a chance to review this yet? Thanks, Oskari
participants (4)
-
Daniel P. Berrange
-
Eric Blake
-
Michal Privoznik
-
Oskari Saarenmaa