[PATCH v2 0/2] kexec-tools: Tweak run-time handling of libxenctrl.so

[PATCH v2 0/2] kexec-tools: Tweak run-time handling of libxenctrl.so

[Eric DeVolder]

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.

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
---
v1: 23jan2018
 - Implemented feedback from Daniel Kiper

v2: 23jan2018
 - Implemented feedback from Daniel Kiper
 - Broke patch into two

Eric DeVolder (2):
  kexec-tools: Call dlclose() from within __xc_interface_close()
  kexec-tools: Make xc_dlhandle static

 kexec/kexec-xen.c | 10 +++++++++-
 kexec/kexec-xen.h |  9 ++++-----
 2 files changed, 13 insertions(+), 6 deletions(-)

-- 
2.7.4


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v2 1/2] kexec-tools: Call dlclose() from within __xc_interface_close()

[Eric DeVolder]

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.

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

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
---
v1: 23jan2018
 - Implemented feedback from Daniel Kiper

v2: 23jan2018
 - Implemented feedback from Daniel Kiper
 - Broke patch into two
---
 kexec/kexec-xen.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kexec/kexec-xen.c b/kexec/kexec-xen.c
index d42a45a..75f8758 100644
--- a/kexec/kexec-xen.c
+++ b/kexec/kexec-xen.c
@@ -47,6 +47,7 @@ int __xc_interface_close(xc_interface *xch)
 
 		func_t func = (func_t)dlsym(xc_dlhandle, "xc_interface_close");
 		rc = func(xch);
+		dlclose(xc_dlhandle);
 		xc_dlhandle = NULL;
 	}
 
-- 
2.7.4


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v2 2/2] kexec-tools: Make xc_dlhandle static

[Eric DeVolder]

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.

This patch implements Daniel's suggestion to make the
xc_dlhandle variable static.

Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
---
v1: 23jan2018
 - Implemented feedback from Daniel Kiper

v2: 23jan2018
 - Implemented feedback from Daniel Kiper
 - Broke patch into two
---
 kexec/kexec-xen.c | 9 ++++++++-
 kexec/kexec-xen.h | 9 ++++-----
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/kexec/kexec-xen.c b/kexec/kexec-xen.c
index 75f8758..960fa16 100644
--- a/kexec/kexec-xen.c
+++ b/kexec/kexec-xen.c
@@ -15,8 +15,15 @@
 #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)
+{
+	return dlsym(xc_dlhandle, symbol);
+}
+
 xc_interface *__xc_interface_open(xentoollog_logger *logger,
 				  xentoollog_logger *dombuild_logger,
 				  unsigned open_flags)
diff --git a/kexec/kexec-xen.h b/kexec/kexec-xen.h
index ffb8743..8955334 100644
--- a/kexec/kexec-xen.h
+++ b/kexec/kexec-xen.h
@@ -6,9 +6,8 @@
 
 #ifdef CONFIG_LIBXENCTRL_DL
 #include <dlfcn.h>
