Re: [Xen-devel] [xen-unstable bisection] complete test-amd64-i386-rhel6hvm-amd

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2012-02-27 at 09:03 +0000, Jan Beulich wrote:
> >>> On 25.02.12 at 01:56, xen.org <ian.jackson@xxxxxxxxxxxxx> wrote:
> > branch xen-unstable
> > xen branch xen-unstable
> > job test-amd64-i386-rhel6hvm-amd
> > test redhat-install
> > 
> > Tree: linux git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.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 http://xenbits.xen.org/staging/xen-unstable.hg 
> > 
> > *** Found and reproduced problem changeset ***
> 
> I had warned a number of times that a change like this should not be
> done: Afaict, this broke qemu-xen (and upstream qemu as well), which
> use BLKIF_MAX_SEGMENTS_PER_REQUEST without caring to set a
> specific __XEN_INTERFACE_VERSION__ (the tools generally just use
> 'latest').

Yes, we should have better foreseen this, especially given that you were
trying to tell us what we should see!

AFAICT the places which needed changing are:
      * mini-os
      * qemu-xen-traditional
      * qemu-xen (upstream)

A patch (untested, but mostly done via sed) for mini-os follows. This
one really ought to have been forseen.

I was surprised that qemu (particularly upstream qemu) is building
against the Xen headers using __XEN_INTERFACE_VERSION__.

I think it is not worth fixing this for qemu-xen-traditional, so I will
follow up with the

> The same would go for tools/blktap*/.

That one never occurred to me, even after I grepped for the symbol.

> So either all uses within tools land get properly fixed (which is going to
> be particularly hard to synchronize with upstream qemu),

IMHO upstream qemu should either include it's own copy of the i/o
interface headers that it uses or it should use a specific
__XEN_INTERFACE_VERSION__. Is this something which is wrong with the
qemu tree or is it the Xen build system integration which is at fault?

I'll followup with a quick fix but IMHO it could be done better.

>  or (my
> preference) the change gets reverted and done backward
> compatibly without involving __XEN_INTERFACE_VERSION__.

Given that it is blocking the test gate I think reverting it should be
strongly considered. As for how best to do this properly I have less of
an opinion.

That mini-os fix:

8<----------------------------------------------

# HG changeset patch
# User Ian Campbell <ian.campbell@xxxxxxxxxx>
# Date 1330335066 0
# Node ID f351fec6d3960d537dde2cb35374982f6ea13e16
# Parent  a4ef50b6313066d953314969494654f4f393558a
mini-os: Fix breakage causes by blkif.h interface change.

24875:a59c1dcfe968 made an incompatible change to the interface headers which
needs reflecting in mini-os.

Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

diff -r a4ef50b63130 -r f351fec6d396 extras/mini-os/blkfront.c
--- a/extras/mini-os/blkfront.c Sat Feb 25 09:36:19 2012 +0000
+++ b/extras/mini-os/blkfront.c Mon Feb 27 09:31:06 2012 +0000
@@ -352,7 +352,7 @@ void blkfront_aio(struct blkfront_aiocb 
 
     /* qemu's IDE max multsect is 16 (8KB) and SCSI max DMA was set to 32KB,
      * so max 44KB can't happen */
-    ASSERT(n <= BLKIF_MAX_SEGMENTS_PER_REQUEST);
+    ASSERT(n <= BLKIF_MAX_SEGMENTS_PER_HEADER_BLOCK);
 
     blkfront_wait_slot(dev);
     i = dev->ring.req_prod_pvt;
diff -r a4ef50b63130 -r f351fec6d396 extras/mini-os/include/blkfront.h
--- a/extras/mini-os/include/blkfront.h Sat Feb 25 09:36:19 2012 +0000
+++ b/extras/mini-os/include/blkfront.h Mon Feb 27 09:31:06 2012 +0000
@@ -12,7 +12,7 @@ struct blkfront_aiocb
     uint8_t is_write;
     void *data;
 
-    grant_ref_t gref[BLKIF_MAX_SEGMENTS_PER_REQUEST];
+    grant_ref_t gref[BLKIF_MAX_SEGMENTS_PER_HEADER_BLOCK];
     int n;
 
     void (*aio_cb)(struct blkfront_aiocb *aiocb, int ret);



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel