Re: [libvirt] [Xen-devel] [xen-unstable bisection] complete build-i386-libvirt
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)
Dario Faggioli wrote:
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).
Patch2 is more along the lines of current practice wrt LIBXL_HAVE_.
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?
Yes.
If yes, is it an issue?
As you say, only when the new parameter is actually used. But that will cause build failures when warnings are treated as errors.
If yes, a big enough one to make us prefer patch2?
Yes, I think so. And as mentioned above, it is similar to how other LIBXL_HAVE_ is handled. Regards, Jim
On lun, 2014-06-30 at 11:14 -0600, Jim Fehlig wrote:
Dario Faggioli wrote:
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?
Yes.
If yes, is it an issue?
As you say, only when the new parameter is actually used. But that will cause build failures when warnings are treated as errors.
If yes, a big enough one to make us prefer patch2?
Yes, I think so. And as mentioned above, it is similar to how other LIBXL_HAVE_ is handled.
Patch2 it is then: http://lists.xen.org/archives/html/xen-devel/2014-06/msg03930.html Thanks and Regards, Dario -- <<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)
participants (2)
-
Dario Faggioli -
Jim Fehlig