-
-/* The handle from dlopen(), needed by dlsym(), dlclose() */
-extern void *xc_dlhandle;
+/* Lookup symbols in libxenctrl.so */
+void *__xc_dlsym(const char *symbol);
 
 /* Wrappers around xc_interface_open/close() to insert dlopen/dlclose() */
 xc_interface *__xc_interface_open(xentoollog_logger *logger,
@@ -21,13 +20,13 @@ int __xc_interface_close(xc_interface *xch);
 ( \
 	{ dtype value; \
 	typedef dtype (*func_t)(xc_interface *, ...); \
-	func_t func = dlsym(xc_dlhandle, #name); \
+	func_t func = __xc_dlsym(#name); \
 	value = func(args); \
 	value; } \
 )
 #define __xc_data(dtype, name) \
 ( \
-	{ dtype *value = (dtype *)dlsym(xc_dlhandle, #name); value; } \
+	{ dtype *value = (dtype *)__xc_dlsym(#name); value; } \
 )
 
 /* The wrappers around utilized xenctrl.h functions */
-- 
2.7.4


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2 1/2] kexec-tools: Call dlclose() from within __xc_interface_close()

[Daniel Kiper]

On Tue, Jan 23, 2018 at 01:12:50PM -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.
>
> 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).
>
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2 2/2] kexec-tools: Make xc_dlhandle static

[Daniel Kiper]

On Tue, Jan 23, 2018 at 01:12:51PM -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.
>
> This patch implements Daniel's suggestion to make the
> xc_dlhandle variable static.
>
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>

Just two nitpicks... Otherwise you can add

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

> ---
> v1: 23jan2018
>  - Implemented feedback from Daniel Kiper
>
> v2: 23jan2018
>  - Implemented feedback from Daniel Kiper
>  - Broke patch into two
> ---
>  kexec/kexec-xen.c | 9 ++++++++-
>  kexec/kexec-xen.h | 9 ++++-----
>  2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/kexec/kexec-xen.c b/kexec/kexec-xen.c
> index 75f8758..960fa16 100644
> --- a/kexec/kexec-xen.c
> +++ b/kexec/kexec-xen.c
> @@ -15,8 +15,15 @@
>  #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)
> +{
> +	return dlsym(xc_dlhandle, symbol);
> +}
> +
>  xc_interface *__xc_interface_open(xentoollog_logger *logger,
>  				  xentoollog_logger *dombuild_logger,
>  				  unsigned open_flags)
> diff --git a/kexec/kexec-xen.h b/kexec/kexec-xen.h
> index ffb8743..8955334 100644
> --- a/kexec/kexec-xen.h
> +++ b/kexec/kexec-xen.h
> @@ -6,9 +6,8 @@
>
>  #ifdef CONFIG_LIBXENCTRL_DL
>  #include <dlfcn.h>

I think that you can drop this include too.

> -
> -/* The handle from dlopen(), needed by dlsym(), dlclose() */
> -extern void *xc_dlhandle;
> +/* Lookup symbols in libxenctrl.so */
> +void *__xc_dlsym(const char *symbol);

extern void *__xc_dlsym(const char *symbol);?

But then, IMO, you should put extern for __xc_interface_open()
and __xc_interface_close() below too. You can do it in separate
patch with Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2 1/2] kexec-tools: Call dlclose() from within __xc_interface_close()

[Simon Horman]

On Tue, Jan 23, 2018 at 09:34:08PM +0100, Daniel Kiper wrote:
> On Tue, Jan 23, 2018 at 01:12:50PM -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.
> >
> > 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).
> >
> > Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> 
> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Thanks, applied.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2 2/2] kexec-tools: Make xc_dlhandle static

[Simon Horman]

On Tue, Jan 23, 2018 at 09:59:14PM +0100, Daniel Kiper wrote:
> On Tue, Jan 23, 2018 at 01:12:51PM -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.
> >
> > This patch implements Daniel's suggestion to make the
> > xc_dlhandle variable static.
> >
> > Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> 
> Just two nitpicks... Otherwise you can add
> 
> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Eric, please address Daniel's review and repost this patch as appropriate.
There is no need to repost patch 1/2 as I've applied it.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2 2/2] kexec-tools: Make xc_dlhandle static

[Eric DeVolder]

Daniel, thanks for the review, inline responses below.
eric

On 01/23/2018 02:59 PM, Daniel Kiper wrote:
> On Tue, Jan 23, 2018 at 01:12:51PM -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.
>>
>> This patch implements Daniel's suggestion to make the
>> xc_dlhandle variable static.
>>
>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> 
> Just two nitpicks... Otherwise you can add
> 
> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
> 
>> ---
>> v1: 23jan2018
>>   - Implemented feedback from Daniel Kiper
>>
>> v2: 23jan2018
>>   - Implemented feedback from Daniel Kiper
>>   - Broke patch into two
>> ---
>>   kexec/kexec-xen.c | 9 ++++++++-
>>   kexec/kexec-xen.h | 9 ++++-----
>>   2 files changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/kexec/kexec-xen.c b/kexec/kexec-xen.c
>> index 75f8758..960fa16 100644
>> --- a/kexec/kexec-xen.c
>> +++ b/kexec/kexec-xen.c
>> @@ -15,8 +15,15 @@
>>   #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)
>> +{
>> +	return dlsym(xc_dlhandle, symbol);
>> +}
>> +
>>   xc_interface *__xc_interface_open(xentoollog_logger *logger,
>>   				  xentoollog_logger *dombuild_logger,
>>   				  unsigned open_flags)
>> diff --git a/kexec/kexec-xen.h b/kexec/kexec-xen.h
>> index ffb8743..8955334 100644
>> --- a/kexec/kexec-xen.h
>> +++ b/kexec/kexec-xen.h
>> @@ -6,9 +6,8 @@
>>
>>   #ifdef CONFIG_LIBXENCTRL_DL
>>   #include <dlfcn.h>
> 
> I think that you can drop this include too.

I did check and this is the only place in kexec-tools that includes this 
header file (it has the prototypes for dlopen/close/sym()). Without this 
header file included, compilation fails. Thus I've left it in.

> 
>> -
>> -/* The handle from dlopen(), needed by dlsym(), dlclose() */
>> -extern void *xc_dlhandle;
>> +/* Lookup symbols in libxenctrl.so */
>> +void *__xc_dlsym(const char *symbol);
> 
> extern void *__xc_dlsym(const char *symbol);?

Done for this and the others.

> 
> But then, IMO, you should put extern for __xc_interface_open()
> and __xc_interface_close() below too. You can do it in separate
> patch with Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
> 
> Daniel

Thanks, I've posted v3 of this patch, including your RB.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2 2/2] kexec-tools: Make xc_dlhandle static

[Eric DeVolder]

On 01/24/2018 02:36 AM, Simon Horman wrote:
> On Tue, Jan 23, 2018 at 09:59:14PM +0100, Daniel Kiper wrote:
>> On Tue, Jan 23, 2018 at 01:12:51PM -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.
>>>
>>> This patch implements Daniel's suggestion to make the
>>> xc_dlhandle variable static.
>>>
>>> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
>>
>> Just two nitpicks... Otherwise you can add
>>
>> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
> 
> Eric, please address Daniel's review and repost this patch as appropriate.
> There is no need to repost patch 1/2 as I've applied it.
> 

Simon, I've incorporated Daniel's feedback and posted v3 of this patch.
Regards,
eric

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec