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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 31.08.2024 12:34, Andrei Stan wrote:
> Ping

This is pinging what exactly? From ...

> On Fri, Jul 5, 2024 at 7:05 PM Andrei Stan <andreistan2003@xxxxxxxxx> 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?

... Jürgen's question it imo follows that ...

>>>> ---
>>>>   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?
>>
>> 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.

... this answer is insufficient, from two angles:
- You do in no way answer the question itself, as such an answer would clearly
  need to say something about the commit Jürgen pointed you at.
- It needs to be in the patch description, not just in a reply on list. IOW a
  re-submission is needed anyway.

Jan