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

[Niels de Vos]

On Tue, Jul 31, 2018 at 03:51:22PM -0400, Jeff Cody wrote:
> On Tue, Jul 31, 2018 at 11:18:02AM +0200, Niels de Vos wrote:
> > On Mon, Jul 30, 2018 at 03:27:29PM -0400, Jeff Cody wrote:
> > > On Mon, Jul 30, 2018 at 10:07:27AM -0500, Eric Blake wrote:
> > > > On 07/28/2018 02:50 AM, Niels de Vos wrote:
> > > > >>
> > > > >>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.
> > > > 
> > > > >>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.
> > > > >
> > > > 
> > > > Wait - so you're saying gluster has not yet released the incompatible
> > > > change? Now would be the right time to get rid of the API breakage, 
> > > > before
> > > > you bake it in, rather than relying solely on the versioned symbols to 
> > > > avoid
> > > > an ABI breakage but forcing all clients to compensate to the API 
> > > > breakage.
> > > > 
> > > 
> > > If this is not yet in a released version of Gluster, I'm not real eager to
> > > pollute the QEMU driver codebase with #ifdef's, especially if there is a
> > > possibility the API change may not actually materialize.
> > > 
> > > Is there any reason that this change is being approached in a way that
> > > breaks API usage, and is it too late in the Gluster development pipeline 
> > > to
> > > change that?
> > 
> > There recently have been a few changes like this in libgfapi. These have
> > been introduced to improve performance in common use-cases where an
> > updated 'struct stat' is needed after an operation. Some functions have
> > been adapted in previous releases, glfs_ftruncate() landed too late for
> > that. I hope we'll get a complete coherent API with glusterfs-5 again.
> > 
> 
> Am I correct in assuming the API changes that happened in previous libgfapi
> release are for APIs that QEMU does not use (i.e. no action needed from
> us?)

Yes, that is correct.

> > For QEMU this means additional changes will come related to
> > glfs_fallocate(), glfs_zerofill() and probably more. The current
> > glusterfs-4.1 version will be maintained for one year, after which the
> > detection/#ifdef can be removed as the than maintained versions should
> > all have the updated API. I'm sorry for the inconvenience that this
> > causes.
> > 
> 
> Understood.  I don't want to make a huge deal out of it.  I guess my
> recommendation/request is that API enhancements happen in a way that don't
> break existing APIs (e.g. use parallel APIs), because QEMU version usage and
> supported glusterfs usage may not always follow supported versions in the
> wild.  I know versioned symbols helps with this, but it still complicates
> the code (and couples QEMU's code to gluster support windows).

Once the last Gluster version that has the old API goes out of
maintenance, we will remove the legacy functions. At that point, we can
also provide a new patch to QEMU (and other projcets) that removes the
compatibility #ifdef's.

> > If you prefer, I can wait with sending patches for QEMU with future
> > Gluster releases until additional changes have landed in libgfapi.
> >
> 
> I think generally, we don't want to start introducing #ifdef's and new APi
> usage for unreleased versions of libgfapi if possible (as unreleased APIs,
> it could technically change, and then QEMU releases would have 'dead' API
> support in it).
> 
> If glusterfs-5 releases in October, that may line up with 3.1 or 3.2.

Ok, I'll keep these kind of patches in my tree as work-in-progress and
when the glusterfs-5 gets tagged for alpha/beta send it again.

Even if not optimal, would you accept this as a way going forward?

Thanks,
Niels