[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/5] hyperv: Add configure check for OpenWSMAN



2011/7/13 Eric Blake <eblake redhat com>:
> On 07/13/2011 01:01 PM, Matthias Bolte wrote:
>> ---
>>  configure.ac |   38 ++++++++++++++++++++++++++++++++++++++
>>  1 files changed, 38 insertions(+), 0 deletions(-)
>
> I'd like to see the libvirt.spec.in changes from patch 2/5 squashed back
> into this patch - that is, both introduce the new ./configure option,
> and control whether the new option gets used in an rpm, all in the same
> patch (even if the option is otherwise a no-op until the rest of patch
> 2).  So I'll review those changes here:
>
>> diff --git a/libvirt.spec.in b/libvirt.spec.in
>> index 230237e..c971681 100644
>> --- a/libvirt.spec.in
>> +++ b/libvirt.spec.in
>> @@ -50,6 +50,7 @@
>>  # Then the hypervisor drivers that talk a native remote protocol
>>  %define with_phyp          0%{!?_without_phyp:1}
>>  %define with_esx           0%{!?_without_esx:1}
>> +%define with_hyperv        0%{!?_without_hyperv:1}
>>  %define with_xenapi        0%{!?_without_xenapi:1}
>>
>>  # Then the secondary host drivers
>> @@ -437,6 +438,9 @@ BuildRequires: libcurl-devel
>>  BuildRequires: curl-devel
>>  %endif
>>  %endif
>> +%if %{with_hyperv}
>> +BuildRequires: openwsman-devel >= 2.2.6
>> +%endif
>
> On Fedora, the package is named libwsman-devel (with counterparts
> openwsman-server, libwsman1, and openwsman-client).  So this line needs
> to be fixed.
>
>> diff --git a/configure.ac b/configure.ac
>> index e9d5be4..d7ebe79 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -66,6 +66,7 @@ XMLRPC_REQUIRED=1.14.0
>>  HAL_REQUIRED=0.5.0
>>  DEVMAPPER_REQUIRED=1.0.0
>>  LIBCURL_REQUIRED="7.18.0"
>> +OPENWSMAN_REQUIRED="2.2.6"
>
> Fedora 14 is only at 2.2.3 (the libwsman-devel package), Fedora 15 at
> 2.2.4, and rawhide at 2.2.5, which will slightly hamper my ability to
> test remaining patches (I can inspect them, but can't compile-test them,
> without installing an out-of-distro build).  Are we sure we can't
> support anything earlier than 2.2.6?  Which distros already have 2.2.6
> available?
>
> But that's not necessarily a show-stopper for this patch.

Actually, I'd like to depend on a not yet existing future version that
includes the patches I've posted upstream for it ..

Memory leaks in wsmc_action_enum_and_pull and wsmc_release
http://sf.net/tracker/?func=detail&aid=3317862&group_id=151841&atid=782284

ws_serializer_free_mem doesn't work
http://sf.net/tracker/?func=detail&aid=3317870&group_id=151841&atid=782284

TRACE_EXIT can be skipped in some functions
http://sf.net/tracker/?func=detail&aid=3317873&group_id=151841&atid=782284

optarg and optind in u/os.h conflict with system headers
http://sf.net/tracker/?func=detail&aid=3323325&group_id=151841&atid=782284

wsman-xml-serializer.h misses SER_NS_INT* macros
http://sf.net/tracker/?func=detail&aid=3325111&group_id=151841&atid=782284

.. just to reference them. Most important are the patches for
ws_serializer_free_mem. This problem could be worked around in libvirt
code, but would require duplicating several 100 lines of OpenWSMAN
code.

In the future we'll need to detect the actual version and enable some
currently disabled code sections in the driver to improve the memory
usage footprint. We'll need to get this info from pkg-config as
OpenWSMAN doesn't provide a way to get it's version at compile time.

Anyway, I checked and tested the history of OpenWSMAN and it seems
that this requirement can be relaxed to 2.2.3 without problems.

