[libvirt] [PATCH 0/15] Various MinGW/Windows fixes
This series fixes several MinGW compile problem on Windows. Most patches are about exported symbols or missing function bodies. There is also a gnulib issue with MinGW on Windows pending. Eric and I are working on a patch for it. Patches: 01/15 gnulib: Add usleep for MinGW builds 02/15 Make sure uid_t and gid_t are available 03/15 Export conditional state driver symbols only when they are defined 04/15 Remove interfaceRegister from libvirt_private.syms 05/15 Fix export of virConnectAuthPtrDefault for MinGW builds 06/15 Generate libvirt.def from libvirt.syms 07/15 virsh: Handle absence of SA_SIGINFO 08/15 Make sure virtTestCaptureProgramOutput has a body on Windows 09/15 util: Make some conditional symbols unconditional 10/15 bootstrap: Remove rsync from buildreq list 11/15 util: Handle lack of (f)chmod and (f)chown on Windows 12/15 bootstrap: Enable copy-mode for MinGW builds 13/15 util: Replace pciWaitForDeviceCleanup with a stub on Windows 14/15 Add HAVE_PTHREAD_H guard for pthread_sigmask 15/15 util: Add stubs for some functions on Windows Overall diffstat: bootstrap.conf | 8 ++- configure.ac | 17 +++++ docs/apibuild.py | 1 + include/libvirt/libvirt.h.in | 16 +++++- src/.gitignore | 1 + src/Makefile.am | 26 ++++++-- src/libvirt_daemon.syms | 10 +++ src/libvirt_private.syms | 9 --- src/remote/remote_driver.c | 6 ++ src/util/pci.c | 14 ++++ src/util/util.c | 135 ++++++++++++++++++++++++++++++++++++------ src/util/util.h | 4 - tests/testutils.c | 6 ++ tools/virsh.c | 4 + 14 files changed, 218 insertions(+), 39 deletions(-) Matthias
--- bootstrap.conf | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index 157092f..fb862ad 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -56,6 +56,7 @@ strsep sys_stat time_r useless-if-before-free +usleep vasprintf verify vc-list-files -- 1.6.3.3
On Mon, Mar 22, 2010 at 02:25:31AM +0100, Matthias Bolte wrote:
--- bootstrap.conf | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/bootstrap.conf b/bootstrap.conf index 157092f..fb862ad 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -56,6 +56,7 @@ strsep sys_stat time_r useless-if-before-free +usleep vasprintf verify vc-list-files --
ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On 03/22/2010 08:54 AM, Daniel P. Berrange wrote:
On Mon, Mar 22, 2010 at 02:25:31AM +0100, Matthias Bolte wrote:
--- bootstrap.conf | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/bootstrap.conf b/bootstrap.conf index 157092f..fb862ad 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -56,6 +56,7 @@ strsep sys_stat time_r useless-if-before-free +usleep vasprintf verify vc-list-files --
ACK
POSIX 2008 delisted usleep as an obsolete interface; it has some usability issues when compared to nanosleep. While gnulib at least fixes the worst of the portability problems (namely, the fact that usleep is not required to sleep longer than 1 second, even though it can sleep over 4000 seconds on glibc), I'm wondering if we should instead be converting all existing uses of usleep to a more modern interface. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Mon, Mar 22, 2010 at 10:15:35AM -0600, Eric Blake wrote:
On 03/22/2010 08:54 AM, Daniel P. Berrange wrote:
On Mon, Mar 22, 2010 at 02:25:31AM +0100, Matthias Bolte wrote:
--- bootstrap.conf | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/bootstrap.conf b/bootstrap.conf index 157092f..fb862ad 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -56,6 +56,7 @@ strsep sys_stat time_r useless-if-before-free +usleep vasprintf verify vc-list-files --
ACK
POSIX 2008 delisted usleep as an obsolete interface; it has some usability issues when compared to nanosleep. While gnulib at least fixes the worst of the portability problems (namely, the fact that usleep is not required to sleep longer than 1 second, even though it can sleep over 4000 seconds on glibc), I'm wondering if we should instead be converting all existing uses of usleep to a more modern interface.
Sure, we use nansleep in the QEMU driver already too Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--- configure.ac | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/configure.ac b/configure.ac index 654b9a8..1a3c788 100644 --- a/configure.ac +++ b/configure.ac @@ -61,6 +61,7 @@ gl_INIT AM_PROG_CC_STDC AC_C_CONST +AC_TYPE_UID_T dnl Make sure we have an ANSI compiler AM_C_PROTOTYPES -- 1.6.3.3
On Mon, Mar 22, 2010 at 02:25:32AM +0100, Matthias Bolte wrote:
--- configure.ac | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/configure.ac b/configure.ac index 654b9a8..1a3c788 100644 --- a/configure.ac +++ b/configure.ac @@ -61,6 +61,7 @@ gl_INIT
AM_PROG_CC_STDC AC_C_CONST +AC_TYPE_UID_T
dnl Make sure we have an ANSI compiler AM_C_PROTOTYPES --
ACK, didn't know about that macro ! I had previously just changed uid_t to int in various functions for Mingw Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
2010/3/22 Daniel P. Berrange <berrange@redhat.com>:
On Mon, Mar 22, 2010 at 02:25:32AM +0100, Matthias Bolte wrote:
--- configure.ac | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/configure.ac b/configure.ac index 654b9a8..1a3c788 100644 --- a/configure.ac +++ b/configure.ac @@ -61,6 +61,7 @@ gl_INIT
AM_PROG_CC_STDC AC_C_CONST +AC_TYPE_UID_T
dnl Make sure we have an ANSI compiler AM_C_PROTOTYPES --
ACK, didn't know about that macro ! I had previously just changed uid_t to int in various functions for Mingw
Daniel
Thanks, pushed. Matthias
This is necessary for MinGW builds. --- src/Makefile.am | 7 ++++++- src/libvirt_daemon.syms | 10 ++++++++++ src/libvirt_private.syms | 5 ----- 3 files changed, 16 insertions(+), 6 deletions(-) create mode 100644 src/libvirt_daemon.syms diff --git a/src/Makefile.am b/src/Makefile.am index c6371fb..08e204d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -813,13 +813,18 @@ if WITH_MACVTAP USED_SYM_FILES += libvirt_macvtap.syms endif +if WITH_LIBVIRTD +USED_SYM_FILES += libvirt_daemon.syms +endif + EXTRA_DIST += \ libvirt_public.syms \ libvirt_private.syms \ libvirt_driver_modules.syms \ libvirt_bridge.syms \ libvirt_linux.syms \ - libvirt_macvtap.syms + libvirt_macvtap.syms \ + libvirt_daemon.syms BUILT_SOURCES = libvirt.syms diff --git a/src/libvirt_daemon.syms b/src/libvirt_daemon.syms new file mode 100644 index 0000000..eb6e594 --- /dev/null +++ b/src/libvirt_daemon.syms @@ -0,0 +1,10 @@ +# +# These symbols are dependent upon --with-libvirtd via WITH_LIBVIRTD. +# + +# libvirt_internal.h +virStateInitialize; +virStateCleanup; +virStateReload; +virStateActive; +virRegisterStateDriver; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c5ee23d..5d30b90 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -285,10 +285,6 @@ iptablesRemoveUdpInput; # libvirt_internal.h -virStateInitialize; -virStateCleanup; -virStateReload; -virStateActive; virDrvSupportsFeature; virDomainMigratePrepare; virDomainMigratePerform; @@ -299,7 +295,6 @@ virDomainMigratePrepareTunnel; virRegisterDriver; virRegisterInterfaceDriver; virRegisterNetworkDriver; -virRegisterStateDriver; virRegisterStorageDriver; virRegisterDeviceMonitor; virRegisterSecretDriver; -- 1.6.3.3
On Mon, Mar 22, 2010 at 02:25:33AM +0100, Matthias Bolte wrote:
This is necessary for MinGW builds. --- src/Makefile.am | 7 ++++++- src/libvirt_daemon.syms | 10 ++++++++++ src/libvirt_private.syms | 5 ----- 3 files changed, 16 insertions(+), 6 deletions(-) create mode 100644 src/libvirt_daemon.syms
diff --git a/src/Makefile.am b/src/Makefile.am index c6371fb..08e204d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -813,13 +813,18 @@ if WITH_MACVTAP USED_SYM_FILES += libvirt_macvtap.syms endif
+if WITH_LIBVIRTD +USED_SYM_FILES += libvirt_daemon.syms +endif + EXTRA_DIST += \ libvirt_public.syms \ libvirt_private.syms \ libvirt_driver_modules.syms \ libvirt_bridge.syms \ libvirt_linux.syms \ - libvirt_macvtap.syms + libvirt_macvtap.syms \ + libvirt_daemon.syms
BUILT_SOURCES = libvirt.syms
diff --git a/src/libvirt_daemon.syms b/src/libvirt_daemon.syms new file mode 100644 index 0000000..eb6e594 --- /dev/null +++ b/src/libvirt_daemon.syms @@ -0,0 +1,10 @@ +# +# These symbols are dependent upon --with-libvirtd via WITH_LIBVIRTD. +# + +# libvirt_internal.h +virStateInitialize; +virStateCleanup; +virStateReload; +virStateActive; +virRegisterStateDriver; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c5ee23d..5d30b90 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -285,10 +285,6 @@ iptablesRemoveUdpInput;
# libvirt_internal.h -virStateInitialize; -virStateCleanup; -virStateReload; -virStateActive; virDrvSupportsFeature; virDomainMigratePrepare; virDomainMigratePerform; @@ -299,7 +295,6 @@ virDomainMigratePrepareTunnel; virRegisterDriver; virRegisterInterfaceDriver; virRegisterNetworkDriver; -virRegisterStateDriver; virRegisterStorageDriver; virRegisterDeviceMonitor; virRegisterSecretDriver; --
ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
2010/3/22 Daniel P. Berrange <berrange@redhat.com>:
On Mon, Mar 22, 2010 at 02:25:33AM +0100, Matthias Bolte wrote:
This is necessary for MinGW builds. --- src/Makefile.am | 7 ++++++- src/libvirt_daemon.syms | 10 ++++++++++ src/libvirt_private.syms | 5 ----- 3 files changed, 16 insertions(+), 6 deletions(-) create mode 100644 src/libvirt_daemon.syms
diff --git a/src/Makefile.am b/src/Makefile.am index c6371fb..08e204d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -813,13 +813,18 @@ if WITH_MACVTAP USED_SYM_FILES += libvirt_macvtap.syms endif
+if WITH_LIBVIRTD +USED_SYM_FILES += libvirt_daemon.syms +endif + EXTRA_DIST += \ libvirt_public.syms \ libvirt_private.syms \ libvirt_driver_modules.syms \ libvirt_bridge.syms \ libvirt_linux.syms \ - libvirt_macvtap.syms + libvirt_macvtap.syms \ + libvirt_daemon.syms
BUILT_SOURCES = libvirt.syms
diff --git a/src/libvirt_daemon.syms b/src/libvirt_daemon.syms new file mode 100644 index 0000000..eb6e594 --- /dev/null +++ b/src/libvirt_daemon.syms @@ -0,0 +1,10 @@ +# +# These symbols are dependent upon --with-libvirtd via WITH_LIBVIRTD. +# + +# libvirt_internal.h +virStateInitialize; +virStateCleanup; +virStateReload; +virStateActive; +virRegisterStateDriver; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c5ee23d..5d30b90 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -285,10 +285,6 @@ iptablesRemoveUdpInput;
# libvirt_internal.h -virStateInitialize; -virStateCleanup; -virStateReload; -virStateActive; virDrvSupportsFeature; virDomainMigratePrepare; virDomainMigratePerform; @@ -299,7 +295,6 @@ virDomainMigratePrepareTunnel; virRegisterDriver; virRegisterInterfaceDriver; virRegisterNetworkDriver; -virRegisterStateDriver; virRegisterStorageDriver; virRegisterDeviceMonitor; virRegisterSecretDriver; --
ACK
Daniel
Thanks, pushed. Matthias
This symbol is conditional, it would need to be exported conditional to work properly with MinGW. So just remove it, as no other driver register function is listed in the symbols files. --- src/libvirt_private.syms | 4 ---- 1 files changed, 0 insertions(+), 4 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5d30b90..f1ad6db 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -257,10 +257,6 @@ virInterfaceObjUnlock; virInterfaceObjListFree; -# interface_driver.h -interfaceRegister; - - # iptables.h iptablesAddForwardAllowCross; iptablesAddForwardAllowIn; -- 1.6.3.3
On Mon, Mar 22, 2010 at 02:25:34AM +0100, Matthias Bolte wrote:
This symbol is conditional, it would need to be exported conditional to work properly with MinGW. So just remove it, as no other driver register function is listed in the symbols files. --- src/libvirt_private.syms | 4 ---- 1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5d30b90..f1ad6db 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -257,10 +257,6 @@ virInterfaceObjUnlock; virInterfaceObjListFree;
-# interface_driver.h -interfaceRegister; - - # iptables.h iptablesAddForwardAllowCross; iptablesAddForwardAllowIn;
ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
2010/3/22 Daniel P. Berrange <berrange@redhat.com>:
On Mon, Mar 22, 2010 at 02:25:34AM +0100, Matthias Bolte wrote:
This symbol is conditional, it would need to be exported conditional to work properly with MinGW. So just remove it, as no other driver register function is listed in the symbols files. --- src/libvirt_private.syms | 4 ---- 1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5d30b90..f1ad6db 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -257,10 +257,6 @@ virInterfaceObjUnlock; virInterfaceObjListFree;
-# interface_driver.h -interfaceRegister; - - # iptables.h iptablesAddForwardAllowCross; iptablesAddForwardAllowIn;
ACK
Daniel
Thanks, pushed. Matthias
Use the __declspec(dllexport/dllimport) stuff to export the symbol, otherwise accessing virConnectAuthPtrDefault triggers a segfault. --- configure.ac | 11 +++++++++++ docs/apibuild.py | 1 + include/libvirt/libvirt.h.in | 16 +++++++++++++++- src/Makefile.am | 4 +++- 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 1a3c788..bcf1d5a 100644 --- a/configure.ac +++ b/configure.ac @@ -1732,6 +1732,7 @@ CYGWIN_EXTRA_LDFLAGS= CYGWIN_EXTRA_LIBADD= CYGWIN_EXTRA_PYTHON_LIBADD= MINGW_EXTRA_LDFLAGS= +WIN32_EXTRA_CFLAGS= case "$host" in *-*-cygwin*) CYGWIN_EXTRA_LDFLAGS="-no-undefined" @@ -1744,10 +1745,20 @@ case "$host" in MINGW_EXTRA_LDFLAGS="-no-undefined" ;; esac +case "$host" in + *-*-mingw* | *-*-cygwin* | *-*-msvc* ) + # If the host is Windows, and shared libraries are disabled, we + # need to add -DLIBVIRT_STATIC to the CFLAGS for proper linking + if test "x$enable_shared" = "xno"; then + WIN32_EXTRA_CFLAGS="-DLIBVIRT_STATIC" + fi + ;; +esac AC_SUBST([CYGWIN_EXTRA_LDFLAGS]) AC_SUBST([CYGWIN_EXTRA_LIBADD]) AC_SUBST([CYGWIN_EXTRA_PYTHON_LIBADD]) AC_SUBST([MINGW_EXTRA_LDFLAGS]) +AC_SUBST([WIN32_EXTRA_CFLAGS]) dnl Look for windres to build a Windows icon resource. AC_CHECK_TOOL([WINDRES], [windres], [no]) diff --git a/docs/apibuild.py b/docs/apibuild.py index 0ab5db2..2dda4df 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -28,6 +28,7 @@ ignored_words = { "ATTRIBUTE_UNUSED": (0, "macro keyword"), "ATTRIBUTE_SENTINEL": (0, "macro keyword"), "VIR_DEPRECATED": (0, "macro keyword"), + "VIR_EXPORT_VAR": (0, "macro keyword"), "WINAPI": (0, "Windows keyword"), "__declspec": (3, "Windows keyword"), "__stdcall": (0, "Windows keyword"), diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 12b8ea1..aaefa09 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -29,6 +29,20 @@ extern "C" { # endif #endif /* VIR_DEPRECATED */ +#ifdef WIN32 +# ifdef LIBVIRT_STATIC +# define VIR_EXPORT_VAR extern +# else +# ifdef IN_LIBVIRT +# define VIR_EXPORT_VAR __declspec(dllexport) +# else +# define VIR_EXPORT_VAR __declspec(dllimport) extern +# endif +# endif +#else +# define VIR_EXPORT_VAR extern +#endif + /** * virConnect: * @@ -499,7 +513,7 @@ struct _virConnectAuth { typedef struct _virConnectAuth virConnectAuth; typedef virConnectAuth *virConnectAuthPtr; -extern virConnectAuthPtr virConnectAuthPtrDefault; +VIR_EXPORT_VAR virConnectAuthPtr virConnectAuthPtrDefault; /** * VIR_UUID_BUFLEN: diff --git a/src/Makefile.am b/src/Makefile.am index 08e204d..0aa3443 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -20,7 +20,9 @@ INCLUDES = \ -DLOCAL_STATE_DIR=\""$(localstatedir)"\" \ -DGETTEXT_PACKAGE=\"$(PACKAGE)\" \ $(WARN_CFLAGS) \ - $(LOCK_CHECKING_CFLAGS) + $(LOCK_CHECKING_CFLAGS) \ + -DIN_LIBVIRT \ + $(WIN32_EXTRA_CFLAGS) EXTRA_DIST = $(conf_DATA) -- 1.6.3.3
On Mon, Mar 22, 2010 at 02:25:35AM +0100, Matthias Bolte wrote:
Use the __declspec(dllexport/dllimport) stuff to export the symbol, otherwise accessing virConnectAuthPtrDefault triggers a segfault. --- configure.ac | 11 +++++++++++ docs/apibuild.py | 1 + include/libvirt/libvirt.h.in | 16 +++++++++++++++- src/Makefile.am | 4 +++- 4 files changed, 30 insertions(+), 2 deletions(-)
ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
2010/3/22 Daniel P. Berrange <berrange@redhat.com>:
On Mon, Mar 22, 2010 at 02:25:35AM +0100, Matthias Bolte wrote:
Use the __declspec(dllexport/dllimport) stuff to export the symbol, otherwise accessing virConnectAuthPtrDefault triggers a segfault. --- configure.ac | 11 +++++++++++ docs/apibuild.py | 1 + include/libvirt/libvirt.h.in | 16 +++++++++++++++- src/Makefile.am | 4 +++- 4 files changed, 30 insertions(+), 2 deletions(-)
ACK
Daniel
Thanks, pushed. Matthias
The MinGW linker needs the libvirt.def file. --- configure.ac | 5 +++++ src/.gitignore | 1 + src/Makefile.am | 15 +++++++++++---- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/configure.ac b/configure.ac index bcf1d5a..6e568ee 100644 --- a/configure.ac +++ b/configure.ac @@ -1733,6 +1733,7 @@ CYGWIN_EXTRA_LIBADD= CYGWIN_EXTRA_PYTHON_LIBADD= MINGW_EXTRA_LDFLAGS= WIN32_EXTRA_CFLAGS= +LIBVIRT_SYMBOL_FILE=libvirt.syms case "$host" in *-*-cygwin*) CYGWIN_EXTRA_LDFLAGS="-no-undefined" @@ -1752,6 +1753,9 @@ case "$host" in if test "x$enable_shared" = "xno"; then WIN32_EXTRA_CFLAGS="-DLIBVIRT_STATIC" fi + # Also set the symbol file to .def, so src/Makefile generates libvirt.def + # from libvirt.syms and passes libvirt.def instead of libvirt.syms to the linker + LIBVIRT_SYMBOL_FILE=libvirt.def ;; esac AC_SUBST([CYGWIN_EXTRA_LDFLAGS]) @@ -1759,6 +1763,7 @@ AC_SUBST([CYGWIN_EXTRA_LIBADD]) AC_SUBST([CYGWIN_EXTRA_PYTHON_LIBADD]) AC_SUBST([MINGW_EXTRA_LDFLAGS]) AC_SUBST([WIN32_EXTRA_CFLAGS]) +AC_SUBST([LIBVIRT_SYMBOL_FILE]) dnl Look for windres to build a Windows icon resource. AC_CHECK_TOOL([WINDRES], [windres], [no]) diff --git a/src/.gitignore b/src/.gitignore index 26b8689..a5c27a5 100644 --- a/src/.gitignore +++ b/src/.gitignore @@ -12,6 +12,7 @@ Makefile.in *.cov libvirt_parthelper libvirt_lxc +libvirt.def libvirt.syms *.i *.s diff --git a/src/Makefile.am b/src/Makefile.am index 0aa3443..fea1bd3 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -828,7 +828,7 @@ EXTRA_DIST += \ libvirt_macvtap.syms \ libvirt_daemon.syms -BUILT_SOURCES = libvirt.syms +BUILT_SOURCES = libvirt.syms libvirt.def libvirt.syms: libvirt_public.syms $(USED_SYM_FILES) rm -f $@-tmp $@ @@ -844,18 +844,25 @@ libvirt.syms: libvirt_public.syms $(USED_SYM_FILES) chmod a-w $@-tmp mv $@-tmp libvirt.syms +libvirt.def: libvirt.syms + rm -f -- $@-tmp $@ + printf 'EXPORTS\n' > $@-tmp + sed -e '/^$$/d; /#/d; /:/d; /\}/d; /\*/d; /LIBVIRT_/d; s/\(.*\)\;/\1/g' $^ >> $@-tmp + chmod a-w $@-tmp + mv $@-tmp libvirt.def + # Empty source list - it merely links a bunch of convenience libs together libvirt_la_SOURCES = libvirt_la_LIBADD += \ $(CYGWIN_EXTRA_LIBADD) ../gnulib/lib/libgnu.la -libvirt_la_LDFLAGS = $(VERSION_SCRIPT_FLAGS)libvirt.syms \ +libvirt_la_LDFLAGS = $(VERSION_SCRIPT_FLAGS)$(LIBVIRT_SYMBOL_FILE) \ -version-info $(LIBVIRT_VERSION_INFO) \ $(COVERAGE_CFLAGS:-f%=-Wc,-f%) \ $(LIBXML_LIBS) \ $(DRIVER_MODULE_LIBS) \ $(CYGWIN_EXTRA_LDFLAGS) $(MINGW_EXTRA_LDFLAGS) libvirt_la_CFLAGS = $(COVERAGE_CFLAGS) -DIN_LIBVIRT -libvirt_la_DEPENDENCIES = $(libvirt_la_LIBADD) libvirt.syms +libvirt_la_DEPENDENCIES = $(libvirt_la_LIBADD) $(LIBVIRT_SYMBOL_FILE) # Create an automake "convenience library" version of libvirt_la, # just for testing, since the test harness requires access to internal @@ -865,7 +872,7 @@ noinst_LTLIBRARIES += libvirt_test.la # Remove version script from convenience library test_LDFLAGS = \ $$(echo '$(libvirt_la_LDFLAGS)' \ - |sed 's!$(VERSION_SCRIPT_FLAGS)libvirt.syms!!' \ + |sed 's!$(VERSION_SCRIPT_FLAGS)$(LIBVIRT_SYMBOL_FILE)!!' \ |sed 's!-version-info $(LIBVIRT_VERSION_INFO)!!') # Just like the above, but with a slightly different set of public symbols. -- 1.6.3.3
On Mon, Mar 22, 2010 at 02:25:36AM +0100, Matthias Bolte wrote:
The MinGW linker needs the libvirt.def file. --- configure.ac | 5 +++++ src/.gitignore | 1 + src/Makefile.am | 15 +++++++++++---- 3 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am index 0aa3443..fea1bd3 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -828,7 +828,7 @@ EXTRA_DIST += \ libvirt_macvtap.syms \ libvirt_daemon.syms
-BUILT_SOURCES = libvirt.syms +BUILT_SOURCES = libvirt.syms libvirt.def
libvirt.syms: libvirt_public.syms $(USED_SYM_FILES) rm -f $@-tmp $@ @@ -844,18 +844,25 @@ libvirt.syms: libvirt_public.syms $(USED_SYM_FILES) chmod a-w $@-tmp mv $@-tmp libvirt.syms
+libvirt.def: libvirt.syms + rm -f -- $@-tmp $@ + printf 'EXPORTS\n' > $@-tmp + sed -e '/^$$/d; /#/d; /:/d; /\}/d; /\*/d; /LIBVIRT_/d; s/\(.*\)\;/\1/g' $^ >> $@-tmp + chmod a-w $@-tmp + mv $@-tmp libvirt.def
Hmm, this is effectively exporting all our private symbols on Win32 too :-( I thought the GCC/LD toolchain on Mingw already knew about the current libvirt.syms file format ? Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Mon, Mar 22, 2010 at 02:25:36AM +0100, Matthias Bolte wrote:
The MinGW linker needs the libvirt.def file. --- configure.ac | 5 +++++ src/.gitignore | 1 + src/Makefile.am | 15 +++++++++++---- 3 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/configure.ac b/configure.ac index bcf1d5a..6e568ee 100644 --- a/configure.ac +++ b/configure.ac @@ -1733,6 +1733,7 @@ CYGWIN_EXTRA_LIBADD= CYGWIN_EXTRA_PYTHON_LIBADD= MINGW_EXTRA_LDFLAGS= WIN32_EXTRA_CFLAGS= +LIBVIRT_SYMBOL_FILE=libvirt.syms case "$host" in *-*-cygwin*) CYGWIN_EXTRA_LDFLAGS="-no-undefined" @@ -1752,6 +1753,9 @@ case "$host" in if test "x$enable_shared" = "xno"; then WIN32_EXTRA_CFLAGS="-DLIBVIRT_STATIC" fi + # Also set the symbol file to .def, so src/Makefile generates libvirt.def + # from libvirt.syms and passes libvirt.def instead of libvirt.syms to the linker + LIBVIRT_SYMBOL_FILE=libvirt.def ;; esac AC_SUBST([CYGWIN_EXTRA_LDFLAGS]) @@ -1759,6 +1763,7 @@ AC_SUBST([CYGWIN_EXTRA_LIBADD]) AC_SUBST([CYGWIN_EXTRA_PYTHON_LIBADD]) AC_SUBST([MINGW_EXTRA_LDFLAGS]) AC_SUBST([WIN32_EXTRA_CFLAGS]) +AC_SUBST([LIBVIRT_SYMBOL_FILE])
dnl Look for windres to build a Windows icon resource. AC_CHECK_TOOL([WINDRES], [windres], [no]) diff --git a/src/.gitignore b/src/.gitignore index 26b8689..a5c27a5 100644 --- a/src/.gitignore +++ b/src/.gitignore @@ -12,6 +12,7 @@ Makefile.in *.cov libvirt_parthelper libvirt_lxc +libvirt.def libvirt.syms *.i *.s diff --git a/src/Makefile.am b/src/Makefile.am index 0aa3443..fea1bd3 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -828,7 +828,7 @@ EXTRA_DIST += \ libvirt_macvtap.syms \ libvirt_daemon.syms
-BUILT_SOURCES = libvirt.syms +BUILT_SOURCES = libvirt.syms libvirt.def
libvirt.syms: libvirt_public.syms $(USED_SYM_FILES) rm -f $@-tmp $@ @@ -844,18 +844,25 @@ libvirt.syms: libvirt_public.syms $(USED_SYM_FILES) chmod a-w $@-tmp mv $@-tmp libvirt.syms
+libvirt.def: libvirt.syms + rm -f -- $@-tmp $@ + printf 'EXPORTS\n' > $@-tmp + sed -e '/^$$/d; /#/d; /:/d; /\}/d; /\*/d; /LIBVIRT_/d; s/\(.*\)\;/\1/g' $^ >> $@-tmp + chmod a-w $@-tmp + mv $@-tmp libvirt.def + # Empty source list - it merely links a bunch of convenience libs together libvirt_la_SOURCES = libvirt_la_LIBADD += \ $(CYGWIN_EXTRA_LIBADD) ../gnulib/lib/libgnu.la -libvirt_la_LDFLAGS = $(VERSION_SCRIPT_FLAGS)libvirt.syms \ +libvirt_la_LDFLAGS = $(VERSION_SCRIPT_FLAGS)$(LIBVIRT_SYMBOL_FILE) \ -version-info $(LIBVIRT_VERSION_INFO) \ $(COVERAGE_CFLAGS:-f%=-Wc,-f%) \ $(LIBXML_LIBS) \ $(DRIVER_MODULE_LIBS) \ $(CYGWIN_EXTRA_LDFLAGS) $(MINGW_EXTRA_LDFLAGS) libvirt_la_CFLAGS = $(COVERAGE_CFLAGS) -DIN_LIBVIRT -libvirt_la_DEPENDENCIES = $(libvirt_la_LIBADD) libvirt.syms +libvirt_la_DEPENDENCIES = $(libvirt_la_LIBADD) $(LIBVIRT_SYMBOL_FILE)
# Create an automake "convenience library" version of libvirt_la, # just for testing, since the test harness requires access to internal @@ -865,7 +872,7 @@ noinst_LTLIBRARIES += libvirt_test.la # Remove version script from convenience library test_LDFLAGS = \ $$(echo '$(libvirt_la_LDFLAGS)' \ - |sed 's!$(VERSION_SCRIPT_FLAGS)libvirt.syms!!' \ + |sed 's!$(VERSION_SCRIPT_FLAGS)$(LIBVIRT_SYMBOL_FILE)!!' \ |sed 's!-version-info $(LIBVIRT_VERSION_INFO)!!')
# Just like the above, but with a slightly different set of public symbols.
I've re-examined this now to discover why we had this regression. Originally, say in 0.7.5, everything was linking fine on Mingw32 without this .defs file. I figured out that this is because Mingw32 was ignoring our .syms file, and using its default logic of exporting *everything* :-) Then, in commit 190aaa2627a8c6e455088f1e7801708fb5f123b1 Author: Matthias Bolte <matthias.bolte@googlemail.com> Date: Tue Mar 16 23:54:22 2010 +0100 Fix export of virConnectAuthPtrDefault for MinGW builds Use the __declspec(dllexport/dllimport) stuff to export the symbol, otherwise accessing virConnectAuthPtrDefault triggers a segfault. We used declspec() on the virConnectAuthPtrDefault. This turned off the Mingw32 logic that exported everything & thus caused virsh link failures. Adding this .defs file as per your patch re-exports everything. It sucks that we export everything, but it is no worse than the old situation we had on mingw. ACK to this patch Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
2010/4/7 Daniel P. Berrange <berrange@redhat.com>:
On Mon, Mar 22, 2010 at 02:25:36AM +0100, Matthias Bolte wrote:
The MinGW linker needs the libvirt.def file. --- configure.ac | 5 +++++ src/.gitignore | 1 + src/Makefile.am | 15 +++++++++++---- 3 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/configure.ac b/configure.ac index bcf1d5a..6e568ee 100644 --- a/configure.ac +++ b/configure.ac @@ -1733,6 +1733,7 @@ CYGWIN_EXTRA_LIBADD= CYGWIN_EXTRA_PYTHON_LIBADD= MINGW_EXTRA_LDFLAGS= WIN32_EXTRA_CFLAGS= +LIBVIRT_SYMBOL_FILE=libvirt.syms case "$host" in *-*-cygwin*) CYGWIN_EXTRA_LDFLAGS="-no-undefined" @@ -1752,6 +1753,9 @@ case "$host" in if test "x$enable_shared" = "xno"; then WIN32_EXTRA_CFLAGS="-DLIBVIRT_STATIC" fi + # Also set the symbol file to .def, so src/Makefile generates libvirt.def + # from libvirt.syms and passes libvirt.def instead of libvirt.syms to the linker + LIBVIRT_SYMBOL_FILE=libvirt.def ;; esac AC_SUBST([CYGWIN_EXTRA_LDFLAGS]) @@ -1759,6 +1763,7 @@ AC_SUBST([CYGWIN_EXTRA_LIBADD]) AC_SUBST([CYGWIN_EXTRA_PYTHON_LIBADD]) AC_SUBST([MINGW_EXTRA_LDFLAGS]) AC_SUBST([WIN32_EXTRA_CFLAGS]) +AC_SUBST([LIBVIRT_SYMBOL_FILE])
dnl Look for windres to build a Windows icon resource. AC_CHECK_TOOL([WINDRES], [windres], [no]) diff --git a/src/.gitignore b/src/.gitignore index 26b8689..a5c27a5 100644 --- a/src/.gitignore +++ b/src/.gitignore @@ -12,6 +12,7 @@ Makefile.in *.cov libvirt_parthelper libvirt_lxc +libvirt.def libvirt.syms *.i *.s diff --git a/src/Makefile.am b/src/Makefile.am index 0aa3443..fea1bd3 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -828,7 +828,7 @@ EXTRA_DIST += \ libvirt_macvtap.syms \ libvirt_daemon.syms
-BUILT_SOURCES = libvirt.syms +BUILT_SOURCES = libvirt.syms libvirt.def
libvirt.syms: libvirt_public.syms $(USED_SYM_FILES) rm -f $@-tmp $@ @@ -844,18 +844,25 @@ libvirt.syms: libvirt_public.syms $(USED_SYM_FILES) chmod a-w $@-tmp mv $@-tmp libvirt.syms
+libvirt.def: libvirt.syms + rm -f -- $@-tmp $@ + printf 'EXPORTS\n' > $@-tmp + sed -e '/^$$/d; /#/d; /:/d; /\}/d; /\*/d; /LIBVIRT_/d; s/\(.*\)\;/\1/g' $^ >> $@-tmp + chmod a-w $@-tmp + mv $@-tmp libvirt.def + # Empty source list - it merely links a bunch of convenience libs together libvirt_la_SOURCES = libvirt_la_LIBADD += \ $(CYGWIN_EXTRA_LIBADD) ../gnulib/lib/libgnu.la -libvirt_la_LDFLAGS = $(VERSION_SCRIPT_FLAGS)libvirt.syms \ +libvirt_la_LDFLAGS = $(VERSION_SCRIPT_FLAGS)$(LIBVIRT_SYMBOL_FILE) \ -version-info $(LIBVIRT_VERSION_INFO) \ $(COVERAGE_CFLAGS:-f%=-Wc,-f%) \ $(LIBXML_LIBS) \ $(DRIVER_MODULE_LIBS) \ $(CYGWIN_EXTRA_LDFLAGS) $(MINGW_EXTRA_LDFLAGS) libvirt_la_CFLAGS = $(COVERAGE_CFLAGS) -DIN_LIBVIRT -libvirt_la_DEPENDENCIES = $(libvirt_la_LIBADD) libvirt.syms +libvirt_la_DEPENDENCIES = $(libvirt_la_LIBADD) $(LIBVIRT_SYMBOL_FILE)
# Create an automake "convenience library" version of libvirt_la, # just for testing, since the test harness requires access to internal @@ -865,7 +872,7 @@ noinst_LTLIBRARIES += libvirt_test.la # Remove version script from convenience library test_LDFLAGS = \ $$(echo '$(libvirt_la_LDFLAGS)' \ - |sed 's!$(VERSION_SCRIPT_FLAGS)libvirt.syms!!' \ + |sed 's!$(VERSION_SCRIPT_FLAGS)$(LIBVIRT_SYMBOL_FILE)!!' \ |sed 's!-version-info $(LIBVIRT_VERSION_INFO)!!')
# Just like the above, but with a slightly different set of public symbols.
I've re-examined this now to discover why we had this regression.
Originally, say in 0.7.5, everything was linking fine on Mingw32 without this .defs file. I figured out that this is because Mingw32 was ignoring our .syms file, and using its default logic of exporting *everything* :-)
Then, in
commit 190aaa2627a8c6e455088f1e7801708fb5f123b1 Author: Matthias Bolte <matthias.bolte@googlemail.com> Date: Tue Mar 16 23:54:22 2010 +0100
Fix export of virConnectAuthPtrDefault for MinGW builds
Use the __declspec(dllexport/dllimport) stuff to export the symbol, otherwise accessing virConnectAuthPtrDefault triggers a segfault.
We used declspec() on the virConnectAuthPtrDefault. This turned off the Mingw32 logic that exported everything & thus caused virsh link failures.
Adding this .defs file as per your patch re-exports everything.
It sucks that we export everything, but it is no worse than the old situation we had on mingw.
ACK to this patch
Daniel
Okay, I rebased and pushed it. Matthias
MinGW and gnulib don't provide SA_SIGINFO on Windows. --- tools/virsh.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 32895b2..1c932bd 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -411,11 +411,13 @@ static int disconnected = 0; /* we may have been disconnected */ * We get here when a SIGPIPE is being raised, we can't do much in the * handler, just save the fact it was raised */ +#ifdef SA_SIGINFO static void vshCatchDisconnect(int sig, siginfo_t * siginfo, void* context ATTRIBUTE_UNUSED) { if ((sig == SIGPIPE) || (siginfo->si_signo == SIGPIPE)) disconnected++; } +#endif /* * vshSetupSignals: @@ -425,6 +427,7 @@ static void vshCatchDisconnect(int sig, siginfo_t * siginfo, */ static void vshSetupSignals(void) { +#ifdef SA_SIGINFO struct sigaction sig_action; sig_action.sa_sigaction = vshCatchDisconnect; @@ -432,6 +435,7 @@ vshSetupSignals(void) { sigemptyset(&sig_action.sa_mask); sigaction(SIGPIPE, &sig_action, NULL); +#endif } /* -- 1.6.3.3
On Mon, Mar 22, 2010 at 02:25:37AM +0100, Matthias Bolte wrote:
MinGW and gnulib don't provide SA_SIGINFO on Windows. --- tools/virsh.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 32895b2..1c932bd 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -411,11 +411,13 @@ static int disconnected = 0; /* we may have been disconnected */ * We get here when a SIGPIPE is being raised, we can't do much in the * handler, just save the fact it was raised */ +#ifdef SA_SIGINFO static void vshCatchDisconnect(int sig, siginfo_t * siginfo, void* context ATTRIBUTE_UNUSED) { if ((sig == SIGPIPE) || (siginfo->si_signo == SIGPIPE)) disconnected++; } +#endif
/* * vshSetupSignals: @@ -425,6 +427,7 @@ static void vshCatchDisconnect(int sig, siginfo_t * siginfo, */ static void vshSetupSignals(void) { +#ifdef SA_SIGINFO struct sigaction sig_action;
sig_action.sa_sigaction = vshCatchDisconnect; @@ -432,6 +435,7 @@ vshSetupSignals(void) { sigemptyset(&sig_action.sa_mask);
sigaction(SIGPIPE, &sig_action, NULL); +#endif }
/*
ACK, though we could fallback to the old style signal handler for Win32 Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On 03/22/2010 09:00 AM, Daniel P. Berrange wrote:
On Mon, Mar 22, 2010 at 02:25:37AM +0100, Matthias Bolte wrote:
MinGW and gnulib don't provide SA_SIGINFO on Windows.
ACK, though we could fallback to the old style signal handler for Win32
gnulib provides sigaction, and even SIGPIPE emulation, just not SA_SIGINFO. So yes, it would still make sense to fall back on a sigaction() that just avoids using the SA_SIGINFO flag, rather than just skipping out on the signal registration altogether. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Mon, Mar 22, 2010 at 02:25:37AM +0100, Matthias Bolte wrote:
MinGW and gnulib don't provide SA_SIGINFO on Windows. --- tools/virsh.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 32895b2..1c932bd 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -411,11 +411,13 @@ static int disconnected = 0; /* we may have been disconnected */ * We get here when a SIGPIPE is being raised, we can't do much in the * handler, just save the fact it was raised */ +#ifdef SA_SIGINFO static void vshCatchDisconnect(int sig, siginfo_t * siginfo, void* context ATTRIBUTE_UNUSED) { if ((sig == SIGPIPE) || (siginfo->si_signo == SIGPIPE)) disconnected++; } +#endif
/* * vshSetupSignals: @@ -425,6 +427,7 @@ static void vshCatchDisconnect(int sig, siginfo_t * siginfo, */ static void vshSetupSignals(void) { +#ifdef SA_SIGINFO struct sigaction sig_action;
sig_action.sa_sigaction = vshCatchDisconnect; @@ -432,6 +435,7 @@ vshSetupSignals(void) { sigemptyset(&sig_action.sa_mask);
sigaction(SIGPIPE, &sig_action, NULL); +#endif }
/* --
THis patch doesn't appear to have been pushed... Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
2010/4/7 Daniel P. Berrange <berrange@redhat.com>:
On Mon, Mar 22, 2010 at 02:25:37AM +0100, Matthias Bolte wrote:
MinGW and gnulib don't provide SA_SIGINFO on Windows. --- tools/virsh.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 32895b2..1c932bd 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -411,11 +411,13 @@ static int disconnected = 0; /* we may have been disconnected */ * We get here when a SIGPIPE is being raised, we can't do much in the * handler, just save the fact it was raised */ +#ifdef SA_SIGINFO static void vshCatchDisconnect(int sig, siginfo_t * siginfo, void* context ATTRIBUTE_UNUSED) { if ((sig == SIGPIPE) || (siginfo->si_signo == SIGPIPE)) disconnected++; } +#endif
/* * vshSetupSignals: @@ -425,6 +427,7 @@ static void vshCatchDisconnect(int sig, siginfo_t * siginfo, */ static void vshSetupSignals(void) { +#ifdef SA_SIGINFO struct sigaction sig_action;
sig_action.sa_sigaction = vshCatchDisconnect; @@ -432,6 +435,7 @@ vshSetupSignals(void) { sigemptyset(&sig_action.sa_mask);
sigaction(SIGPIPE, &sig_action, NULL); +#endif }
/* --
THis patch doesn't appear to have been pushed...
Regards, Daniel
You already included this into your "Fix Win32 portability problems" patch. so this patch is not necessary anymore. Matthias
Now the virsh tests compile at least. --- tests/testutils.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/tests/testutils.c b/tests/testutils.c index 8764673..4f17e51 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -280,6 +280,12 @@ int virtTestCaptureProgramOutput(const char *const argv[], } } } +#else /* !WIN32 */ +int virtTestCaptureProgramOutput(const char *const argv[] ATTRIBUTE_UNUSED, + char **buf ATTRIBUTE_UNUSED, + int buflen ATTRIBUTE_UNUSED) { + return -1; +} #endif /* !WIN32 */ -- 1.6.3.3
On Mon, Mar 22, 2010 at 02:25:38AM +0100, Matthias Bolte wrote:
Now the virsh tests compile at least. --- tests/testutils.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/tests/testutils.c b/tests/testutils.c index 8764673..4f17e51 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -280,6 +280,12 @@ int virtTestCaptureProgramOutput(const char *const argv[], } } } +#else /* !WIN32 */ +int virtTestCaptureProgramOutput(const char *const argv[] ATTRIBUTE_UNUSED, + char **buf ATTRIBUTE_UNUSED, + int buflen ATTRIBUTE_UNUSED) { + return -1; +} #endif /* !WIN32 */
ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
2010/3/22 Daniel P. Berrange <berrange@redhat.com>:
On Mon, Mar 22, 2010 at 02:25:38AM +0100, Matthias Bolte wrote:
Now the virsh tests compile at least. --- tests/testutils.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/tests/testutils.c b/tests/testutils.c index 8764673..4f17e51 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -280,6 +280,12 @@ int virtTestCaptureProgramOutput(const char *const argv[], } } } +#else /* !WIN32 */ +int virtTestCaptureProgramOutput(const char *const argv[] ATTRIBUTE_UNUSED, + char **buf ATTRIBUTE_UNUSED, + int buflen ATTRIBUTE_UNUSED) { + return -1; +} #endif /* !WIN32 */
ACK
Daniel
Thanks, pushed. Matthias
Add dummy bodies for HAVE_GETPWUID_R and HAVE_MNTENT_H dependent functions for MinGW builds. --- src/util/util.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- src/util/util.h | 4 ---- 2 files changed, 52 insertions(+), 6 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index 87b0714..3e89260 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -2585,7 +2585,46 @@ int virGetGroupID(const char *name, return 0; } -#endif + +#else /* HAVE_GETPWUID_R */ + +char * +virGetUserDirectory(uid_t uid ATTRIBUTE_UNUSED) +{ + virUtilError(VIR_ERR_INTERNAL_ERROR, + "%s", _("virGetUserDirectory is not avialable")); + + return NULL; +} + +char * +virGetUserName(uid_t uid ATTRIBUTE_UNUSED) +{ + virUtilError(VIR_ERR_INTERNAL_ERROR, + "%s", _("virGetUserName is not avialable")); + + return NULL; +} + +int virGetUserID(const char *name ATTRIBUTE_UNUSED, + uid_t *uid ATTRIBUTE_UNUSED) +{ + virUtilError(VIR_ERR_INTERNAL_ERROR, + "%s", _("virGetUserID is not avialable")); + + return 0; +} + + +int virGetGroupID(const char *name ATTRIBUTE_UNUSED, + gid_t *gid ATTRIBUTE_UNUSED) +{ + virUtilError(VIR_ERR_INTERNAL_ERROR, + "%s", _("virGetGroupID is not avialable")); + + return 0; +} +#endif /* HAVE_GETPWUID_R */ #ifdef HAVE_MNTENT_H @@ -2619,7 +2658,18 @@ cleanup: return ret; } -#endif + +#else /* HAVE_MNTENT_H */ + +char * +virFileFindMountPoint(const char *type ATTRIBUTE_UNUSED) +{ + errno = ENOSYS; + + return NULL; +} + +#endif /* HAVE_MNTENT_H */ #ifndef PROXY # if defined(UDEVADM) || defined(UDEVSETTLE) diff --git a/src/util/util.h b/src/util/util.h index e8fc565..e69eb5c 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -247,21 +247,17 @@ char *virGetHostname(virConnectPtr conn); int virKillProcess(pid_t pid, int sig); -# ifdef HAVE_GETPWUID_R char *virGetUserDirectory(uid_t uid); char *virGetUserName(uid_t uid); int virGetUserID(const char *name, uid_t *uid) ATTRIBUTE_RETURN_CHECK; int virGetGroupID(const char *name, gid_t *gid) ATTRIBUTE_RETURN_CHECK; -# endif int virRandomInitialize(unsigned int seed) ATTRIBUTE_RETURN_CHECK; int virRandom(int max); -# ifdef HAVE_MNTENT_H char *virFileFindMountPoint(const char *type); -# endif void virFileWaitForDevices(void); -- 1.6.3.3
On Mon, Mar 22, 2010 at 02:25:39AM +0100, Matthias Bolte wrote:
Add dummy bodies for HAVE_GETPWUID_R and HAVE_MNTENT_H dependent functions for MinGW builds. --- src/util/util.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- src/util/util.h | 4 ---- 2 files changed, 52 insertions(+), 6 deletions(-)
ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
2010/3/22 Daniel P. Berrange <berrange@redhat.com>:
On Mon, Mar 22, 2010 at 02:25:39AM +0100, Matthias Bolte wrote:
Add dummy bodies for HAVE_GETPWUID_R and HAVE_MNTENT_H dependent functions for MinGW builds. --- src/util/util.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- src/util/util.h | 4 ---- 2 files changed, 52 insertions(+), 6 deletions(-)
ACK
Daniel
Thanks, pushed. Matthias
On Mon, 2010-03-22 at 02:25 +0100, Matthias Bolte wrote:
Add dummy bodies for HAVE_GETPWUID_R and HAVE_MNTENT_H dependent functions for MinGW builds. --- src/util/util.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- src/util/util.h | 4 ---- 2 files changed, 52 insertions(+), 6 deletions(-)
diff --git a/src/util/util.c b/src/util/util.c index 87b0714..3e89260 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -2585,7 +2585,46 @@ int virGetGroupID(const char *name,
return 0; } -#endif + +#else /* HAVE_GETPWUID_R */ + +char * +virGetUserDirectory(uid_t uid ATTRIBUTE_UNUSED) +{ + virUtilError(VIR_ERR_INTERNAL_ERROR, + "%s", _("virGetUserDirectory is not avialable")); + + return NULL; +} + +char * +virGetUserName(uid_t uid ATTRIBUTE_UNUSED) +{ + virUtilError(VIR_ERR_INTERNAL_ERROR, + "%s", _("virGetUserName is not avialable")); + + return NULL; +} + +int virGetUserID(const char *name ATTRIBUTE_UNUSED, + uid_t *uid ATTRIBUTE_UNUSED) +{ + virUtilError(VIR_ERR_INTERNAL_ERROR, + "%s", _("virGetUserID is not avialable")); + + return 0; +} + + +int virGetGroupID(const char *name ATTRIBUTE_UNUSED, + gid_t *gid ATTRIBUTE_UNUSED) +{ + virUtilError(VIR_ERR_INTERNAL_ERROR, + "%s", _("virGetGroupID is not avialable")); + + return 0; +} +#endif /* HAVE_GETPWUID_R */
Minor typo - these should all be "available" rather than "avialable". Hope this helps, Paul
2010/4/6 Paul Jenner <psj@familyjenner.co.uk>:
On Mon, 2010-03-22 at 02:25 +0100, Matthias Bolte wrote:
Add dummy bodies for HAVE_GETPWUID_R and HAVE_MNTENT_H dependent functions for MinGW builds. --- src/util/util.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- src/util/util.h | 4 ---- 2 files changed, 52 insertions(+), 6 deletions(-)
diff --git a/src/util/util.c b/src/util/util.c index 87b0714..3e89260 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -2585,7 +2585,46 @@ int virGetGroupID(const char *name,
return 0; } -#endif + +#else /* HAVE_GETPWUID_R */ + +char * +virGetUserDirectory(uid_t uid ATTRIBUTE_UNUSED) +{ + virUtilError(VIR_ERR_INTERNAL_ERROR, + "%s", _("virGetUserDirectory is not avialable")); + + return NULL; +} + +char * +virGetUserName(uid_t uid ATTRIBUTE_UNUSED) +{ + virUtilError(VIR_ERR_INTERNAL_ERROR, + "%s", _("virGetUserName is not avialable")); + + return NULL; +} + +int virGetUserID(const char *name ATTRIBUTE_UNUSED, + uid_t *uid ATTRIBUTE_UNUSED) +{ + virUtilError(VIR_ERR_INTERNAL_ERROR, + "%s", _("virGetUserID is not avialable")); + + return 0; +} + + +int virGetGroupID(const char *name ATTRIBUTE_UNUSED, + gid_t *gid ATTRIBUTE_UNUSED) +{ + virUtilError(VIR_ERR_INTERNAL_ERROR, + "%s", _("virGetGroupID is not avialable")); + + return 0; +} +#endif /* HAVE_GETPWUID_R */
Minor typo - these should all be "available" rather than "avialable".
Hope this helps,
Paul
Thanks, I pushed a fix for this. Mathias
rsync is used to download .po files, but SKIP_PO=true is set and downloading .po files is skipped. This also fixes a problem with MinGW builds, because rsync is not available for MinGW. --- bootstrap.conf | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index fb862ad..be4ad21 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -130,7 +130,6 @@ git 1.5.5 gzip - libtool - perl 5.5 -rsync - tar - " -- 1.6.3.3
On Mon, Mar 22, 2010 at 02:25:40AM +0100, Matthias Bolte wrote:
rsync is used to download .po files, but SKIP_PO=true is set and downloading .po files is skipped.
This also fixes a problem with MinGW builds, because rsync is not available for MinGW. --- bootstrap.conf | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/bootstrap.conf b/bootstrap.conf index fb862ad..be4ad21 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -130,7 +130,6 @@ git 1.5.5 gzip - libtool - perl 5.5 -rsync - tar - "
ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
2010/3/22 Daniel P. Berrange <berrange@redhat.com>:
On Mon, Mar 22, 2010 at 02:25:40AM +0100, Matthias Bolte wrote:
rsync is used to download .po files, but SKIP_PO=true is set and downloading .po files is skipped.
This also fixes a problem with MinGW builds, because rsync is not available for MinGW. --- bootstrap.conf | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/bootstrap.conf b/bootstrap.conf index fb862ad..be4ad21 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -130,7 +130,6 @@ git 1.5.5 gzip - libtool - perl 5.5 -rsync - tar - "
ACK
Daniel
Thanks, pushed. Matthias
Even if gnulib can provide stubs, it won't help that much. So just replace affected util functions (virFileOperation and virDirCreate) with stubs on Windows. Both functions aren't used on libvirt's client side, so this is fine for MinGW builds. --- src/util/util.c | 37 +++++++++++++++++++++++++------------ 1 files changed, 25 insertions(+), 12 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index 3e89260..c312719 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -1221,7 +1221,7 @@ int virFileExists(const char *path) return(0); } - +# ifndef WIN32 static int virFileOperationNoFork(const char *path, int openflags, mode_t mode, uid_t uid, gid_t gid, virFileOperationHook hook, void *hookdata, @@ -1311,7 +1311,6 @@ error: return ret; } -# ifndef WIN32 int virFileOperation(const char *path, int openflags, mode_t mode, uid_t uid, gid_t gid, virFileOperationHook hook, void *hookdata, @@ -1532,19 +1531,33 @@ childerror: # else /* WIN32 */ -int virFileOperation(const char *path, int openflags, mode_t mode, - uid_t uid, gid_t gid, - virFileOperationHook hook, void *hookdata, - unsigned int flags) { - return virFileOperationNoFork(path, openflags, mode, uid, gid, - hook, hookdata, flags); +int virFileOperation(const char *path ATTRIBUTE_UNUSED, + int openflags ATTRIBUTE_UNUSED, + mode_t mode ATTRIBUTE_UNUSED, + uid_t uid ATTRIBUTE_UNUSED, + gid_t gid ATTRIBUTE_UNUSED, + virFileOperationHook hook ATTRIBUTE_UNUSED, + void *hookdata ATTRIBUTE_UNUSED, + unsigned int flags ATTRIBUTE_UNUSED) +{ + virUtilError(VIR_ERR_INTERNAL_ERROR, + "%s", _("virFileOperation is not implemented for WIN32")); + + return -1; } -int virDirCreate(const char *path, mode_t mode, - uid_t uid, gid_t gid, unsigned int flags) { - return virDirCreateNoFork(path, mode, uid, gid, flags); +int virDirCreate(const char *path ATTRIBUTE_UNUSED, + mode_t mode ATTRIBUTE_UNUSED, + uid_t uid ATTRIBUTE_UNUSED, + gid_t gid ATTRIBUTE_UNUSED, + unsigned int flags ATTRIBUTE_UNUSED) +{ + virUtilError(VIR_ERR_INTERNAL_ERROR, + "%s", _("virDirCreate is not implemented for WIN32")); + + return -1; } -# endif +# endif /* WIN32 */ static int virFileMakePathHelper(char *path) { struct stat st; -- 1.6.3.3
On Mon, Mar 22, 2010 at 02:25:41AM +0100, Matthias Bolte wrote:
Even if gnulib can provide stubs, it won't help that much. So just replace affected util functions (virFileOperation and virDirCreate) with stubs on Windows. Both functions aren't used on libvirt's client side, so this is fine for MinGW builds. --- src/util/util.c | 37 +++++++++++++++++++++++++------------ 1 files changed, 25 insertions(+), 12 deletions(-)
ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
2010/3/22 Daniel P. Berrange <berrange@redhat.com>:
On Mon, Mar 22, 2010 at 02:25:41AM +0100, Matthias Bolte wrote:
Even if gnulib can provide stubs, it won't help that much. So just replace affected util functions (virFileOperation and virDirCreate) with stubs on Windows. Both functions aren't used on libvirt's client side, so this is fine for MinGW builds. --- src/util/util.c | 37 +++++++++++++++++++++++++------------ 1 files changed, 25 insertions(+), 12 deletions(-)
ACK
Daniel
Thanks, pushed. Matthias
MSYS' ln doesn't work well in the way bootstrap uses it with relative paths. --- bootstrap.conf | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index be4ad21..d55dc71 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -74,6 +74,12 @@ MSGID_BUGS_ADDRESS=libvir-list@redhat.com COPYRIGHT_HOLDER='Red Hat, Inc.' SKIP_PO=true +# Enable copy-mode for MSYS/MinGW. MSYS' ln doesn't work well in the way +# bootstrap uses it with relative paths. +if test -n "$MSYSTEM"; then + copy=true +fi + # If "AM_GNU_GETTEXT(external" or "AM_GNU_GETTEXT([external]" # appears in configure.ac, exclude some unnecessary files. # Without grep's -E option (not portable enough, pre-configure), -- 1.6.3.3
On Mon, Mar 22, 2010 at 02:25:42AM +0100, Matthias Bolte wrote:
MSYS' ln doesn't work well in the way bootstrap uses it with relative paths. --- bootstrap.conf | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/bootstrap.conf b/bootstrap.conf index be4ad21..d55dc71 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -74,6 +74,12 @@ MSGID_BUGS_ADDRESS=libvir-list@redhat.com COPYRIGHT_HOLDER='Red Hat, Inc.' SKIP_PO=true
+# Enable copy-mode for MSYS/MinGW. MSYS' ln doesn't work well in the way +# bootstrap uses it with relative paths. +if test -n "$MSYSTEM"; then + copy=true +fi + # If "AM_GNU_GETTEXT(external" or "AM_GNU_GETTEXT([external]" # appears in configure.ac, exclude some unnecessary files. # Without grep's -E option (not portable enough, pre-configure),
ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
2010/3/22 Daniel P. Berrange <berrange@redhat.com>:
On Mon, Mar 22, 2010 at 02:25:42AM +0100, Matthias Bolte wrote:
MSYS' ln doesn't work well in the way bootstrap uses it with relative paths. --- bootstrap.conf | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/bootstrap.conf b/bootstrap.conf index be4ad21..d55dc71 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -74,6 +74,12 @@ MSGID_BUGS_ADDRESS=libvir-list@redhat.com COPYRIGHT_HOLDER='Red Hat, Inc.' SKIP_PO=true
+# Enable copy-mode for MSYS/MinGW. MSYS' ln doesn't work well in the way +# bootstrap uses it with relative paths. +if test -n "$MSYSTEM"; then + copy=true +fi + # If "AM_GNU_GETTEXT(external" or "AM_GNU_GETTEXT([external]" # appears in configure.ac, exclude some unnecessary files. # Without grep's -E option (not portable enough, pre-configure),
ACK
Daniel
Thanks, pushed. Matthias
sscanf doesn't support the L modifier on Windows and gnulib has no replacement for the scanf functions. Just replace the function with a stub on Windows, because it's not used on the libvirt client side. --- src/util/pci.c | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-) diff --git a/src/util/pci.c b/src/util/pci.c index 99ec22a..87cdb78 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -908,6 +908,9 @@ pciReAttachDevice(pciDevice *dev) * Returns 0 if we are clear to continue, and 1 if the hypervisor is still * holding onto the resource. */ + +#ifndef WIN32 + int pciWaitForDeviceCleanup(pciDevice *dev, const char *matcher) { @@ -976,6 +979,17 @@ pciWaitForDeviceCleanup(pciDevice *dev, const char *matcher) return ret; } +#else /* !WIN32 */ + +int +pciWaitForDeviceCleanup(pciDevice *dev ATTRIBUTE_UNUSED, + const char *matcher ATTRIBUTE_UNUSED) +{ + return 0; +} + +#endif /* !WIN32 */ + static char * pciReadDeviceID(pciDevice *dev, const char *id_name) { -- 1.6.3.3
On Mon, Mar 22, 2010 at 02:25:43AM +0100, Matthias Bolte wrote:
sscanf doesn't support the L modifier on Windows and gnulib has no replacement for the scanf functions. Just replace the function with a stub on Windows, because it's not used on the libvirt client side. --- src/util/pci.c | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-)
We already had this problem with printf(). For that gnulib provided us with a replacement that worked. We should probably pull in the scanf module from gnulib for equivalent compatability. Even though this code isn't technically required, other places may start using scanf & trip up on this problem Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On 03/22/2010 09:03 AM, Daniel P. Berrange wrote:
On Mon, Mar 22, 2010 at 02:25:43AM +0100, Matthias Bolte wrote:
sscanf doesn't support the L modifier on Windows and gnulib has no replacement for the scanf functions. Just replace the function with a stub on Windows, because it's not used on the libvirt client side. --- src/util/pci.c | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-)
We already had this problem with printf(). For that gnulib provided us with a replacement that worked. We should probably pull in the scanf module from gnulib for equivalent compatability. Even though this code isn't technically required, other places may start using scanf & trip up on this problem
But that's the point that Matthias made - currently, gnulib does NOT provide a scanf module. Why? Because scanf comes with its own set of usability pitfalls (scanf("%d",&int) cannot report whether integer overflow occurred), so no one has made it a high enough priority to start replacing the portability pitfalls. I've already mentioned that it would be a better cleanup to stop using *scanf altogether; but that would be an independent cleanup, unrelated to this particular patch. For this particular patch, mingw also lacks /proc/iomem, so the fopen earlier in pciWaitForDeviceCleanup should have already failed before we ever get to the problematic sscanf("%Lx"). Therefore, do we even need this patch? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Mon, Mar 22, 2010 at 10:54:09AM -0600, Eric Blake wrote:
On 03/22/2010 09:03 AM, Daniel P. Berrange wrote:
On Mon, Mar 22, 2010 at 02:25:43AM +0100, Matthias Bolte wrote:
sscanf doesn't support the L modifier on Windows and gnulib has no replacement for the scanf functions. Just replace the function with a stub on Windows, because it's not used on the libvirt client side. --- src/util/pci.c | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-)
We already had this problem with printf(). For that gnulib provided us with a replacement that worked. We should probably pull in the scanf module from gnulib for equivalent compatability. Even though this code isn't technically required, other places may start using scanf & trip up on this problem
But that's the point that Matthias made - currently, gnulib does NOT provide a scanf module. Why? Because scanf comes with its own set of usability pitfalls (scanf("%d",&int) cannot report whether integer overflow occurred), so no one has made it a high enough priority to start replacing the portability pitfalls.
Oh, I mis-read the original description!
I've already mentioned that it would be a better cleanup to stop using *scanf altogether; but that would be an independent cleanup, unrelated to this particular patch.
For this particular patch, mingw also lacks /proc/iomem, so the fopen earlier in pciWaitForDeviceCleanup should have already failed before we ever get to the problematic sscanf("%Lx"). Therefore, do we even need this patch?
I don't think its high priority to merge this, since its not an actual compile failure - I agree we'd be better to just kill off the use of scanf() altogether Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
2010/3/22 Daniel P. Berrange <berrange@redhat.com>:
On Mon, Mar 22, 2010 at 10:54:09AM -0600, Eric Blake wrote:
On 03/22/2010 09:03 AM, Daniel P. Berrange wrote:
On Mon, Mar 22, 2010 at 02:25:43AM +0100, Matthias Bolte wrote:
sscanf doesn't support the L modifier on Windows and gnulib has no replacement for the scanf functions. Just replace the function with a stub on Windows, because it's not used on the libvirt client side. --- src/util/pci.c | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-)
We already had this problem with printf(). For that gnulib provided us with a replacement that worked. We should probably pull in the scanf module from gnulib for equivalent compatability. Even though this code isn't technically required, other places may start using scanf & trip up on this problem
But that's the point that Matthias made - currently, gnulib does NOT provide a scanf module. Why? Because scanf comes with its own set of usability pitfalls (scanf("%d",&int) cannot report whether integer overflow occurred), so no one has made it a high enough priority to start replacing the portability pitfalls.
Oh, I mis-read the original description!
I've already mentioned that it would be a better cleanup to stop using *scanf altogether; but that would be an independent cleanup, unrelated to this particular patch.
For this particular patch, mingw also lacks /proc/iomem, so the fopen earlier in pciWaitForDeviceCleanup should have already failed before we ever get to the problematic sscanf("%Lx"). Therefore, do we even need this patch?
I don't think its high priority to merge this, since its not an actual compile failure - I agree we'd be better to just kill off the use of scanf() altogether
Daniel
Well, MinGW's GCC on Windows warns about the L modifier being unknown. So this patch was about the compile warning and not about an actual runtime error. I should have said that more clearly. I withdraw this patch, as it's superseded by the scanf/atoi removal patches I just posted. The next part of that series will also address the usage of the unportable %as field in a scanf call in the ESX driver that results in an runtime error on Windows. Matthias
Correctly disable pthread related code if pthread is not avialable, in order to get it compile with MinGW on Windows. --- src/remote/remote_driver.c | 6 ++++++ src/util/util.c | 10 ++++++++++ 2 files changed, 16 insertions(+), 0 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index b7e1c03..1476f19 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8507,7 +8507,9 @@ remoteIOEventLoop(virConnectPtr conn, struct remote_thread_call *tmp = priv->waitDispatch; struct remote_thread_call *prev; char ignore; +#ifdef HAVE_PTHREAD_H sigset_t oldmask, blockedsigs; +#endif fds[0].events = fds[0].revents = 0; fds[1].events = fds[1].revents = 0; @@ -8534,18 +8536,22 @@ remoteIOEventLoop(virConnectPtr conn, * after the call (RHBZ#567931). Same for SIGCHLD and SIGPIPE * at the suggestion of Paolo Bonzini and Daniel Berrange. */ +#ifdef HAVE_PTHREAD_H sigemptyset (&blockedsigs); sigaddset (&blockedsigs, SIGWINCH); sigaddset (&blockedsigs, SIGCHLD); sigaddset (&blockedsigs, SIGPIPE); ignore_value (pthread_sigmask(SIG_BLOCK, &blockedsigs, &oldmask)); +#endif repoll: ret = poll(fds, ARRAY_CARDINALITY(fds), -1); if (ret < 0 && errno == EAGAIN) goto repoll; +#ifdef HAVE_PTHREAD_H ignore_value (pthread_sigmask(SIG_SETMASK, &oldmask, NULL)); +#endif remoteDriverLock(priv); diff --git a/src/util/util.c b/src/util/util.c index c312719..0d5e0a8 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -316,7 +316,9 @@ static int virClearCapabilities(void) */ int virFork(pid_t *pid) { +# ifdef HAVE_PTHREAD_H sigset_t oldmask, newmask; +# endif struct sigaction sig_action; int saved_errno, ret = -1; @@ -326,6 +328,7 @@ int virFork(pid_t *pid) { * Need to block signals now, so that child process can safely * kill off caller's signal handlers without a race. */ +# ifdef HAVE_PTHREAD_H sigfillset(&newmask); if (pthread_sigmask(SIG_SETMASK, &newmask, &oldmask) != 0) { saved_errno = errno; @@ -333,6 +336,7 @@ int virFork(pid_t *pid) { "%s", _("cannot block signals")); goto cleanup; } +# endif /* Ensure we hold the logging lock, to protect child processes * from deadlocking on another thread's inherited mutex state */ @@ -345,9 +349,11 @@ int virFork(pid_t *pid) { virLogUnlock(); if (*pid < 0) { +# ifdef HAVE_PTHREAD_H /* attempt to restore signal mask, but ignore failure, to avoid obscuring the fork failure */ ignore_value (pthread_sigmask(SIG_SETMASK, &oldmask, NULL)); +# endif virReportSystemError(saved_errno, "%s", _("cannot fork child process")); goto cleanup; @@ -357,6 +363,7 @@ int virFork(pid_t *pid) { /* parent process */ +# ifdef HAVE_PTHREAD_H /* Restore our original signal mask now that the child is safely running */ if (pthread_sigmask(SIG_SETMASK, &oldmask, NULL) != 0) { @@ -364,6 +371,7 @@ int virFork(pid_t *pid) { virReportSystemError(errno, "%s", _("cannot unblock signals")); goto cleanup; } +# endif ret = 0; } else { @@ -399,6 +407,7 @@ int virFork(pid_t *pid) { sigaction(i, &sig_action, NULL); } +# ifdef HAVE_PTHREAD_H /* Unmask all signals in child, since we've no idea what the caller's done with their signal mask and don't want to propagate that to children */ @@ -408,6 +417,7 @@ int virFork(pid_t *pid) { virReportSystemError(errno, "%s", _("cannot unblock signals")); goto cleanup; } +# endif ret = 0; } -- 1.6.3.3
On Mon, Mar 22, 2010 at 02:25:44AM +0100, Matthias Bolte wrote:
Correctly disable pthread related code if pthread is not avialable, in order to get it compile with MinGW on Windows. --- src/remote/remote_driver.c | 6 ++++++ src/util/util.c | 10 ++++++++++ 2 files changed, 16 insertions(+), 0 deletions(-)
ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
2010/3/22 Daniel P. Berrange <berrange@redhat.com>:
On Mon, Mar 22, 2010 at 02:25:44AM +0100, Matthias Bolte wrote:
Correctly disable pthread related code if pthread is not avialable, in order to get it compile with MinGW on Windows. --- src/remote/remote_driver.c | 6 ++++++ src/util/util.c | 10 ++++++++++ 2 files changed, 16 insertions(+), 0 deletions(-)
ACK
Daniel
Thanks, pushed. Matthias
On 03/22/2010 02:25 AM, Matthias Bolte wrote:
Correctly disable pthread related code if pthread is not avialable, in order to get it compile with MinGW on Windows.
What about, instead #ifndef HAVE_PTHREAD_H #define pthread_sigmask sigprocmask #endif (or actually the equivalent if...AC_DEFINE...fi in configure)? Paolo
virSetCloseExec and virExecDaemonize were missing a body on Windows. --- src/util/util.c | 34 ++++++++++++++++++++++++++++++---- 1 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index 0d5e0a8..119f0cd 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -954,7 +954,12 @@ virRunWithHook(const char *const*argv, return ret; } -# else /* __MINGW32__ */ +# else /* WIN32 */ + +int virSetCloseExec(int fd ATTRIBUTE_UNUSED) +{ + return -1; +} int virRunWithHook(const char *const *argv ATTRIBUTE_UNUSED, @@ -965,7 +970,8 @@ virRunWithHook(const char *const *argv ATTRIBUTE_UNUSED, if (status) *status = ENOTSUP; else - virUtilError(VIR_ERR_INTERNAL_ERROR, __FUNCTION__); + virUtilError(VIR_ERR_INTERNAL_ERROR, + "%s", _("virRunWithHook is not implemented for WIN32")); return -1; } @@ -979,11 +985,31 @@ virExec(const char *const*argv ATTRIBUTE_UNUSED, int *errfd ATTRIBUTE_UNUSED, int flags ATTRIBUTE_UNUSED) { - virUtilError(VIR_ERR_INTERNAL_ERROR, __FUNCTION__); + virUtilError(VIR_ERR_INTERNAL_ERROR, + "%s", _("virExec is not implemented for WIN32")); return -1; } -# endif /* __MINGW32__ */ +int +virExecDaemonize(const char *const*argv ATTRIBUTE_UNUSED, + const char *const*envp ATTRIBUTE_UNUSED, + const fd_set *keepfd ATTRIBUTE_UNUSED, + pid_t *retpid ATTRIBUTE_UNUSED, + int infd ATTRIBUTE_UNUSED, + int *outfd ATTRIBUTE_UNUSED, + int *errfd ATTRIBUTE_UNUSED, + int flags ATTRIBUTE_UNUSED, + virExecHook hook ATTRIBUTE_UNUSED, + void *data ATTRIBUTE_UNUSED, + char *pidfile ATTRIBUTE_UNUSED) +{ + virUtilError(VIR_ERR_INTERNAL_ERROR, + "%s", _("virExecDaemonize is not implemented for WIN32")); + + return -1; +} + +# endif /* WIN32 */ int virRun(const char *const*argv, -- 1.6.3.3
On Mon, Mar 22, 2010 at 02:25:45AM +0100, Matthias Bolte wrote:
virSetCloseExec and virExecDaemonize were missing a body on Windows. --- src/util/util.c | 34 ++++++++++++++++++++++++++++++---- 1 files changed, 30 insertions(+), 4 deletions(-)
ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
2010/3/22 Daniel P. Berrange <berrange@redhat.com>:
On Mon, Mar 22, 2010 at 02:25:45AM +0100, Matthias Bolte wrote:
virSetCloseExec and virExecDaemonize were missing a body on Windows. --- src/util/util.c | 34 ++++++++++++++++++++++++++++++---- 1 files changed, 30 insertions(+), 4 deletions(-)
ACK
Daniel
Thanks, pushed. Matthias
Picks up fix for gethostname compilation problems on mingw. * .gnulib: Update to latest. * build-aux/.gitignore: Regenerate. ---
There is also a gnulib issue with MinGW on Windows pending. Eric and I are working on a patch for it.
This should fix it. http://lists.gnu.org/archive/html/bug-gnulib/2010-03/msg00306.html .gnulib | 2 +- build-aux/.gitignore | 1 + 2 files changed, 2 insertions(+), 1 deletions(-) diff --git a/.gnulib b/.gnulib index 10d66ae..3390966 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 10d66aedfdd610f731c8c54152b9dfca3efbee12 +Subproject commit 339096647215cb4a675239020ec247b6031fce1f diff --git a/build-aux/.gitignore b/build-aux/.gitignore index 29b1e03..8717628 100644 --- a/build-aux/.gitignore +++ b/build-aux/.gitignore @@ -6,6 +6,7 @@ /useless-if-before-free /vc-list-files arg-nonnull.h +c++defs.h config.rpath gitlog-to-changelog mkinstalldirs -- 1.6.6.1
participants (5)
-
Daniel P. Berrange -
Eric Blake -
Matthias Bolte -
Paolo Bonzini -
Paul Jenner