Re: [RFC PATCH] storage: initial support for linking with libglfapi

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]