[libvirt] [PATCH] driver: conditionalize use of dlopen functions & use mingw-dlfcn

Daniel P. Berrange posted 1 patch 6 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20170802102606.10827-1-berrange@redhat.com
configure.ac          |  2 +-
mingw-libvirt.spec.in |  2 ++
src/driver.c          | 18 ++++++++++++++++--
3 files changed, 19 insertions(+), 3 deletions(-)
[libvirt] [PATCH] driver: conditionalize use of dlopen functions & use mingw-dlfcn
Posted by Daniel P. Berrange 6 years, 8 months ago
Not every platform is guaranteed to have dlopen/dlsym, so we should
conditionalize its use. Suprisingly it is actually present for Win32
via the mingw-dlfcn add on, but we should still conditionalize it.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 configure.ac          |  2 +-
 mingw-libvirt.spec.in |  2 ++
 src/driver.c          | 18 ++++++++++++++++--
 3 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index b12b7fae1..2b3375138 100644
--- a/configure.ac
+++ b/configure.ac
@@ -328,7 +328,7 @@ dnl Availability of various common headers (non-fatal if missing).
 AC_CHECK_HEADERS([pwd.h regex.h sys/un.h \
   sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h \
   sys/un.h sys/syscall.h sys/sysctl.h netinet/tcp.h ifaddrs.h \
-  libtasn1.h sys/ucred.h sys/mount.h stdarg.h])
+  libtasn1.h sys/ucred.h sys/mount.h stdarg.h dlfcn.h])
 dnl Check whether endian provides handy macros.
 AC_CHECK_DECLS([htole64], [], [], [[#include <endian.h>]])
 AC_CHECK_FUNCS([stat stat64 __xstat __xstat64 lstat lstat64 __lxstat __lxstat64])
diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in
index 553d14022..dcb0837f7 100644
--- a/mingw-libvirt.spec.in
+++ b/mingw-libvirt.spec.in
@@ -54,6 +54,8 @@ BuildRequires:  mingw32-libxml2
 BuildRequires:  mingw64-libxml2
 BuildRequires:  mingw32-portablexdr
 BuildRequires:  mingw64-portablexdr
+BuildRequires:  mingw32-dlfcn
+BuildRequires:  mingw64-dlfcn
 
 BuildRequires:  pkgconfig
 # Need native version for msgfmt
diff --git a/src/driver.c b/src/driver.c
index 2e7dd01df..04dd0a443 100644
--- a/src/driver.c
+++ b/src/driver.c
@@ -34,10 +34,11 @@ VIR_LOG_INIT("driver");
 
 
 /* XXX re-implement this for other OS, or use libtools helper lib ? */
-
-#include <dlfcn.h>
 #define DEFAULT_DRIVER_DIR LIBDIR "/libvirt/connection-driver"
 
+#ifdef HAVE_DLFCN_H
+# include <dlfcn.h>
+
 
 static void *
 virDriverLoadModuleFile(const char *file)
@@ -126,6 +127,19 @@ virDriverLoadModuleFull(const char *path,
     return ret;
 }
 
+#else /* ! HAVE_DLFCN_H */
+int
+virDriverLoadModuleFull(const char *path ATTRIBUTE_UNUSED,
+                        const char *regfunc ATTRIBUTE_UNUSED,
+                        void **handle)
+{
+    VIR_DEBUG("dlopen not available on this platform");
+    if (handle)
+        *handle = NULL;
+    return -1;
+}
+#endif /* ! HAVE_DLFCN_H */
+
 
 int
 virDriverLoadModule(const char *name,
-- 
2.13.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] driver: conditionalize use of dlopen functions & use mingw-dlfcn
Posted by Peter Krempa 6 years, 8 months ago
On Wed, Aug 02, 2017 at 11:26:06 +0100, Daniel Berrange wrote:
> Not every platform is guaranteed to have dlopen/dlsym, so we should
> conditionalize its use. Suprisingly it is actually present for Win32
> via the mingw-dlfcn add on, but we should still conditionalize it.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  configure.ac          |  2 +-
>  mingw-libvirt.spec.in |  2 ++
>  src/driver.c          | 18 ++++++++++++++++--
>  3 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index b12b7fae1..2b3375138 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -328,7 +328,7 @@ dnl Availability of various common headers (non-fatal if missing).
>  AC_CHECK_HEADERS([pwd.h regex.h sys/un.h \
>    sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h \
>    sys/un.h sys/syscall.h sys/sysctl.h netinet/tcp.h ifaddrs.h \
> -  libtasn1.h sys/ucred.h sys/mount.h stdarg.h])
> +  libtasn1.h sys/ucred.h sys/mount.h stdarg.h dlfcn.h])

A similar check is done in m4/virt-dlopen.m4:

AC_CHECK_HEADER([dlfcn.h],, [with_dlfcn=no])

Is it not enough? We already use HAVE_DLFCN_H in few places.

>  dnl Check whether endian provides handy macros.
>  AC_CHECK_DECLS([htole64], [], [], [[#include <endian.h>]])
>  AC_CHECK_FUNCS([stat stat64 __xstat __xstat64 lstat lstat64 __lxstat __lxstat64])
> diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in
> index 553d14022..dcb0837f7 100644
> --- a/mingw-libvirt.spec.in
> +++ b/mingw-libvirt.spec.in
> @@ -54,6 +54,8 @@ BuildRequires:  mingw32-libxml2
>  BuildRequires:  mingw64-libxml2
>  BuildRequires:  mingw32-portablexdr
>  BuildRequires:  mingw64-portablexdr
> +BuildRequires:  mingw32-dlfcn
> +BuildRequires:  mingw64-dlfcn
>  
>  BuildRequires:  pkgconfig
>  # Need native version for msgfmt
> diff --git a/src/driver.c b/src/driver.c
> index 2e7dd01df..04dd0a443 100644
> --- a/src/driver.c
> +++ b/src/driver.c
> @@ -34,10 +34,11 @@ VIR_LOG_INIT("driver");
>  
>  
>  /* XXX re-implement this for other OS, or use libtools helper lib ? */
> -
> -#include <dlfcn.h>
>  #define DEFAULT_DRIVER_DIR LIBDIR "/libvirt/connection-driver"
>  
> +#ifdef HAVE_DLFCN_H
> +# include <dlfcn.h>
> +
>  
>  static void *
>  virDriverLoadModuleFile(const char *file)
> @@ -126,6 +127,19 @@ virDriverLoadModuleFull(const char *path,
>      return ret;
>  }
>  
> +#else /* ! HAVE_DLFCN_H */
> +int
> +virDriverLoadModuleFull(const char *path ATTRIBUTE_UNUSED,
> +                        const char *regfunc ATTRIBUTE_UNUSED,
> +                        void **handle)
> +{
> +    VIR_DEBUG("dlopen not available on this platform");
> +    if (handle)
> +        *handle = NULL;
> +    return -1;
> +}
> +#endif /* ! HAVE_DLFCN_H */

ACK
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] driver: conditionalize use of dlopen functions & use mingw-dlfcn
Posted by Daniel P. Berrange 6 years, 8 months ago
On Wed, Aug 02, 2017 at 01:31:03PM +0200, Peter Krempa wrote:
> On Wed, Aug 02, 2017 at 11:26:06 +0100, Daniel Berrange wrote:
> > Not every platform is guaranteed to have dlopen/dlsym, so we should
> > conditionalize its use. Suprisingly it is actually present for Win32
> > via the mingw-dlfcn add on, but we should still conditionalize it.
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> >  configure.ac          |  2 +-
> >  mingw-libvirt.spec.in |  2 ++
> >  src/driver.c          | 18 ++++++++++++++++--
> >  3 files changed, 19 insertions(+), 3 deletions(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index b12b7fae1..2b3375138 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -328,7 +328,7 @@ dnl Availability of various common headers (non-fatal if missing).
> >  AC_CHECK_HEADERS([pwd.h regex.h sys/un.h \
> >    sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h \
> >    sys/un.h sys/syscall.h sys/sysctl.h netinet/tcp.h ifaddrs.h \
> > -  libtasn1.h sys/ucred.h sys/mount.h stdarg.h])
> > +  libtasn1.h sys/ucred.h sys/mount.h stdarg.h dlfcn.h])
> 
> A similar check is done in m4/virt-dlopen.m4:
> 
> AC_CHECK_HEADER([dlfcn.h],, [with_dlfcn=no])
> 
> Is it not enough? We already use HAVE_DLFCN_H in few places.

Oh, I missed that. I'll drop this chunk

> 
> >  dnl Check whether endian provides handy macros.
> >  AC_CHECK_DECLS([htole64], [], [], [[#include <endian.h>]])
> >  AC_CHECK_FUNCS([stat stat64 __xstat __xstat64 lstat lstat64 __lxstat __lxstat64])
> > diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in
> > index 553d14022..dcb0837f7 100644
> > --- a/mingw-libvirt.spec.in
> > +++ b/mingw-libvirt.spec.in
> > @@ -54,6 +54,8 @@ BuildRequires:  mingw32-libxml2
> >  BuildRequires:  mingw64-libxml2
> >  BuildRequires:  mingw32-portablexdr
> >  BuildRequires:  mingw64-portablexdr
> > +BuildRequires:  mingw32-dlfcn
> > +BuildRequires:  mingw64-dlfcn
> >  
> >  BuildRequires:  pkgconfig
> >  # Need native version for msgfmt
> > diff --git a/src/driver.c b/src/driver.c
> > index 2e7dd01df..04dd0a443 100644
> > --- a/src/driver.c
> > +++ b/src/driver.c
> > @@ -34,10 +34,11 @@ VIR_LOG_INIT("driver");
> >  
> >  
> >  /* XXX re-implement this for other OS, or use libtools helper lib ? */
> > -
> > -#include <dlfcn.h>
> >  #define DEFAULT_DRIVER_DIR LIBDIR "/libvirt/connection-driver"
> >  
> > +#ifdef HAVE_DLFCN_H
> > +# include <dlfcn.h>
> > +
> >  
> >  static void *
> >  virDriverLoadModuleFile(const char *file)
> > @@ -126,6 +127,19 @@ virDriverLoadModuleFull(const char *path,
> >      return ret;
> >  }
> >  
> > +#else /* ! HAVE_DLFCN_H */
> > +int
> > +virDriverLoadModuleFull(const char *path ATTRIBUTE_UNUSED,
> > +                        const char *regfunc ATTRIBUTE_UNUSED,
> > +                        void **handle)
> > +{
> > +    VIR_DEBUG("dlopen not available on this platform");
> > +    if (handle)
> > +        *handle = NULL;
> > +    return -1;
> > +}
> > +#endif /* ! HAVE_DLFCN_H */
> 
> ACK



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list