Re: [dm-devel] multipath-tools: Debian patches

Re: [dm-devel] multipath-tools: Debian patches

[Martin Wilck]

On Mon, 2023-06-05 at 21:59 +0200, Xose Vazquez Perez wrote:
> Hi,
> 

I have no Debian salsa account, so I reply here, trying to reach Chris
via email.

> 
> A complaint about upstream, "Remove library development files and all
> of libdmmp":
> https://salsa.debian.org/linux-blocks-team/multipath-tools/-/commit/8c46661697d757763192d8e011c9ad53358d20b7

If Chris has followed the upstream discussion, he is probably aware
that we do care about the ABI. We don't keep the libmultipath ABI
stable, but track it using ABI versioning. It is true that most of the
libmultipath headers are not used for other projects. Not installing
any headers except the public ones makes sense, actually.

The libmpathpersist API (LIBMPATHPERSIST_2.1.0) that's used by qemu is
supposed to remain stable. We have moved those parts of the ABI that
used to be more volatile into __LIBMPATHPERSIST_INT_1.0.0.

Therefore it makes sense to keep shipping mpath_persist.h and drop the
rest. If that works for Debian, it will probably work for other
distros, too.

libdmpp comes from Red Hat, perhaps Ben knows whether it is still used
by any alive project. It does have a stable API/ABI.

> And maybe these are relevant for upstream ( repo:
> https://salsa.debian.org/linux-blocks-team/multipath-tools/-/tree/master/debian/patches
>  ):
> 
> https://udd.debian.org/patches.cgi?src=multipath-tools&version=0.9.4-3
> 

That's not how we work. We don't pick downstream patches. If
something's wrong with the upstream code, we'll happily discuss patches
from the Debian project, preferably here on dm-devel.

Regards
Martin

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] multipath-tools: Debian patches

[Benjamin Marzinski]

On Tue, Jun 06, 2023 at 05:18:36PM +0200, Martin Wilck wrote:
> On Mon, 2023-06-05 at 21:59 +0200, Xose Vazquez Perez wrote:
> > Hi,
> > 
> 
> I have no Debian salsa account, so I reply here, trying to reach Chris
> via email.
> 
> > 
> > A complaint about upstream, "Remove library development files and all
> > of libdmmp":
> > https://salsa.debian.org/linux-blocks-team/multipath-tools/-/commit/8c46661697d757763192d8e011c9ad53358d20b7
> 
> If Chris has followed the upstream discussion, he is probably aware
> that we do care about the ABI. We don't keep the libmultipath ABI
> stable, but track it using ABI versioning. It is true that most of the
> libmultipath headers are not used for other projects. Not installing
> any headers except the public ones makes sense, actually.
> 
> The libmpathpersist API (LIBMPATHPERSIST_2.1.0) that's used by qemu is
> supposed to remain stable. We have moved those parts of the ABI that
> used to be more volatile into __LIBMPATHPERSIST_INT_1.0.0.
> 
> Therefore it makes sense to keep shipping mpath_persist.h and drop the
> rest. If that works for Debian, it will probably work for other
> distros, too.
> 
> libdmpp comes from Red Hat, perhaps Ben knows whether it is still used
> by any alive project. It does have a stable API/ABI.
> 

I don't know of any users, but I don't feel confident in saying that
there aren't any, and I agree that it's ABI is stable.

libmpathvalid has a stable API/ABI as well.

So does libmpathcmd. That one doesn't even have any connection to
libmultipath in the library itself. It's just sockets.

-Ben

> > And maybe these are relevant for upstream ( repo:
> > https://salsa.debian.org/linux-blocks-team/multipath-tools/-/tree/master/debian/patches
> >  ):
> > 
> > https://udd.debian.org/patches.cgi?src=multipath-tools&version=0.9.4-3
> > 
> 
> That's not how we work. We don't pick downstream patches. If
> something's wrong with the upstream code, we'll happily discuss patches
> from the Debian project, preferably here on dm-devel.
> 
> Regards
> Martin
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] multipath-tools: Debian patches

[Chris Hofstaedtler]

Hi,

* Martin Wilck <mwilck@suse.com> [230606 17:18]:
> On Mon, 2023-06-05 at 21:59 +0200, Xose Vazquez Perez wrote:
> > Hi,

