[dm-devel] [PATCH v2 03/48] libmultipath: add optional wakeup functionality to lock.c

[Martin Wilck]

On Mon, 2021-11-29 at 13:27 -0600, Benjamin Marzinski wrote:
> On Wed, Nov 24, 2021 at 10:20:53PM +0100, Martin Wilck wrote:
> > On Wed, 2021-11-24 at 14:41 -0600, Benjamin Marzinski wrote:
> > > On Thu, Nov 18, 2021 at 11:57:55PM +0100, mwilck at suse.com wrote:
> > > > From: Martin Wilck <mwilck at suse.com>
> > > > 
> > > > Have struct mutex_lock take an optional wakeup function.
> > > > unlock() is renamed to __unlock() in order to prevent it from
> > > > being called by mistake.
> > > > 
> > > > This changes offsets in "struct vectors", requiring a major
> > > > libmultipath version bump. While the strucure is already
> > > > changed,
> > > > in order to avoid this in the future, move the lock to the end
> > > > of "struct vectors".
> > > > 
> > > > Signed-off-by: Martin Wilck <mwilck at suse.com>
> > > 
> > > In libmultipath.version, I would have said that set_wakeup_fn was
> > > added
> > > in 10.0.0, instead of 9.2.0, which is a version that never
> > > actually
> > > existed, but I don't think that's going to cause any problems so,
> > > 
> > > Reviewed-by: Benjamin Marzinski <bmarzins at redhat.com>
> > 
> > We should consider whether we want bump the version numbers only
> > when
> > we submit to Christophe, or maybe once per major patch series that
> > touches the ABI. This is something that I could 
> 
> I suppose that we could just bump the version the first time after a
> push to Christophe that an ABI change is made, and then just make all
> the symbol changes under that version until Christophe makes another
> release. If the first change only caused a minor version bump, and a
> later change needed a major bump we could have two version bumps in a
> release, but I don't think we're getting any benefit from making
> more.
> But once per patchset works fine for me too.

I agree. Also, I realize that we've bumped the library version too
often in the past. If we add a function, we don't need to bump. Because
a program that needs the added function will require e.g.
foo at LIBMULTIPATH_10.0.0, and this will fail for a library that doesn't
export foo (which is what we want). Likewise for function deletion - a
program that calls the deleted function will fail to link with the
updated library. OTOH, programs that use this version of the ABI
*without* using the functions which are added or removed are unaffected
by the addition / removal.

The only case in which the ABI version must be bumped is when we have
changed functions or data structures.

Furthermore, I believe now that the habit (which I introduced) to list
added functions at the end of the .version files, with comments
indicating when they were added, is useless. We should rather keep the
.version file ordered alphabetically.

Regards
Martin