On Mon, Nov 11, 2013 at 09:19:28PM -0700, Eric Blake wrote:
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index 72815f4..a90ee2b 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -555,6 +561,10 @@ BuildRequires: device-mapper-devel
> BuildRequires: ceph-devel
> %endif
> %endif
> +%if %{with_storage_gluster}
> +BuildRequires: glusterfs-api-devel
> +BuildRequires: glusterfs-devel
> +%endif
This accepts any version of the packages
> diff --git a/m4/virt-gluster.m4 b/m4/virt-gluster.m4
> new file mode 100644
> index 0000000..4851e17
> --- /dev/null
> +++ b/m4/virt-gluster.m4
> +
> +AC_DEFUN([LIBVIRT_CHECK_GLUSTER],[
> + LIBVIRT_CHECK_PKG([GLFS], [glusterfs-api], [3.0])
> +])
But this suggests only version >= 3.0 is acceptable. Should we make
the RPM dep explicit for this.
I think I'd prefer s/GLFS/GLUSTERFS/ - its only used in a few
places, so being more verbose doesn't hurt us much.
> diff --git a/src/Makefile.am b/src/Makefile.am
> index ad39b74..be9978a 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -1434,6 +1437,11 @@ 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_LIBADD += $(GLFS_LIBS)
Hmm, I don't see CFLAGS ever mentioning GLFS_CFLAGS which could
cause problems for anyone who has installed gluster outside of
the /usr hiearchy.
> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> index 975e662..8299f2a 100644
> --- a/src/conf/storage_conf.c
> +++ b/src/conf/storage_conf.c
> @@ -55,7 +55,7 @@ VIR_ENUM_IMPL(virStoragePool,
> VIR_STORAGE_POOL_LAST,
> "dir", "fs", "netfs",
> "logical", "disk", "iscsi",
> - "scsi", "mpath", "rbd", "sheepdog")
> + "scsi", "mpath", "rbd", "sheepdog", "gluster")
Hmm, well that sucks - in some places we use 'gluster' and others
we use 'glusterfs' :-( We've released code using both so we can't
standardize everywhere now.
So guess we should stick with what you have here.
> + {.poolType = VIR_STORAGE_POOL_GLUSTER,
> + .poolOptions = {
> + .flags = (VIR_STORAGE_POOL_SOURCE_HOST |
> + VIR_STORAGE_POOL_SOURCE_NETWORK |
> + VIR_STORAGE_POOL_SOURCE_NAME),
> + },
Are gluster volume names always wellformed paths ? If so is
it worth using SOURCE_PATH instad of SOURCE_NAME ? I don't
feel too strongly either way.
> + .volOptions = {
> + .defaultFormat = VIR_STORAGE_FILE_RAW,
> + .formatToString = virStorageFileFormatTypeToString,
> + }
> + },
> {.poolType = VIR_STORAGE_POOL_MPATH,
> .volOptions = {
> .formatToString = virStoragePoolFormatDiskTypeToString,
ACK if you fix the nitpicks.
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