Re: [Qemu-devel] [PATCH v3] block/gluster: Handle changed glfs_ftruncate signature

[Niels de Vos <ndevos@redhat.com>]

On Sat, Jul 28, 2018 at 12:18:39AM -0400, Jeff Cody wrote:
> On Fri, Jul 27, 2018 at 08:24:05AM -0500, Eric Blake wrote:
> > On 07/27/2018 03:19 AM, Niels de Vos wrote:
> > >From: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
> > >
> > >New versions of Glusters libgfapi.so have an updated glfs_ftruncate()
> > >function that returns additional 'struct stat' structures to enable
> > >advanced caching of attributes. This is useful for file servers, not so
> > >much for QEMU. Nevertheless, the API has changed and needs to be
> > >adopted.
> > >
> > 
> > Oh, one other comment.
> > 
> > >+++ b/block/gluster.c
> > >@@ -20,6 +20,10 @@
> > >  #include "qemu/option.h"
> > >  #include "qemu/cutils.h"
> > >+#ifdef CONFIG_GLUSTERFS_LEGACY_FTRUNCATE
> > >+# define glfs_ftruncate(fd, offset, _u1, _u2) glfs_ftruncate(fd, offset)
> > >+#endif
> > 
> > Someday, when we can assume new enough gluster everywhere, we can drop this
> > hunk...
> > 
> 
> Part of me wishes that libgfapi had just created a new function
> 'glfs_ftruncate2', so that existing users don't need to handle the api
> change.  But I guess in the grand scheme, not a huge deal either way.

Gluster uses versioned symbols, so older binaries will keep working with
new libraries. It is (hopefully) rare that existing symbols get updated.
We try to send patches for these kind of changes to the projects we know
well in advance, reducing the number of surprises.

> > >+++ b/configure
> > 
> > >+	/* new glfs_ftruncate() passes two additional args */
> > >+	return glfs_ftruncate(NULL, 0 /*, NULL, NULL */);
> > >+}
> > >+EOF
> > >+    if compile_prog "$glusterfs_cflags" "$glusterfs_libs" ; then
> > >+      glusterfs_legacy_ftruncate="yes"
> > >+    fi
> > 
> > ...but it will be easier to remember to do so if this comment in configure
> > calls out the upstream gluster version that no longer requires the legacy
> > workaround, as our hint for when...
> > 
> 
> I can go ahead and add that to the comment in my branch after applying, if
> Niels can let me know what that version is/will be (if known).

The new glfs_ftruncate() will be part of glusterfs-5 (planned for
October). We're changing the numbering scheme, it was expected to come
in glusterfs-4.2, but that is a version that never will be released.

Thanks for correcting the last bits of the patch!
Niels


> > >    else
> > >      if test "$glusterfs" = "yes" ; then
> > >        feature_not_found "GlusterFS backend support" \
> > >@@ -6644,6 +6658,10 @@ if test "$glusterfs_zerofill" = "yes" ; then
> > >    echo "CONFIG_GLUSTERFS_ZEROFILL=y" >> $config_host_mak
> > >  fi
> > >+if test "$glusterfs_legacy_ftruncate" = "yes" ; then
> > >+  echo "CONFIG_GLUSTERFS_LEGACY_FTRUNCATE=y" >> $config_host_mak
> > 
> > ...this #define is no longer necessary.
> > 
> > -- 
> > Eric Blake, Principal Software Engineer
> > Red Hat, Inc.           +1-919-301-3266
> > Virtualization:  qemu.org | libvirt.org