Re: [libvirt] [Xen-devel] [xen-unstable bisection] complete build-i386-libvirt

[Dario Faggioli <dario.faggioli@citrix.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 2984 bytes --]

On lun, 2014-06-30 at 08:11 +0100, Ian Campbell wrote:
> On Sun, 2014-06-29 at 18:35 +0100, xen.org wrote:
> > branch xen-unstable
> > xen branch xen-unstable
> > job build-i386-libvirt
> > test libvirt-build
> > 
> > Tree: gnulib_libvirt git://drall.uk.xensource.com:9419/git://git.sv.gnu.org/gnulib.git%20[fetch=try]
> > Tree: libvirt git://xenbits.xen.org/libvirt.git
> > Tree: qemu git://xenbits.xen.org/staging/qemu-xen-unstable.git
> > Tree: qemuu git://xenbits.xen.org/staging/qemu-upstream-unstable.git
> > Tree: xen git://xenbits.xen.org/xen.git
> > 
> > *** Found and reproduced problem changeset ***
> > 
> >   Bug is in tree:  xen git://xenbits.xen.org/xen.git
> >   Bug introduced:  871b43a309d80ac99458c13c2c3da8d15c482d30
> >   Bug not present: 6cc89d3101d8874e01a69a89a65736a2adfbd199
> > 
> > 
> >   commit 871b43a309d80ac99458c13c2c3da8d15c482d30
> >   Author: Dario Faggioli <dario.faggioli@citrix.com>
> >   Date:   Fri Jun 20 18:19:12 2014 +0200
> >   
> >       libxl: get and set soft affinity
> 
> Dario,
> 
> libvirt doesn't use the LIBXL_API_VERSION mechanism but instead uses the
> LIBXL_HAVE stuff to retain compatibility.
> 
> Will you be able to send a patch against libvirt today to make it use
> the new interface (conditional on LIBXL_HAVE_VCPUINFO_SOFT_AFFINITY)?
> 
So, brief recap for the ones not knowing the details of this, libxl
interface for vcpu pinning is changing (basically,
libxl_set_vcpuaffinity() wants one more param).

Libxl provides some ifdefs for these situations, and in this case, the
gate to be used is, as Ian is saying:

#ifdef LIBXL_HAVE_VCPUINFO_SOFT_AFFINITY

One possible approach is to enclose all the calls into such
#ifdef-#endif but, although there are only two of them right now, I
don't like it (what if we need more calls in the future?).

I could come up with the alternatives attached to this message. In
patch1, I use the new interface in the code and #define it to the old
one if !LIBXL_HAV_VCPUINFO_SOFT_AFFINITY. In patch2 I do the opposite
(keep old interface in the code and redefine to new, with additional
param equal to NULL).

I like patch1 better, but I think it can cause "unused variable" like
warnings if, at some point in future, we will actually use the new soft
affinity parameter, when compiling on a version of libxl that does not
define HAVE_VCPUINFO_SOFT_AFFINITY, can't it? If yes, is it an issue? If
yes, a big enough one to make us prefer patch2?

Just let me know your thoughts, and I'll submit the one you prefer
appropriately.

Regards,
Dario

PS. patches not tested, I'm updating my xen+libvirt testbox. Will be
able to test soon (for sure within today)

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.1.2: patch1 --]
[-- Type: text/x-patch, Size: 2068 bytes --]

diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 6aa36d2..6449b46 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -55,6 +55,13 @@
 # define LIBXL_DUMP_DIR LIBXL_LIB_DIR "/dump"
 # define LIBXL_BOOTLOADER_PATH BINDIR "/pygrub"
 
+# ifndef LIBXL_HAVE_VCPUINFO_SOFT_AFFINITY
+#  define libxl_set_vcpuaffinity(ctx, domid, vcpuid, map, softmap) \
+    libxl_set_vcpuaffinity((ctx), (domid), (vcpuid), (map))
+#  define libxl_set_vcpuaffinity_all(ctx, domid, max_vcpus, map, softmap) \
+    libxl_set_vcpuaffinity_all((ctx), (domid), (max_vcpus), (map))
+# endif
+
 
 typedef struct _libxlDriverPrivate libxlDriverPrivate;
 typedef libxlDriverPrivate *libxlDriverPrivatePtr;
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index da3f241..fafd9c5 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1029,7 +1029,7 @@ libxlDomainSetVcpuAffinities(libxlDriverPrivatePtr driver, virDomainObjPtr vm)
         map.size = cpumaplen;
         map.map = cpumap;
 
-        if (libxl_set_vcpuaffinity(priv->ctx, def->id, vcpu, &map) != 0) {
+        if (libxl_set_vcpuaffinity(priv->ctx, def->id, vcpu, &map, NULL) != 0) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("Failed to pin vcpu '%d' with libxenlight"), vcpu);
             goto cleanup;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 646c9b9..6f4a2f2 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1981,7 +1981,8 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu,
         libxlDomainObjPrivatePtr priv;
 
         priv = vm->privateData;
-        if (libxl_set_vcpuaffinity(priv->ctx, vm->def->id, vcpu, &map) != 0) {
+        if (libxl_set_vcpuaffinity(priv->ctx, vm->def->id,
+                                   vcpu, &map, NULL) != 0) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("Failed to pin vcpu '%d' with libxenlight"),
                            vcpu);

[-- Attachment #1.1.3: patch2 --]
[-- Type: text/x-patch, Size: 718 bytes --]

diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 6aa36d2..164b578 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -55,6 +55,13 @@
 # define LIBXL_DUMP_DIR LIBXL_LIB_DIR "/dump"
 # define LIBXL_BOOTLOADER_PATH BINDIR "/pygrub"
 
+# ifdef LIBXL_HAVE_VCPUINFO_SOFT_AFFINITY
+#  define libxl_set_vcpuaffinity(ctx, domid, vcpuid, map) \
+    libxl_set_vcpuaffinity((ctx), (domid), (vcpuid), (map), NULL)
+#  define libxl_set_vcpuaffinity_all(ctx, domid, max_vcpus, map) \
+    libxl_set_vcpuaffinity_all((ctx), (domid), (max_vcpus), (map), NULL)
+# endif
+
 
 typedef struct _libxlDriverPrivate libxlDriverPrivate;
 typedef libxlDriverPrivate *libxlDriverPrivatePtr;

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]