> I have no Debian salsa account, so I reply here, trying to reach Chris
> via email.

(not sure where this mail thread started, I don't see the first mail
in the dm-devel archives)

> > A complaint about upstream, "Remove library development files and all
> > of libdmmp":
> > https://salsa.debian.org/linux-blocks-team/multipath-tools/-/commit/8c46661697d757763192d8e011c9ad53358d20b7

FTR, I don't consider this a "complaint".

This commit mostly exists to rectify a few Debian-specific, historic
issues:

1) we ship(ped) libraries in the daemon package. This would be okay,
if the libraries are private libs, but given libmpathpersist exists,
they are clearly not private.

2) we install(ed) various files into old non-/usr paths. These were
mostly the pkg-config, headers and .so files. For Debian reasons, we
cannot "just move them" (yet) into /usr, however at the same time
there existence is a (Debian-specific) problem in the future.

Given there are currently no users in Debian for any of these, it
was easiest to remove all the development files.

If and when other packages in Debian want to use the libs, the
packaging will have to become a lot more complicated.

> If Chris has followed the upstream discussion, he is probably aware
> that we do care about the ABI. We don't keep the libmultipath ABI
> stable, but track it using ABI versioning.

I was only vaguely aware of the changes in the <lib>.version files,
and that all .so files are ".so.0".

From a quick glance it looks like libmultipath.so.0 from 0.9.4
exports a lot of symbols versioned @LIBMULTIPATH_17.0.0, but f.e.
none versioned @LIBMULTIPATH_15.0.0, but the soname didn't change.

So - I'm not sure if, for a Debian library package, it is okay to do
essentially drop symbols without a new soname.

> It is true that most of the
> libmultipath headers are not used for other projects. Not installing
> any headers except the public ones makes sense, actually.
> 
> The libmpathpersist API (LIBMPATHPERSIST_2.1.0) that's used by qemu is
> supposed to remain stable. We have moved those parts of the ABI that
> used to be more volatile into __LIBMPATHPERSIST_INT_1.0.0.
> 
> Therefore it makes sense to keep shipping mpath_persist.h and drop the
> rest. If that works for Debian, it will probably work for other
> distros, too.

I haven't tried building anything against libmpathpersist, but
wouldn't people also need libmultipath.so, and thus transitively
link in libmultipath.so.0, possibly using its symbols?

> libdmpp comes from Red Hat, perhaps Ben knows whether it is still used
> by any alive project. It does have a stable API/ABI.

I couldn't find any users in Debian, so it didn't seem useful to
keep shipping it.

> > And maybe these are relevant for upstream ( repo:
> > https://salsa.debian.org/linux-blocks-team/multipath-tools/-/tree/master/debian/patches
> >  ):
> > 
> > https://udd.debian.org/patches.cgi?src=multipath-tools&version=0.9.4-3
> > 
> 
> That's not how we work. We don't pick downstream patches. If
> something's wrong with the upstream code, we'll happily discuss patches
> from the Debian project, preferably here on dm-devel.

I think most of these patches are not useful for upstream. Do you
care about our historic install paths?

Chris


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] multipath-tools: Debian patches

[Martin Wilck]

On Tue, 2023-06-06 at 18:48 +0200, Chris Hofstaedtler wrote:
> Hi,
> 
> * Martin Wilck <mwilck@suse.com> [230606 17:18]:
> > On Mon, 2023-06-05 at 21:59 +0200, Xose Vazquez Perez wrote:
> > > Hi,
> 
> > I have no Debian salsa account, so I reply here, trying to reach
> > Chris
> > via email.
> 
> (not sure where this mail thread started, I don't see the first mail
> in the dm-devel archives)

It started off-list, but I thought this should rather be a public
discussion. I hope that's ok for everybody.

> 
> > > A complaint about upstream, "Remove library development files and
> > > all
> > > of libdmmp":
> > > https://salsa.debian.org/linux-blocks-team/multipath-tools/-/commit/8c46661697d757763192d8e011c9ad53358d20b7
> 
> FTR, I don't consider this a "complaint".
> 
> This commit mostly exists to rectify a few Debian-specific, historic
> issues:
> 
> 1) we ship(ped) libraries in the daemon package. This would be okay,
> if the libraries are private libs, but given libmpathpersist exists,
> they are clearly not private.
> 
> 2) we install(ed) various files into old non-/usr paths. These were
> mostly the pkg-config, headers and .so files. For Debian reasons, we
> cannot "just move them" (yet) into /usr, however at the same time
> there existence is a (Debian-specific) problem in the future.
> 
> Given there are currently no users in Debian for any of these, it
> was easiest to remove all the development files.
> 
> If and when other packages in Debian want to use the libs, the
> packaging will have to become a lot more complicated.
> 
> > If Chris has followed the upstream discussion, he is probably aware
> > that we do care about the ABI. We don't keep the libmultipath ABI
> > stable, but track it using ABI versioning.
> 
> I was only vaguely aware of the changes in the <lib>.version files,
> and that all .so files are ".so.0".
> From a quick glance it looks like libmultipath.so.0 from 0.9.4
> exports a lot of symbols versioned @LIBMULTIPATH_17.0.0, but f.e.
> none versioned @LIBMULTIPATH_15.0.0, but the soname didn't change.

Maybe Debian enforces additional policies I'm unaware of, but
technically, the soname, or filename based library versioning, doesn't
matter if symbol versioning is used [1]. Symbol versioning can be used
in different ways. For libmultipath, we don't attempt to provide
multiple backward-compatible ABIs. We just want to avoid undefined
behavior which would result from an executable calling a library
routine with a different version of the ABI, and this is what our
library versioning scheme achieves. The idea is similar to the kernel's
symvers mechanism, which avoids loading binary-incompatible modules.

> So - I'm not sure if, for a Debian library package, it is okay to do
> essentially drop symbols without a new soname.

I can't tell for Debian, but other distributions haven't complained so
far. I don't think that multipath-tools is the only project that uses
symbol versioning this way.

> 
> > It is true that most of the
> > libmultipath headers are not used for other projects. Not
> > installing
> > any headers except the public ones makes sense, actually.
> > 
> > The libmpathpersist API (LIBMPATHPERSIST_2.1.0) that's used by qemu
> > is
> > supposed to remain stable. We have moved those parts of the ABI
> > that
> > used to be more volatile into __LIBMPATHPERSIST_INT_1.0.0.
> > 
> > Therefore it makes sense to keep shipping mpath_persist.h and drop
> > the
> > rest. If that works for Debian, it will probably work for other
> > distros, too.
> 
> I haven't tried building anything against libmpathpersist, but
> wouldn't people also need libmultipath.so, and thus transitively
> link in libmultipath.so.0, possibly using its symbols?

The libmultipath data structures that libmpathpersist uses are set up
in libmpathpersist, they are opaque to the calling application.
libmpathpersist will require versioned symbols from libmultipath, and
if these don't match, runtime linking will fail. So if you ship
libmpathpersist with a matching libmultipath, you should be fine.

As Ben noted, the same holds for libmpathvalid and libmpathcmd. We
continuously test our ABI using abigail.

On rpm-based distributions,package management will be able to figure
out this kind of (in)compatibility, albeit in a more coarse-grained way
(e.g. the multipathd package requires
libmpathpersist.so.0(LIBMPATHPERSIST_2.1.0)(64bit), which must match
the provided features of the library package). I suppose something
similar exists in the Debian realm, too.

> > libdmpp comes from Red Hat, perhaps Ben knows whether it is still
> > used
> > by any alive project. It does have a stable API/ABI.
> 
> I couldn't find any users in Debian, so it didn't seem useful to
> keep shipping it.

Sure. It's your decision. Time will tell if anyone complains. IIRC the
purpose of libdmmp was to provide a stable API for 3rd-party
applications to use.

> > > And maybe these are relevant for upstream ( repo:
> > > https://salsa.debian.org/linux-blocks-team/multipath-tools/-/tree/master/debian/patches
> > >  ):
> > > 
> > > https://udd.debian.org/patches.cgi?src=multipath-tools&version=0.9.4-3
> > > 
> > 
> > That's not how we work. We don't pick downstream patches. If
> > something's wrong with the upstream code, we'll happily discuss
> > patches
> > from the Debian project, preferably here on dm-devel.
> 
> I think most of these patches are not useful for upstream. Do you
> care about our historic install paths?
> 

Historic - no. But in general we have the goal that distributions
should be able to customize install paths using make variables, without
having to resort to patching.

Thanks,
Martin

[1] https://akkadia.org/drepper/dsohowto.pdf

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] multipath-tools: Debian patches

[Chris Hofstaedtler]

* Martin Wilck <mwilck@suse.com> [230606 21:40]:
> On Tue, 2023-06-06 at 18:48 +0200, Chris Hofstaedtler wrote:
[..]
> > > If Chris has followed the upstream discussion, he is probably aware
> > > that we do care about the ABI. We don't keep the libmultipath ABI
> > > stable, but track it using ABI versioning.
> > 
> > I was only vaguely aware of the changes in the <lib>.version files,
> > and that all .so files are ".so.0".
> > From a quick glance it looks like libmultipath.so.0 from 0.9.4
> > exports a lot of symbols versioned @LIBMULTIPATH_17.0.0, but f.e.
> > none versioned @LIBMULTIPATH_15.0.0, but the soname didn't change.
> 
> Maybe Debian enforces additional policies I'm unaware of, but
> technically, the soname, or filename based library versioning, doesn't
> matter if symbol versioning is used [1]. Symbol versioning can be used
> in different ways. For libmultipath, we don't attempt to provide
> multiple backward-compatible ABIs. We just want to avoid undefined
> behavior which would result from an executable calling a library
> routine with a different version of the ABI, and this is what our
> library versioning scheme achieves. The idea is similar to the kernel's
> symvers mechanism, which avoids loading binary-incompatible modules.
> 
> > So - I'm not sure if, for a Debian library package, it is okay to do
> > essentially drop symbols without a new soname.
> 
> I can't tell for Debian, but other distributions haven't complained so
> far. I don't think that multipath-tools is the only project that uses
> symbol versioning this way.
> 

[..]

> On rpm-based distributions,package management will be able to figure
> out this kind of (in)compatibility, albeit in a more coarse-grained way
> (e.g. the multipathd package requires
> libmpathpersist.so.0(LIBMPATHPERSIST_2.1.0)(64bit), which must match
> the provided features of the library package). I suppose something
> similar exists in the Debian realm, too.

Do you also rename the package that ships libpathmpersist.so.0 in
this case?

I.e. how do you make this example work:

old libmpathpersist.so.0 provides ABI 1
new libmpathpersist.so.0(?) provides ABI 2

multipathd was linked against the new libmpathpersist.so.0 (ABI 2)
qemu was linked against the old libmpathpersist.so.0 (ABI 1)

I don't see how these can be co-installable if libmpathpersist.so.0
is never renamed?

Maybe other distributions do not need coinstallability, but Debian
needs this, because a) on actual installs we support partial upgrades,
and b) we do not rebuild all reverse-dependencies of a library the
moment a new version of that lib is uploaded.


I also wonder you get from symbol versioning if the old symbols just
disappear. ISTM you could get the same effect - cannot by accident
load the wrong library version - just by bumping the soname, and
ignoring manual symbol versioning?


I asked around a bit and got "this seems very unusual" as feedback.


Thanks,
Chris

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] multipath-tools: Debian patches

[Martin Wilck]

On Wed, 2023-06-07 at 17:12 +0200, Chris Hofstaedtler wrote:
> 
> > On rpm-based distributions,package management will be able to
> > figure
> > out this kind of (in)compatibility, albeit in a more coarse-grained
> > way
> > (e.g. the multipathd package requires
> > libmpathpersist.so.0(LIBMPATHPERSIST_2.1.0)(64bit), which must
> > match
> > the provided features of the library package). I suppose something
> > similar exists in the Debian realm, too.
> 
> Do you also rename the package that ships libpathmpersist.so.0 in
> this case?

No, we don't. The ABI change is solely reflected in the library
package's symbol versions and abstract provides like the one I showed
above. The SONAME remains formally remains "libmpathpersist.so.0".

readelf -W -V -a reports:

Dynamic section at offset 0x8b78 contains 34 entries:
  Tag        Type                         Name/Value
0x000000000000000e (SONAME)             Library soname: [libmpathpersist.so.0]
...
Relocation section '.rela.plt' at offset 0x15e0 contains 60 entries:
    Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
