[PATCH v1] kexec-tools: Tweak run-time handling of libxenctrl.so

[Eric DeVolder]

Thanks Daniel, inline responses below.
Eric

On 01/23/2018 12:15 PM, Daniel Kiper wrote:
> Hi Eric,
> 
> On Tue, Jan 23, 2018 at 11:39:29AM -0600, Eric DeVolder wrote:
>> This patch is a follow-on to commit 43d3932e "kexec-tools: Perform
>> run-time linking of libxenctrl.so". This patch addresses feedback
>> from Daniel Kiper concerning the lack of a dlclose() and the desire
>> to make xc_dlhandle static, rather than extern.
>>
>> In the original patch, the call to dlclose() was omitted, in contrast
>> to the description in the commit message. This patch inserts the call.
>> Note that this dynamic linking feature is dependent upon the proper
>> operation of the RTLD_NODELETE flag to dlopen(), which does work as
>> advertised on Linux (otherwise the call to dlclose() should be omitted).
>>
>> This patch also implements Daniel's suggestion to make the
>> xc_dlhandle variable static.
>>
>> This patch has been re-tested per scenarios outlined in the original
>> patch commit message.
> 
> Please split this patch into two separate patches. One for dlclose() change
> and one for static xc_dlhandle change. This will help people who will be
> doing git bisect in the future. In general it is much appreciated to
> provide one logical change per patch.

Done. I've posted a v2 with these as separate patches.

> 
>> Signed-off-by: Eric DeVolder <eric.devolder at oracle.com>
>> ---
>> v1: 23jan2018
>>   - Implemented feedback from Daniel Kiper
>> ---
>>   kexec/kexec-xen.c | 13 ++++++++++++-
>>   kexec/kexec-xen.h |  9 ++++-----
>>   2 files changed, 16 insertions(+), 6 deletions(-)
>>
>> diff --git a/kexec/kexec-xen.c b/kexec/kexec-xen.c
>> index d42a45a..03ea4a8 100644
>> --- a/kexec/kexec-xen.c
>> +++ b/kexec/kexec-xen.c
>> @@ -15,8 +15,18 @@
>>   #include "crashdump.h"
>>
>>   #ifdef CONFIG_LIBXENCTRL_DL
>> -void *xc_dlhandle;
>> +/* The handle from dlopen(), needed by dlsym(), dlclose() */
>> +static void *xc_dlhandle;
>>   xc_hypercall_buffer_t XC__HYPERCALL_BUFFER_NAME(HYPERCALL_BUFFER_NULL);
>> +
>> +void *__xc_dlsym(const char *symbol)
>> +{
>> +	if (xc_dlhandle)
>> +		return dlsym(xc_dlhandle, symbol);
>> +	else
>> +		return NULL;
> 
> I am not sure why you need this "if (xc_dlhandle)...". I think that
> "return dlsym(xc_dlhandle, symbol);" alone is sufficient here.

I tested and dlsym() can gracefully accept NULL as the handle, and 
return NULL. The dlclose() segfaults with a NULL handle. I've made
the change.

> 
> Daniel
>