We support gluster volumes in domain XML, so we also ought to support them as a storage pool. Besides, a future patch will want to take advantage of libgfapi to handle the case of a gluster device holding qcow2 rather than raw storage, and for that to work, we need a storage backend that can read gluster storage volume contents. This sets up the framework. Note that the new pool is named 'gluster' to match a <disk type='network'><source protocol='gluster'> image source already supported in a <domain>; it does NOT match the <pool type='netfs'><source><target type='glusterfs'>, since that uses a FUSE mount to a local file name rather than a network name. This and subsequent patches have been tested against glusterfs 3.4.1 (available on Fedora 19); there are likely bugs in older versions that may prevent decent use of gfapi, so this patch enforces the minimum version tested. A future patch may lower the minimum. On the other hand, I hit at least two bugs in 3.4.1 that will be fixed in 3.5/3.4.2, where it might be worth raising the minimum: glfs_readdir is nicer to use than glfs_readdir_r [1], and glfs_fini should only return failure on an actual failure [2]. [1] http://lists.gnu.org/archive/html/gluster-devel/2013-10/msg00085.html [2] http://lists.gnu.org/archive/html/gluster-devel/2013-10/msg00086.html * configure.ac (WITH_STORAGE_GLUSTER): New conditional. * m4/virt-gluster.m4: new file. * libvirt.spec.in (BuildRequires): Support gluster in spec file. * src/conf/storage_conf.h (VIR_STORAGE_POOL_GLUSTER): New pool type. * src/conf/storage_conf.c (poolTypeInfo): Treat similar to sheepdog and rbd. (virStoragePoolDefFormat): Don't output target for gluster. * src/storage/storage_backend_gluster.h: New file. * src/storage/storage_backend_gluster.c: Likewise. * po/POTFILES.in: Add new file. * src/storage/storage_backend.c (backends): Register new type. * src/Makefile.am (STORAGE_DRIVER_GLUSTER_SOURCES): Build new files. * src/storage/storage_backend.h (_virStorageBackend): Documet assumption. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- configure.ac | 21 ++++++++++++++++ libvirt.spec.in | 15 ++++++++++++ m4/virt-gluster.m4 | 28 +++++++++++++++++++++ po/POTFILES.in | 1 + src/Makefile.am | 10 ++++++++ src/conf/storage_conf.c | 26 +++++++++++++++++--- src/conf/storage_conf.h | 3 ++- src/storage/storage_backend.c | 6 +++++ src/storage/storage_backend.h | 6 +++-- src/storage/storage_backend_gluster.c | 46 +++++++++++++++++++++++++++++++++++ src/storage/storage_backend_gluster.h | 29 ++++++++++++++++++++++ 11 files changed, 184 insertions(+), 7 deletions(-) create mode 100644 m4/virt-gluster.m4 create mode 100644 src/storage/storage_backend_gluster.c create mode 100644 src/storage/storage_backend_gluster.h diff --git a/configure.ac b/configure.ac index 044cf37..934c87e 100644 --- a/configure.ac +++ b/configure.ac @@ -228,6 +228,7 @@ LIBVIRT_CHECK_CAPNG LIBVIRT_CHECK_CURL LIBVIRT_CHECK_DBUS LIBVIRT_CHECK_FUSE +LIBVIRT_CHECK_GLUSTER LIBVIRT_CHECK_HAL LIBVIRT_CHECK_NETCF LIBVIRT_CHECK_NUMACTL @@ -1643,6 +1644,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-gluster], + [AS_HELP_STRING([--with-storage-gluster], + [with Gluster backend for the storage driver @<:@default=check@:>@])], + [],[with_storage_gluster=check]) if test "$with_libvirtd" = "no"; then with_storage_dir=no @@ -1654,6 +1659,7 @@ if test "$with_libvirtd" = "no"; then with_storage_disk=no with_storage_rbd=no with_storage_sheepdog=no + with_storage_gluster=no fi if test "$with_storage_dir" = "yes" ; then AC_DEFINE_UNQUOTED([WITH_STORAGE_DIR], 1, [whether directory backend for storage driver is enabled]) @@ -1855,6 +1861,19 @@ fi AM_CONDITIONAL([WITH_STORAGE_SHEEPDOG], [test "$with_storage_sheepdog" = "yes"]) +LIBGLUSTER_LIBS= +if test "$with_storage_gluster" = "check"; then + with_storage_gluster=$with_glusterfs +fi +if test "$with_storage_gluster" = "yes"; then + if test "$with_glusterfs" = no; then + AC_MSG_ERROR([Need glusterfs (libgfapi) for gluster storage driver]) + fi + AC_DEFINE_UNQUOTED([WITH_STORAGE_GLUSTER], [1], + [whether Gluster backend for storage driver is enabled]) +fi +AM_CONDITIONAL([WITH_STORAGE_GLUSTER], [test "$with_storage_gluster" = "yes"]) + LIBPARTED_CFLAGS= LIBPARTED_LIBS= @@ -2582,6 +2601,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([ Gluster: $with_storage_gluster]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Security Drivers]) AC_MSG_NOTICE([]) @@ -2607,6 +2627,7 @@ LIBVIRT_RESULT_CAPNG LIBVIRT_RESULT_CURL LIBVIRT_RESULT_DBUS LIBVIRT_RESULT_FUSE +LIBVIRT_RESULT_GLUSTER LIBVIRT_RESULT_HAL LIBVIRT_RESULT_NETCF LIBVIRT_RESULT_NUMACTL diff --git a/libvirt.spec.in b/libvirt.spec.in index 5bc53a0..760148c 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -98,6 +98,11 @@ %else %define with_storage_sheepdog 0 %endif +%if 0%{?fedora} >= 19 + %define with_storage_gluster 0%{!?_without_storage_gluster:%{server_drivers}} +%else + %define with_storage_gluster 0 +%endif %define with_numactl 0%{!?_without_numactl:%{server_drivers}} %define with_selinux 0%{!?_without_selinux:%{server_drivers}} @@ -280,6 +285,7 @@ %define with_storage_mpath 0 %define with_storage_rbd 0 %define with_storage_sheepdog 0 + %define with_storage_gluster 0 %define with_storage_disk 0 %endif @@ -553,6 +559,10 @@ BuildRequires: device-mapper-devel BuildRequires: ceph-devel %endif %endif +%if %{with_storage_gluster} +BuildRequires: glusterfs-api-devel >= 3.4.1 +BuildRequires: glusterfs-devel >= 3.4.1 +%endif %if %{with_numactl} # For QEMU/LXC numa info BuildRequires: numactl-devel @@ -1255,6 +1265,10 @@ driver %define _without_storage_sheepdog --without-storage-sheepdog %endif +%if ! %{with_storage_gluster} + %define _without_storage_gluster --without-storage-gluster +%endif + %if ! %{with_numactl} %define _without_numactl --without-numactl %endif @@ -1376,6 +1390,7 @@ driver %{?_without_storage_mpath} \ %{?_without_storage_rbd} \ %{?_without_storage_sheepdog} \ + %{?_without_storage_gluster} \ %{?_without_numactl} \ %{?_without_numad} \ %{?_without_capng} \ diff --git a/m4/virt-gluster.m4 b/m4/virt-gluster.m4 new file mode 100644 index 0000000..5a4a263 --- /dev/null +++ b/m4/virt-gluster.m4 @@ -0,0 +1,28 @@ +dnl The gluster libgfapi.so library +dnl +dnl Copyright (C) 2013 Red Hat, Inc. +dnl +dnl This library is free software; you can redistribute it and/or +dnl modify it under the terms of the GNU Lesser General Public +dnl License as published by the Free Software Foundation; either +dnl version 2.1 of the License, or (at your option) any later version. +dnl +dnl This library is distributed in the hope that it will be useful, +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +dnl Lesser General Public License for more details. +dnl +dnl You should have received a copy of the GNU Lesser General Public +dnl License along with this library. If not, see +dnl <http://www.gnu.org/licenses/>;. +dnl + +dnl Currently tested against Fedora 19 with glusterfs 3.4.1; earlier +dnl versions may be possible but only with further testing +AC_DEFUN([LIBVIRT_CHECK_GLUSTER],[ + LIBVIRT_CHECK_PKG([GLUSTERFS], [glusterfs-api], [3.4.1]) +]) + +AC_DEFUN([LIBVIRT_RESULT_GLUSTER],[ + LIBVIRT_RESULT_LIB([GLUSTERFS]) +]) diff --git a/po/POTFILES.in b/po/POTFILES.in index 15afdec..0835b52 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -133,6 +133,7 @@ src/storage/parthelper.c src/storage/storage_backend.c src/storage/storage_backend_disk.c src/storage/storage_backend_fs.c +src/storage/storage_backend_gluster.c src/storage/storage_backend_iscsi.c src/storage/storage_backend_logical.c src/storage/storage_backend_mpath.c diff --git a/src/Makefile.am b/src/Makefile.am index ad39b74..e9fd14d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -819,6 +819,9 @@ STORAGE_DRIVER_RBD_SOURCES = \ STORAGE_DRIVER_SHEEPDOG_SOURCES = \ storage/storage_backend_sheepdog.h storage/storage_backend_sheepdog.c +STORAGE_DRIVER_GLUSTER_SOURCES = \ + storage/storage_backend_gluster.h storage/storage_backend_gluster.c + STORAGE_HELPER_DISK_SOURCES = \ storage/parthelper.c @@ -1434,6 +1437,12 @@ if WITH_STORAGE_SHEEPDOG libvirt_driver_storage_impl_la_SOURCES += $(STORAGE_DRIVER_SHEEPDOG_SOURCES) endif WITH_STORAGE_SHEEPDOG +if WITH_STORAGE_GLUSTER +libvirt_driver_storage_impl_la_SOURCES += $(STORAGE_DRIVER_GLUSTER_SOURCES) +libvirt_driver_storage_impl_la_CFLAGS += $(GLUSTERFS_CFLAGS) +libvirt_driver_storage_impl_la_LIBADD += $(GLUSTERFS_LIBS) +endif WITH_STORAGE_GLUSTER + if WITH_NODE_DEVICES # Needed to keep automake quiet about conditionals if WITH_DRIVER_MODULES @@ -1634,6 +1643,7 @@ EXTRA_DIST += \ $(STORAGE_DRIVER_DISK_SOURCES) \ $(STORAGE_DRIVER_RBD_SOURCES) \ $(STORAGE_DRIVER_SHEEPDOG_SOURCES) \ + $(STORAGE_DRIVER_GLUSTER_SOURCES) \ $(NODE_DEVICE_DRIVER_SOURCES) \ $(NODE_DEVICE_DRIVER_HAL_SOURCES) \ $(NODE_DEVICE_DRIVER_UDEV_SOURCES) \ diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 0d2932b..add2ae1 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -59,7 +59,7 @@ VIR_ENUM_IMPL(virStoragePool, VIR_STORAGE_POOL_LAST, "dir", "fs", "netfs", "logical", "disk", "iscsi", - "scsi", "mpath", "rbd", "sheepdog") + "scsi", "mpath", "rbd", "sheepdog", "gluster") VIR_ENUM_IMPL(virStoragePoolFormatFileSystem, VIR_STORAGE_POOL_FS_LAST, @@ -248,6 +248,19 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { .formatToString = virStoragePoolFormatDiskTypeToString, } }, + {.poolType = VIR_STORAGE_POOL_GLUSTER, + .poolOptions = { + .flags = (VIR_STORAGE_POOL_SOURCE_HOST | + VIR_STORAGE_POOL_SOURCE_NETWORK | + VIR_STORAGE_POOL_SOURCE_NAME | + VIR_STORAGE_POOL_SOURCE_DIR), + }, + .volOptions = { + .defaultFormat = VIR_STORAGE_FILE_RAW, + .formatToString = virStorageFileFormatTypeToString, + .formatFromString = virStorageVolumeFormatFromString, + } + }, {.poolType = VIR_STORAGE_POOL_MPATH, .volOptions = { .formatToString = virStoragePoolFormatDiskTypeToString, @@ -652,6 +665,10 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, } source->dir = virXPathString("string(./dir/@path)", ctxt); + /* In gluster, a missing dir defaults to "/" */ + if (!source->dir && pool_type == VIR_STORAGE_POOL_GLUSTER && + VIR_STRDUP(source->dir, "/") < 0) + goto cleanup; if ((adapter_type = virXPathString("string(./adapter/@type)", ctxt))) { if ((source->adapter.type = @@ -1196,10 +1213,11 @@ virStoragePoolDefFormat(virStoragePoolDefPtr def) if (virStoragePoolSourceFormat(&buf, options, &def->source) < 0) goto cleanup; - /* RBD and Sheepdog devices are not local block devs nor files, so it - * doesn't have a target */ + /* RBD, Sheepdog, and Gluster devices are not local block devs nor + * files, so they don't have a target */ if (def->type != VIR_STORAGE_POOL_RBD && - def->type != VIR_STORAGE_POOL_SHEEPDOG) { + def->type != VIR_STORAGE_POOL_SHEEPDOG && + def->type != VIR_STORAGE_POOL_GLUSTER) { virBufferAddLit(&buf, " <target>\n"); virBufferEscapeString(&buf, " <path>%s</path>\n", def->target.path); diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index c4dd403..f8a7eec 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -1,7 +1,7 @@ /* * storage_conf.h: config handling for storage driver * - * Copyright (C) 2006-2008, 2010-2012 Red Hat, Inc. + * Copyright (C) 2006-2008, 2010-2013 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -129,6 +129,7 @@ enum virStoragePoolType { VIR_STORAGE_POOL_MPATH, /* Multipath devices */ VIR_STORAGE_POOL_RBD, /* RADOS Block Device */ VIR_STORAGE_POOL_SHEEPDOG, /* Sheepdog device */ + VIR_STORAGE_POOL_GLUSTER, /* Gluster device */ VIR_STORAGE_POOL_LAST, }; diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 1e3292b..c94e89a 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -80,6 +80,9 @@ #if WITH_STORAGE_SHEEPDOG # include "storage_backend_sheepdog.h" #endif +#if WITH_STORAGE_GLUSTER +# include "storage_backend_gluster.h" +#endif #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -112,6 +115,9 @@ static virStorageBackendPtr backends[] = { #if WITH_STORAGE_SHEEPDOG &virStorageBackendSheepdog, #endif +#if WITH_STORAGE_GLUSTER + &virStorageBackendGluster, +#endif NULL }; diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index d8d3097..9e07dd8 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -1,7 +1,7 @@ /* * storage_backend.h: internal storage driver backend contract * - * Copyright (C) 2007-2010, 2012 Red Hat, Inc. + * Copyright (C) 2007-2010, 2012-2013 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -70,6 +70,8 @@ virStorageBackendFSImageToolTypeToFunc(int tool_type); typedef struct _virStorageBackend virStorageBackend; typedef virStorageBackend *virStorageBackendPtr; +/* Callbacks are optional unless documented otherwise; but adding more + * callbacks provides better pool support. */ struct _virStorageBackend { int type; @@ -77,7 +79,7 @@ struct _virStorageBackend { virStorageBackendCheckPool checkPool; virStorageBackendStartPool startPool; virStorageBackendBuildPool buildPool; - virStorageBackendRefreshPool refreshPool; + virStorageBackendRefreshPool refreshPool; /* Must be non-NULL */ virStorageBackendStopPool stopPool; virStorageBackendDeletePool deletePool; diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c new file mode 100644 index 0000000..2863c73 --- /dev/null +++ b/src/storage/storage_backend_gluster.c @@ -0,0 +1,46 @@ +/* + * storage_backend_gluster.c: storage backend for Gluster handling + * + * Copyright (C) 2013 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + */ + +#include <config.h> + +#include <glusterfs/api/glfs.h> + +#include "virerror.h" +#include "storage_backend_gluster.h" +#include "storage_conf.h" + +#define VIR_FROM_THIS VIR_FROM_STORAGE + + +static int +virStorageBackendGlusterRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool ATTRIBUTE_UNUSED) +{ + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("gluster pool type not fully supported yet")); + return -1; +} + +virStorageBackend virStorageBackendGluster = { + .type = VIR_STORAGE_POOL_GLUSTER, + + .refreshPool = virStorageBackendGlusterRefreshPool, +}; diff --git a/src/storage/storage_backend_gluster.h b/src/storage/storage_backend_gluster.h new file mode 100644 index 0000000..b21bda7 --- /dev/null +++ b/src/storage/storage_backend_gluster.h @@ -0,0 +1,29 @@ +/* + * storage_backend_gluster.h: storage backend for Gluster handling + * + * Copyright (C) 2013 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + */ + +#ifndef __VIR_STORAGE_BACKEND_GLUSTER_H__ +# define __VIR_STORAGE_BACKEND_GLUSTER_H__ + +# include "storage_backend.h" + +extern virStorageBackend virStorageBackendGluster; + +#endif /* __VIR_STORAGE_BACKEND_GLUSTER_H__ */ -- 1.8.3.1

On Fri, Nov 22, 2013 at 08:20:24PM -0700, Eric Blake wrote: well 1.1.5 will become 1.2.0 actually so just change the 2 occurences 1.2.0 That patch set got a lot of review already (though it seems to have increased slighty since v3), but i would love to get it in 1.2.0. Since Peter and Dan fully reviewed it last round, and assuming you handled all comments back, I suggest to push before tomorrow freeze :-) ACK, Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veillard(a)redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/

In the 'directory' and 'netfs' storage pools, a user can see both 'file' and 'dir' storage volume types, to know when they can descend into a subdirectory. But in a network-based storage pool, such as the upcoming 'gluster' pool, we use 'network' instead of 'file', and did not have any counterpart for a directory until this patch. Adding a new volume type 'network-dir' is better than reusing 'dir', because it makes it clear that the only way to access 'network' volumes within that container is through the network mounting (leaving 'dir' for something accessible in the local file system). * include/libvirt/libvirt.h.in (virStorageVolType): Expand enum. * src/qemu/qemu_command.c (qemuBuildVolumeString): Fix client. * src/qemu/qemu_conf.c (qemuTranslateDiskSourcePool): Likewise. * tools/virsh-volume.c (vshVolumeTypeToString): Likewise. * src/storage/storage_backend_fs.c (virStorageBackendFileSystemVolDelete): Likewise. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- include/libvirt/libvirt.h.in | 2 ++ src/conf/storage_conf.c | 2 +- src/qemu/qemu_command.c | 6 ++++-- src/qemu/qemu_conf.c | 4 +++- src/storage/storage_backend_fs.c | 5 +++-- tools/virsh-volume.c | 5 ++++- 6 files changed, 17 insertions(+), 7 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 146a59b..5e8cba6 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2951,6 +2951,8 @@ typedef enum { VIR_STORAGE_VOL_BLOCK = 1, /* Block based volumes */ VIR_STORAGE_VOL_DIR = 2, /* Directory-passthrough based volume */ VIR_STORAGE_VOL_NETWORK = 3, /* Network volumes like RBD (RADOS Block Device) */ + VIR_STORAGE_VOL_NETWORK_DIR = 4, /* Network accessible directory that can + * contain other network volumes */ #ifdef VIR_ENUM_SENTINELS VIR_STORAGE_VOL_LAST diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index add2ae1..493a874 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -53,7 +53,7 @@ VIR_ENUM_IMPL(virStorageVol, VIR_STORAGE_VOL_LAST, - "file", "block", "dir", "network") + "file", "block", "dir", "network", "network-dir") VIR_ENUM_IMPL(virStoragePool, VIR_STORAGE_POOL_LAST, diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8dc7e43..711adcb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3782,7 +3782,7 @@ qemuBuildVolumeString(virConnectPtr conn, { int ret = -1; - switch (disk->srcpool->voltype) { + switch ((virStorageVolType) disk->srcpool->voltype) { case VIR_STORAGE_VOL_DIR: if (!disk->readonly) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -3825,10 +3825,12 @@ qemuBuildVolumeString(virConnectPtr conn, } break; case VIR_STORAGE_VOL_NETWORK: + case VIR_STORAGE_VOL_NETWORK_DIR: + case VIR_STORAGE_VOL_LAST: /* Keep the compiler quiet, qemuTranslateDiskSourcePool already * reported the unsupported error. */ - break; + goto cleanup; } ret = 0; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 5803850..3c1e248 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1332,7 +1332,7 @@ qemuTranslateDiskSourcePool(virConnectPtr conn, goto cleanup; } - switch (info.type) { + switch ((virStorageVolType) info.type) { case VIR_STORAGE_VOL_FILE: case VIR_STORAGE_VOL_DIR: if (!(def->src = virStorageVolGetPath(vol))) @@ -1377,6 +1377,8 @@ qemuTranslateDiskSourcePool(virConnectPtr conn, break; case VIR_STORAGE_VOL_NETWORK: + case VIR_STORAGE_VOL_NETWORK_DIR: + case VIR_STORAGE_VOL_LAST: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Using network volume as disk source is not supported")); goto cleanup; diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 510a3d6..2af6faf 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1137,7 +1137,7 @@ virStorageBackendFileSystemVolDelete(virConnectPtr conn ATTRIBUTE_UNUSED, { virCheckFlags(0, -1); - switch (vol->type) { + switch ((virStorageVolType) vol->type) { case VIR_STORAGE_VOL_FILE: if (unlink(vol->target.path) < 0) { /* Silently ignore failures where the vol has already gone away */ @@ -1159,7 +1159,8 @@ virStorageBackendFileSystemVolDelete(virConnectPtr conn ATTRIBUTE_UNUSED, break; case VIR_STORAGE_VOL_BLOCK: case VIR_STORAGE_VOL_NETWORK: - default: + case VIR_STORAGE_VOL_NETWORK_DIR: + case VIR_STORAGE_VOL_LAST: virReportError(VIR_ERR_NO_SUPPORT, _("removing block or network volumes is not supported: %s"), vol->target.path); diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index cbdb3f0..66d9922 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -947,7 +947,7 @@ out: static const char * vshVolumeTypeToString(int type) { - switch (type) { + switch ((virStorageVolType) type) { case VIR_STORAGE_VOL_FILE: return N_("file"); @@ -960,6 +960,9 @@ vshVolumeTypeToString(int type) case VIR_STORAGE_VOL_NETWORK: return N_("network"); + case VIR_STORAGE_VOL_NETWORK_DIR: + return N_("net-dir"); + case VIR_STORAGE_VOL_LAST: break; } -- 1.8.3.1

On Mon, Nov 25, 2013 at 10:42:30AM +0000, Daniel P. Berrange wrote: Could we at least shorten it to 'netdir' ? 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 :|

Take advantage of the previous patch's addition of 'network-dir' as a distinct volume type, to expose rather than silently skip directories embedded in a gluster pool. Also serves as an XML validation for the previous patch. * src/storage/storage_backend_gluster.c (virStorageBackendGlusterRefreshVol): Don't skip directories. * tests/storagevolxml2xmltest.c (mymain): Add test. * tests/storagevolxml2xmlin/vol-gluster-dir.xml: New file. * tests/storagevolxml2xmlout/vol-gluster-dir.xml: Likewise. --- src/storage/storage_backend_gluster.c | 14 ++++++++++---- tests/storagevolxml2xmlin/vol-gluster-dir.xml | 13 +++++++++++++ tests/storagevolxml2xmlout/vol-gluster-dir.xml | 18 ++++++++++++++++++ tests/storagevolxml2xmltest.c | 1 + 4 files changed, 42 insertions(+), 4 deletions(-) create mode 100644 tests/storagevolxml2xmlin/vol-gluster-dir.xml create mode 100644 tests/storagevolxml2xmlout/vol-gluster-dir.xml diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 120cf0f..89abb57 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -165,9 +165,6 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, /* Silently skip '.' and '..'. */ if (STREQ(name, ".") || STREQ(name, "..")) return 0; - /* FIXME: support directories. For now, silently skip them. */ - if (S_ISDIR(st->st_mode)) - return 0; if (VIR_ALLOC(vol) < 0) goto cleanup; @@ -177,7 +174,6 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, vol->name) < 0) goto cleanup; - vol->type = VIR_STORAGE_VOL_NETWORK; tmp = state->uri->path; state->uri->path = vol->key; if (!(vol->target.path = virURIFormat(state->uri))) { @@ -186,7 +182,17 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, } state->uri->path = tmp; + if (S_ISDIR(st->st_mode)) { + vol->type = VIR_STORAGE_VOL_NETWORK_DIR; + vol->target.format = VIR_STORAGE_FILE_DIR; + *volptr = vol; + vol = NULL; + ret = 0; + goto cleanup; + } + /* FIXME - must open files to determine if they are non-raw */ + vol->type = VIR_STORAGE_VOL_NETWORK; vol->target.format = VIR_STORAGE_FILE_RAW; vol->capacity = vol->allocation = st->st_size; diff --git a/tests/storagevolxml2xmlin/vol-gluster-dir.xml b/tests/storagevolxml2xmlin/vol-gluster-dir.xml new file mode 100644 index 0000000..bd20a6a --- /dev/null +++ b/tests/storagevolxml2xmlin/vol-gluster-dir.xml @@ -0,0 +1,13 @@ +<volume> + <name>dir</name> + <key>/vol/dir</key> + <source> + </source> + <type>network-dir</type> + <capacity unit='bytes'>0</capacity> + <allocation unit='bytes'>0</allocation> + <target> + <format type='dir'/> + <path>gluster://example.com/vol/dir</path> + </target> +</volume> diff --git a/tests/storagevolxml2xmlout/vol-gluster-dir.xml b/tests/storagevolxml2xmlout/vol-gluster-dir.xml new file mode 100644 index 0000000..29e6d1a --- /dev/null +++ b/tests/storagevolxml2xmlout/vol-gluster-dir.xml @@ -0,0 +1,18 @@ +<volume> + <name>dir</name> + <key>/vol/dir</key> + <source> + </source> + <type>network-dir</type> + <capacity unit='bytes'>0</capacity> + <allocation unit='bytes'>0</allocation> + <target> + <path>gluster://example.com/vol/dir</path> + <format type='dir'/> + <permissions> + <mode>0600</mode> + <owner>4294967295</owner> + <group>4294967295</group> + </permissions> + </target> +</volume> diff --git a/tests/storagevolxml2xmltest.c b/tests/storagevolxml2xmltest.c index e1db465..fdcdea1 100644 --- a/tests/storagevolxml2xmltest.c +++ b/tests/storagevolxml2xmltest.c @@ -121,6 +121,7 @@ mymain(void) DO_TEST("pool-logical", "vol-logical"); DO_TEST("pool-logical", "vol-logical-backing"); DO_TEST("pool-sheepdog", "vol-sheepdog"); + DO_TEST("pool-gluster", "vol-gluster-dir"); return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 1.8.3.1

On 11/25/2013 08:56 AM, Daniel P. Berrange wrote: Indeed, 'make check' fails after my tweaks earlier in the series unless I squash in this (now pushed): diff --git i/src/storage/storage_backend_gluster.c w/src/storage/storage_backend_gluster.c index 51dc742..3f4e9f7 100644 --- i/src/storage/storage_backend_gluster.c +++ w/src/storage/storage_backend_gluster.c @@ -183,7 +183,7 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, state->uri->path = tmp; if (S_ISDIR(st->st_mode)) { - vol->type = VIR_STORAGE_VOL_NETWORK_DIR; + vol->type = VIR_STORAGE_VOL_NETDIR; vol->target.format = VIR_STORAGE_FILE_DIR; *volptr = vol; vol = NULL; diff --git i/tests/storagevolxml2xmlin/vol-gluster-dir.xml w/tests/storagevolxml2xmlin/vol-gluster-dir.xml index bd20a6a..208c2c2 100644 --- i/tests/storagevolxml2xmlin/vol-gluster-dir.xml +++ w/tests/storagevolxml2xmlin/vol-gluster-dir.xml @@ -1,9 +1,8 @@ -<volume> +<volume type='netdir'> <name>dir</name> - <key>/vol/dir</key> + <key>vol/dir</key> <source> </source> - <type>network-dir</type> <capacity unit='bytes'>0</capacity> <allocation unit='bytes'>0</allocation> <target> diff --git i/tests/storagevolxml2xmlout/vol-gluster-dir.xml w/tests/storagevolxml2xmlout/vol-gluster-dir.xml index 29e6d1a..f188ceb 100644 --- i/tests/storagevolxml2xmlout/vol-gluster-dir.xml +++ w/tests/storagevolxml2xmlout/vol-gluster-dir.xml @@ -1,9 +1,8 @@ -<volume> +<volume type='netdir'> <name>dir</name> - <key>/vol/dir</key> + <key>vol/dir</key> <source> </source> - <type>network-dir</type> <capacity unit='bytes'>0</capacity> <allocation unit='bytes'>0</allocation> <target> -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

With this patch, dangling and looping symlinks are silently ignored, while links to files and directories are treated the same as the underlying file or directory. This is the same behavior as both 'directory' and 'netfs' pools. * src/storage/storage_backend_gluster.c (virStorageBackendGlusterRefreshVol): Treat symlinks similar to directory and netfs pools. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- src/storage/storage_backend_gluster.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index a4fd4b3..e935583 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -166,6 +166,17 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, if (STREQ(name, ".") || STREQ(name, "..")) return 0; + /* Follow symlinks; silently skip broken links and loops. */ + if (S_ISLNK(st->st_mode) && glfs_stat(state->vol, name, st) < 0) { + if (errno == ENOENT || errno == ELOOP) { + VIR_WARN("ignoring dangling symlink '%s'", name); + ret = 0; + } else { + virReportSystemError(errno, _("cannot stat '%s'"), name); + } + return ret; + } + if (VIR_ALLOC(vol) < 0) goto cleanup; -- 1.8.3.1

On Fri, Nov 22, 2013 at 08:20:30PM -0700, Eric Blake wrote: Further down you're requesting O_NONBLOCK, and here you are not handling EAGAIN explicitly. Is is desirable that we turn EAGAIN into a fatal error, or should we remove the O_NONBLOCK flag ? 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 :|