>> +
>> +if test "$with_hyperv" = "yes" || test "$with_hyperv" = "check"; then
>> +    PKG_CHECK_MODULES(OPENWSMAN, openwsman >= $OPENWSMAN_REQUIRED, [
>
> Autoconf suggests using this quoting:
>
> PKG_CHECK_MODULES([OPENWSMAN], [openwsman >= $OPENWSMAN_REQUIRED], [
>
> ACK with the spec file changes pulled in and nits fixed.

I'd like to delay pushing this series until it's completely ACK'ed. In
the meantime here's a v2 of this patch to simplify compile testing the
whole series.

-- 
Matthias Bolte
http://photron.blogspot.com
From ee290cd4521c02834b7843df6510877bcd40372d Mon Sep 17 00:00:00 2001
From: Matthias Bolte <matthias bolte googlemail com>
Date: Wed, 13 Jul 2011 16:05:18 +0200
Subject: [PATCHv2 1/5] hyperv: Add configure check for OpenWSMAN

---

v2:
- relax OpenWSMAN requirement to 2.2.3
- move libvirt.spec.in change from 2/5 here and fix OpenWSMAN package name
- qoute all PKG_CHECK_MODULES parameters

 configure.ac    |   38 ++++++++++++++++++++++++++++++++++++++
 libvirt.spec.in |    9 +++++++++
 2 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/configure.ac b/configure.ac
index e9d5be4..d7ebe79 100644
--- a/configure.ac
+++ b/configure.ac
@@ -66,6 +66,7 @@ XMLRPC_REQUIRED=1.14.0
 HAL_REQUIRED=0.5.0
 DEVMAPPER_REQUIRED=1.0.0
 LIBCURL_REQUIRED="7.18.0"
+OPENWSMAN_REQUIRED="2.2.3"
 LIBPCAP_REQUIRED="1.0.0"
 LIBNL_REQUIRED="1.1"
 LIBSSH2_REQUIRED="1.0"
@@ -275,6 +276,8 @@ AC_ARG_WITH([lxc],
   AC_HELP_STRING([--with-lxc], [add Linux Container support @<:@default=check@:>@]),[],[with_lxc=check])
 AC_ARG_WITH([esx],
   AC_HELP_STRING([--with-esx], [add ESX support @<:@default=check@:>@]),[],[with_esx=check])
+AC_ARG_WITH([hyperv],
+  AC_HELP_STRING([--with-hyperv], [add Hyper-V support @<:@default=check@:>@]),[],[with_hyperv=check])
 AC_ARG_WITH([test],
   AC_HELP_STRING([--with-test], [add test driver support @<:@default=yes@:>@]),[],[with_test=yes])
 AC_ARG_WITH([remote],
@@ -1912,6 +1915,35 @@ LIBCURL_CFLAGS="-DCURL_DISABLE_TYPECHECK $LIBCURL_CFLAGS"
 AC_SUBST([LIBCURL_CFLAGS])
 AC_SUBST([LIBCURL_LIBS])
 
+
+dnl
+dnl check for openwsman (Hyper-V)
+dnl
+
+OPENWSMAN_CFLAGS=""
+OPENWSMAN_LIBS=""
+
+if test "$with_hyperv" = "yes" || test "$with_hyperv" = "check"; then
+    PKG_CHECK_MODULES([OPENWSMAN], [openwsman >= $OPENWSMAN_REQUIRED], [
+        if test "$with_hyperv" = "check"; then
+            with_hyperv=yes
+        fi
+    ], [
+        if test "$with_hyperv" = "check"; then
+            with_hyperv=no
+            AC_MSG_NOTICE([openwsman is required for the Hyper-V driver, disabling it])
+        elif test "$with_hyperv" = "yes"; then
+            AC_MSG_ERROR([openwsman >= $OPENWSMAN_REQUIRED is required for the Hyper-V driver])
+        fi
+    ])
+fi
+
+if test "$with_hyperv" = "yes" ; then
+    AC_DEFINE_UNQUOTED([WITH_HYPERV], 1, [whether Hyper-V driver is enabled])
+fi
+AM_CONDITIONAL([WITH_HYPERV], [test "$with_hyperv" = "yes"])
+
+
 dnl
 dnl check for python
 dnl
@@ -2414,6 +2446,7 @@ AC_MSG_NOTICE([xenlight: $with_libxl])
 AC_MSG_NOTICE([     LXC: $with_lxc])
 AC_MSG_NOTICE([    PHYP: $with_phyp])
 AC_MSG_NOTICE([     ESX: $with_esx])
+AC_MSG_NOTICE([ Hyper-V: $with_hyperv])
 AC_MSG_NOTICE([    Test: $with_test])
 AC_MSG_NOTICE([  Remote: $with_remote])
 AC_MSG_NOTICE([ Network: $with_network])
@@ -2455,6 +2488,11 @@ AC_MSG_NOTICE([ libcurl: $LIBCURL_CFLAGS $LIBCURL_LIBS])
 else
 AC_MSG_NOTICE([ libcurl: no])
 fi
+if test "$with_hyperv" = "yes" ; then
+AC_MSG_NOTICE([openwsman: $OPENWSMAN_CFLAGS $OPENWSMAN_LIBS])
+else
+AC_MSG_NOTICE([openwsman: no])
+fi
 if test "$with_libssh2" != "no" ; then
 AC_MSG_NOTICE([ libssh2: $LIBSSH2_CFLAGS $LIBSSH2_LIBS])
 else
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 230237e..c971681 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -50,6 +50,7 @@
 # Then the hypervisor drivers that talk a native remote protocol
 %define with_phyp          0%{!?_without_phyp:1}
 %define with_esx           0%{!?_without_esx:1}
+%define with_hyperv        0%{!?_without_hyperv:1}
 %define with_xenapi        0%{!?_without_xenapi:1}
 
 # Then the secondary host drivers
@@ -437,6 +438,9 @@ BuildRequires: libcurl-devel
 BuildRequires: curl-devel
 %endif
 %endif
+%if %{with_hyperv}
+BuildRequires: libwsman-devel >= 2.2.3
+%endif
 %if %{with_audit}
 BuildRequires: audit-libs-devel
 %endif
@@ -569,6 +573,10 @@ of recent versions of Linux (and other OSes).
 %define _without_esx --without-esx
 %endif
 
+%if ! %{with_hyperv}
+%define _without_hyperv --without-hyperv
+%endif
+
 %if ! %{with_vmware}
 %define _without_vmware --without-vmware
 %endif
@@ -687,6 +695,7 @@ of recent versions of Linux (and other OSes).
            %{?_without_one} \
            %{?_without_phyp} \
            %{?_without_esx} \
+           %{?_without_hyperv} \
            %{?_without_vmware} \
            %{?_without_network} \
            %{?_with_rhel5_api} \
-- 
1.7.4.1


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]