Re: [PATCH] tools/libs/store: add missing USE_PTHREAD to target .o

[Jürgen Groß <jgross@xxxxxxxx>]

On 05.07.24 18:05, Andrei Stan wrote:
> On Fri, Jul 5, 2024 at 6:22 PM Jürgen Groß <jgross@xxxxxxxx> wrote:
> > On 05.07.24 16:59, Andrei Stan wrote:
> > > Currently only shared libraries build with USE_PTHREAD enabled.
> > >
> > > This patch adds the macro also to xs.o.
> > >
> > > Backport: 4.18+ # maybe older
> > > Signed-off-by: Andrei Stan <andreistan2003@xxxxxxxxx>
> > Commit dde3e02b7068 did that explicitly, stating that phtreads are
> > not compatible with static linking.
> >
> > Was that reasoning wrong?
> >
> > Juergen
> > ---
> > > ---
> > >    tools/libs/store/Makefile | 1 +
> > >    1 file changed, 1 insertion(+)
> > >
> > > diff --git a/tools/libs/store/Makefile b/tools/libs/store/Makefile
> > > index 0649cf8307..c6f147c72f 100644
> > > --- a/tools/libs/store/Makefile
> > > +++ b/tools/libs/store/Makefile
> > > @@ -20,6 +20,7 @@ CFLAGS += -include $(XEN_ROOT)/tools/config.h
> > >    CFLAGS += $(CFLAGS_libxentoolcore)
> > >
> > >    xs.opic: CFLAGS += -DUSE_PTHREAD
> > > +xs.o: CFLAGS += -DUSE_PTHREAD
> > >    ifeq ($(CONFIG_Linux),y)
> > >    xs.opic: CFLAGS += -DUSE_DLSYM
> > >    endif
> It was a pretty nasty situation, giving some context, this is for a Go based CLI
> tool and some things in there are multithreaded, but i don't think
> calling in the
> bindings happens anywhere in parallel. Without this patch there would be
> some sort of race conditions (or so I assume from the debugging i've done)
> happening withing the xen/tools code, making it impossible to start a domain.
>
> In this case there were no issues in linking pthreads statically. I was not even
> aware of that being a possible issue. Is it really?
I don't know. Said commit suggests it at least was an issue in the
past. No idea what was wrong with it and whether the situation has
changed since then.

Another question: why are you linking statically? Dynamic linking seems
to be the standard for Go.

> Also I am not too sure how much that code path is actually tested, probably the
> majority of the testing is done with dynamic builds.
I think so, yes. I'm not aware of any tests using static libraries.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature