From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Stabellini Subject: Re: [PATCH 8/8] raisin: RFC Add blktap2 as an external tree Date: Mon, 20 Apr 2015 11:08:53 +0100 Message-ID: References: <1429201182-1044-1-git-send-email-george.dunlap@eu.citrix.com> <1429201182-1044-8-git-send-email-george.dunlap@eu.citrix.com> <5534C910.5080506@eu.citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5534C910.5080506@eu.citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: George Dunlap Cc: xen-devel@lists.xen.org, Stefano Stabellini , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On Mon, 20 Apr 2015, George Dunlap wrote: > 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 > >> --- > >> 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 > >> --- > >> 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.) If you test it and it works both with and without blktap, then I am OK with it as is. > > 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 >