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

[Jeff Cody]

On Sat, Jul 28, 2018 at 09:50:05AM +0200, Niels de Vos wrote:
> 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 <address@hidden>
> > > >
> > > >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

So that there is no confusion or miscommunication: I'm not going to pull
this patch in through my tree for 3.0 (or 3.1 yet), since the new API isn't
part of a released version of gluster yet.  (And hopefully we won't ever
need it, if the gluster changes can happen without breaking the existing
API)

-Jeff