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

[Jim Fehlig <jfehlig@suse.com>]

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