0000000000009df0  0000004600000007 R_X86_64_JUMP_SLOT     0000000000003600 prout_do_scsi_ioctl@@__LIBMPATHPERSIST_INT_1.0.0 + 0
0000000000009df8  0000000100000007 R_X86_64_JUMP_SLOT     0000000000000000 __snprintf_chk@GLIBC_2.3.4 + 0
0000000000009e00  0000000200000007 R_X86_64_JUMP_SLOT     0000000000000000 free@GLIBC_2.2.5 + 0
0000000000009e08  0000000300000007 R_X86_64_JUMP_SLOT     0000000000000000 __errno_location@GLIBC_2.2.5 + 0
0000000000009e10  0000000400000007 R_X86_64_JUMP_SLOT     0000000000000000 strncmp@GLIBC_2.2.5 + 0
0000000000009e18  0000000600000007 R_X86_64_JUMP_SLOT     0000000000000000 libmp_put_multipath_config@LIBMULTIPATH_18.0.0 + 0
0000000000009e20  0000000700000007 R_X86_64_JUMP_SLOT     0000000000000000 puts@GLIBC_2.2.5 + 0
0000000000009e28  0000000800000007 R_X86_64_JUMP_SLOT     0000000000000000 select_reservation_key@LIBMULTIPATH_18.0.0 + 0
0000000000009e30  0000000900000007 R_X86_64_JUMP_SLOT     0000000000000000 dm_map_present@LIBMULTIPATH_18.0.0 + 0
0000000000009e38  0000000a00000007 R_X86_64_JUMP_SLOT     0000000000000000 put_multipath_config@LIBMPATHCOMMON_1.0.0 + 0
0000000000009e40  0000000b00000007 R_X86_64_JUMP_SLOT     0000000000000000 free_multipathvec@LIBMULTIPATH_18.0.0 + 0
0000000000009e48  0000000c00000007 R_X86_64_JUMP_SLOT     0000000000000000 strlcpy@LIBMULTIPATH_16.0.0 + 0
...


> I.e. how do you make this example work:
> 
> old libmpathpersist.so.0 provides ABI 1
> new libmpathpersist.so.0(?) provides ABI 2
> 
> multipathd was linked against the new libmpathpersist.so.0 (ABI 2)
> qemu was linked against the old libmpathpersist.so.0 (ABI 1)
> 
> I don't see how these can be co-installable if libmpathpersist.so.0
> is never renamed?

They aren't co-installable. But the dependencies make sure that
qemu, multipathd, and possibly other consumers will use the ABI version
that libmpathpersist actually exports. On openSUSE Tumbleweed, our
rebuild policy will ensure this. On the more long-lived products, we
usually can't update the ABI in a backward-incompatible way; our QA
tools would notice if we mistakenly tried. Severe security issues might
justify an exception, in which case we'd provide patches to update the
depending packages in a batch.

> Maybe other distributions do not need coinstallability, but Debian
> needs this, because a) on actual installs we support partial
> upgrades,

openSUSE supports that, too. In the case at hand, you would need to
update the "libmpath0", "multipath-tools", and "qemu-tools" packages
simultaneously. (If you'd try to update just one, the resolver would
tell you that the other two need to be updated as well).

> and b) we do not rebuild all reverse-dependencies of a library the
> moment a new version of that lib is uploaded.

Well, in this case the set of reverse-dependencies is rather small.

> I also wonder you get from symbol versioning if the old symbols just
> disappear. ISTM you could get the same effect - cannot by accident
> load the wrong library version - just by bumping the soname, and
> ignoring manual symbol versioning?

In theory, we could. I guess we could also use some heuristics to
transform the LIBMPATHPERSIST... string to a SONAME, making the
libraries co-installable. But so far we didn't need to, and tbh,
given the small number of users of our library, it doesn't seem to make
much sense. Feel free to send patches if you want.

One reason for the symbol versioning rather than SONAME is that it
makes the ABI changes more transparent to ourselves. Using abigail
to compare the latest code against the ABI of the stable version, 
we can figure out which parts of the ABI actually changed, and reflect
this in our libmpathpersist.version file.

Thus even if we were to support SONAME, I believe we'd continue using
the symbol versioning.

> I asked around a bit and got "this seems very unusual" as feedback.

I'm sorry to hear that. But I see nothing in the docs that mandates
either providing multiple ABI versions, or modifying the SONAME just
because symbol versions are being used.

Regards,
Martin

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel