On Mon, Oct 14, 2013 at 02:12:33PM -0600, Eric Blake wrote:
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.
* configure.ac (WITH_STORAGE_GLUSTER): New conditional. * libvirt.spec.in (BuildRequires): Support it 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. * src/storage/storage_backend_gluster.h: New file. * src/storage/storage_backend_gluster.c: Likewise. * src/storage/storage_backend.c (backends): Register new type. * src/Makefile.am (STORAGE_DRIVER_GLUSTER_SOURCES): Build new files.
Signed-off-by: Eric Blake <eblake@redhat.com> ---
I'm interested in a couple of things beyond this patch: representing a storage pool on top of gluster, and improving the libvirt backing chain detection to allow a gluster pool to contain qcow2 rather than raw format data (ie. where one gluster image can call out another gluster file as its backing, rather than forcefully treating all network files as raw). Without at least one of those working, this patch should not be applied in isolation. But I figured early review is better to make sure I'm on track.
configure.ac | 32 ++++++++++++++++++++++++++++++++ libvirt.spec.in | 15 +++++++++++++++ src/Makefile.am | 9 +++++++++ src/conf/storage_conf.c | 13 ++++++++++++- src/conf/storage_conf.h | 3 ++- src/storage/storage_backend.c | 6 ++++++ src/storage/storage_backend_gluster.c | 35 +++++++++++++++++++++++++++++++++++ src/storage/storage_backend_gluster.h | 29 +++++++++++++++++++++++++++++ 8 files changed, 140 insertions(+), 2 deletions(-) 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 1993fab..3b7fde7 100644 --- a/configure.ac +++ b/configure.ac @@ -118,6 +118,7 @@ PARTED_REQUIRED="1.8.0" DEVMAPPER_REQUIRED=1.0.0 LIBPCAP_REQUIRED="1.0.0" LIBNL_REQUIRED="1.1" +GLFS_REQUIRED="3.0"
dnl Checks for C compiler. AC_PROG_CC @@ -1646,6 +1647,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 @@ -1657,6 +1662,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]) @@ -1858,6 +1864,26 @@ fi AM_CONDITIONAL([WITH_STORAGE_SHEEPDOG], [test "$with_storage_sheepdog" = "yes"])
+LIBGLUSTER_LIBS= +if test "$with_storage_gluster" = "yes" || \ + test "$with_storage_gluster" = "check"; then + PKG_CHECK_MODULES([GLFS], [glusterfs-api >= $GLFS_REQUIRED], + [GLFS_FOUND=yes], [GLFS_FOUND=no]) + + AC_CHECK_HEADER([glusterfs/api/glfs.h], [GLFS_FOUND=yes; break;]) + + if test "$GLFS_FOUND" = "yes"; then + with_storage_gluster=yes + LIBGLUSTER_LIBS=$GLFS_LIBS + AC_DEFINE_UNQUOTED([WITH_STORAGE_GLUSTER], [1], + [whether Gluster backend for storage driver is enabled]) + else + with_storage_gluster=no + fi +fi +AM_CONDITIONAL([WITH_STORAGE_GLUSTER], [test "$with_storage_gluster" = "yes"]) +AC_SUBST([LIBGLUSTER_LIBS])
Can we get this done in m4/virt-gluster.m4 as just a generic library check. And then have a separate code to handle 'with_storage_gluster' so that we don't couple detection of the library to enablement of the storage driver backend.
diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c new file mode 100644 index 0000000..deb44dc --- /dev/null +++ b/src/storage/storage_backend_gluster.c @@ -0,0 +1,35 @@ +/* + * 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 + + +virStorageBackend virStorageBackendGluster = { + .type = VIR_STORAGE_POOL_GLUSTER, +};
I tend to think that the minimum requirement for adding a storage driver backend is to be able to enumerate volumes. I don't know if there's any mileage in simply copying the content of storage_backend_rbd.c and replacing the rbd API calls with gluster API calls ? Depends how similar their APIs are in style I guess. Regards, 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 :|