Re: [PATCH 8/8] raisin: RFC Add blktap2 as an external tree

[George Dunlap <george.dunlap@eu.citrix.com>]

On 04/17/2015 11:50 AM, Stefano Stabellini wrote:
> On Thu, 16 Apr 2015, George Dunlap wrote:
>> Until we start hosting the blktap repo on xenbits, use the one from github.
>>
>> Also, we need to pass in the directories where to find the
>> libblktapctl headers in the Xen build.
>>
>> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
>> ---
>> Note: For now use the "upstream" XenServer repo.
>>
>> Also, to build with this you need the patches to allow Xen to use an out-of-tree blktap,
>> which can be found here:
>>
>> git://xenbits.xen.org/people/gdunlap/xen.git  out/blktap25/rfc-v2
>>
>> CC: Stefano Stabellini <stefano.stabellini@citrix.com>
>> ---
>>  components/blktap | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>>  components/series |  1 +
>>  components/xen    |  5 +++++
>>  defconfig         |  9 ++++++---
>>  4 files changed, 59 insertions(+), 3 deletions(-)
>>  create mode 100644 components/blktap
>>
>> diff --git a/components/blktap b/components/blktap
>> new file mode 100644
>> index 0000000..3062b2a
>> --- /dev/null
>> +++ b/components/blktap
>> @@ -0,0 +1,47 @@
>> +#!/usr/bin/env bash
>> +
>> +function blktap_check_package() {
>> +    local DEP_Debian_common="build-essential autoconf libtool tar libaio-dev uuid-dev openssl-dev"
>> +    local DEP_Debian_x86_32="$DEP_Debian_common"
>> +    local DEP_Debian_x86_64="$DEP_Debian_x86_32"
>> +    local DEP_Debian_arm32="$DEP_Debian_common"
>> +    local DEP_Debian_arm64="$DEP_Debian_arm32"
>> +
>> +    local DEP_Fedora_common="make gcc python-devel autoconf libtool tar file libaio-devel libuuid-devel openssl-devel"
>> +    local DEP_Fedora_x86_32="$DEP_Fedora_common"
>> +    local DEP_Fedora_x86_64="$DEP_Fedora_x86_32"
>> +
>> +    local DEP_CentOS_common="make gcc python-devel autoconf libtool tar file libaio-devel libuuid-devel openssl-devel"
> 
> DEP_CentOS_common="$DEP_Fedora_common"
> 
> 
>> +    local DEP_CentOS_x86_32="$DEP_Redhat_common"
>> +    local DEP_CentOS_x86_64="$DEP_Redhat_x86_32"
> 
> Redhat? I don't know no Redhat.

Sorry, artifacts from a previous version of the patch...
>> +    fi
>> +}
>> +
>> +function blktap_configure() {
>> +    :
>> +}
>> +
>> +function blktap_unconfigure() {
>> +    :
>> +}
> 
> Much better than my echo. Maybe submit a patch to replace useless echo
> in configure and unconfigure functions with :

I can put it on my to-do list, but it may be a while before it reaches
the top. :-)

>> diff --git a/components/series b/components/series
>> index 8f614f0..953800a 100644
>> --- a/components/series
>> +++ b/components/series
>> @@ -1,3 +1,4 @@
>> +blktap
>>  xen
> 
> I take this is on purpose so that blktap is built before xen

Yes, as Xen has to link against the libraries.

>>  qemu
>>  grub
>> diff --git a/components/xen b/components/xen
>> index f2e1254..03a1970 100644
>> --- a/components/xen
>> +++ b/components/xen
>> @@ -29,7 +29,12 @@ function xen_build() {
>>      cd "$BASEDIR"
>>      git-checkout $XEN_URL $XEN_REVISION xen-dir
>>      cd xen-dir
>> +    CFLAGS="-I$INST_DIR/$PREFIX/include/blktap -I$INST_DIR/$PREFIX/include/vhd" \
>> +    LDFLAGS="-L$INST_DIR/$PREFIX/lib -Wl,-rpath-link=$INST_DIR/$PREFIX/lib" \
>>      ./configure --prefix=$PREFIX --with-system-qemu=/usr/bin/qemu-system-i386
>> +    CFLAGS_libblktapctl="-I$INST_DIR/$PREFIX/include/blktap -I$INST_DIR/$PREFIX/include/vhd" \
>> +    LDLIBS_libblktapctl="-L$INST_DIR/$PREFIX/lib -lblktapctl" \
>> +    SHLIB_libblktapctl="-Wl,-rpath-link=$INST_DIR/$PREFIX/lib" \
> 
> Does this work even if blktap is actually disabled in the raisin build?
> If so, then fine.

I don't think I've tested it, but in theory yes it should work.  If
blktap support isn't specified, the patch series I submitted to
xen-unstable will automatically enable blktap if found and disable it if
not found.  So if you have a clean raisin repo and just build the xen
component, it should build it without blktap.

(With my patches,  if you specify --enable-blktap and it can't find it,
config will throw an error.  But we're not enabling it.)

> Actually I think that the xen configure script should be able to figure
> out whether it can pick up blktap from an external location
> automatically with the CFLAGS and LDFLAGS you set.

Actually, what would be better is if you could just set
"-I$INST_DIR/$PREFIX/include" and "-L$INST_DIR/$PREFIX/lib" and be done
with it.  But the blktap2 headers are broken at the moment: they're
placed in subdirectories on install time that don't reflect the
directory structure used when building, so if you don't specify both
subdirectories explicitly, it won't compile.

That's something I plan to fix, but it may take a bit to get back to it.

 -George