This is a patch to forward-port a Xend behaviour. Xend writes IO ABI used for
all frontends. Blkfront before 2.6.26 relies on this behaviour otherwise guest
cannot boot when running in 32-on-64 mode. Blkfront after 2.6.26 writes that
node itself, in which case it''s just an overwrite to an existing node
which
should be OK.
In fact Xend writes the ABI for all frontends including console and vif. But
nowadays only old disk frontends rely on that behaviour so that we only write
the ABI for disk frontends in libxl, minimizing the impact.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Roger Pau Monne <roger.pau@citrix.com>
Cc: Valtteri Kiviniemi <kiviniemi.valtteri@gmail.com>
---
tools/libxc/xc_dom_arm.c | 7 +++++++
tools/libxc/xc_dom_x86.c | 34 ++++++++++++++++++++++++++++++++++
tools/libxc/xenctrl.h | 10 ++++++++++
tools/libxl/libxl.c | 28 ++++++++++++++++++++++++++++
xen/include/public/io/protocols.h | 1 +
5 files changed, 80 insertions(+)
diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
index 041832e..aaf35ca 100644
--- a/tools/libxc/xc_dom_arm.c
+++ b/tools/libxc/xc_dom_arm.c
@@ -29,6 +29,13 @@
#define CONSOLE_PFN_OFFSET 0
#define XENSTORE_PFN_OFFSET 1
+/* get guest IO ABI protocol */
+const char *xc_domain_get_native_protocol(xc_interface *xch,
+ uint32_t domid)
+{
+ return XEN_IO_PROTO_ABI_ARM;
+}
+
/* ------------------------------------------------------------------------ */
/*
* arm guests are hybrid and start off with paging disabled, therefore no
diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index eb9ac07..84a8de6 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -47,6 +47,40 @@
#define round_down(addr, mask) ((addr) & ~(mask))
#define round_up(addr, mask) ((addr) | (mask))
+/* get guest IO ABI protocol */
+const char *xc_domain_get_native_protocol(xc_interface *xch,
+ uint32_t domid)
+{
+ int ret;
+ uint32_t guest_width;
+ const char *protocol;
+ DECLARE_DOMCTL;
+
+ memset(&domctl, 0, sizeof(domctl));
+ domctl.domain = domid;
+ domctl.cmd = XEN_DOMCTL_get_address_size;
+
+ ret = do_domctl(xch, &domctl);
+
+ if ( ret )
+ return NULL;
+
+ guest_width = domctl.u.address_size.size;
+
+ switch (guest_width) {
+ case 32: /* 32 bit guest */
+ protocol = XEN_IO_PROTO_ABI_X86_32;
+ break;
+ case 64: /* 64 bit guest */
+ protocol = XEN_IO_PROTO_ABI_X86_64;
+ break;
+ default:
+ protocol = NULL;
+ }
+
+ return protocol;
+}
+
static unsigned long
nr_page_tables(struct xc_dom_image *dom,
xen_vaddr_t start, xen_vaddr_t end, unsigned long bits)
diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
index 50853af..2f32151 100644
--- a/tools/libxc/xenctrl.h
+++ b/tools/libxc/xenctrl.h
@@ -637,6 +637,16 @@ int xc_domain_hvm_setcontext(xc_interface *xch,
uint32_t size);
/**
+ * This function will return guest IO ABI protocol
+ *
+ * @parm xch a handle to an open hypervisor interface
+ * @parm domid the domain to get IO ABI protocol for
+ * @return guest protocol on success, NULL on failure
+ */
+const char *xc_domain_get_native_protocol(xc_interface *xch,
+ uint32_t domid);
+
+/**
* This function returns information about the execution context of a
* particular vcpu of a domain.
*
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 572c2c6..550c63d 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -18,6 +18,8 @@
#include "libxl_internal.h"
+#include <xen/io/protocols.h>
+
#define PAGE_TO_MEMKB(pages) ((pages) * 4)
#define BACKEND_STRING_SIZE 5
@@ -2037,6 +2039,14 @@ static void device_disk_add(libxl__egc *egc, uint32_t
domid,
int rc;
libxl_ctx *ctx = gc->owner;
xs_transaction_t t = XBT_NULL;
+ const char *protocol = NULL;
+ char p[XEN_IO_PROTO_ABI_MAX_LEN+1];
+
+ libxl_domain_type type = libxl__domain_type(gc, domid);
+ if (type == LIBXL_DOMAIN_TYPE_INVALID) {
+ rc = ERROR_FAIL;
+ goto out;
+ }
for (;;) {
rc = libxl__xs_transaction_start(gc, &t);
@@ -2156,6 +2166,24 @@ static void device_disk_add(libxl__egc *egc, uint32_t
domid,
flexarray_append(front, "device-type");
flexarray_append(front, disk->is_cdrom ? "cdrom" :
"disk");
+ /*
+ * Old PV kernel disk frontends before 2.6.26 rely on tool stack to
+ * write disk native protocol to frontend node. Xend does this, port
+ * this behaviour to xl.
+ *
+ * New kernels write this node themselves. In that case it just
+ * overwrites an existing node which is OK.
+ */
+ if (type == LIBXL_DOMAIN_TYPE_PV) {
+ protocol = xc_domain_get_native_protocol(ctx->xch, domid);
+ if (protocol) {
+ strncpy(p, protocol, strlen(protocol));
+ p[strlen(protocol)] = 0;
+ flexarray_append(front, "protocol");
+ flexarray_append(front, p);
+ }
+ }
+
libxl__device_generic_add(gc, t, device,
libxl__xs_kvs_of_flexarray(gc, back,
back->count),
libxl__xs_kvs_of_flexarray(gc, front,
front->count));
diff --git a/xen/include/public/io/protocols.h
b/xen/include/public/io/protocols.h
index 80b196b..c2b949e 100644
--- a/xen/include/public/io/protocols.h
+++ b/xen/include/public/io/protocols.h
@@ -26,6 +26,7 @@
#define XEN_IO_PROTO_ABI_X86_32 "x86_32-abi"
#define XEN_IO_PROTO_ABI_X86_64 "x86_64-abi"
#define XEN_IO_PROTO_ABI_ARM "arm-abi"
+#define XEN_IO_PROTO_ABI_MAX_LEN 10 /* maximum length of above strings */
#if defined(__i386__)
# define XEN_IO_PROTO_ABI_NATIVE XEN_IO_PROTO_ABI_X86_32
--
1.7.10.4
On Thu, 2013-04-25 at 22:36 +0100, Wei Liu wrote:> @@ -2156,6 +2166,24 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid, > flexarray_append(front, "device-type"); > flexarray_append(front, disk->is_cdrom ? "cdrom" : "disk"); > > + /* > + * Old PV kernel disk frontends before 2.6.26 rely on tool stack to > + * write disk native protocol to frontend node. Xend does this, port > + * this behaviour to xl. > + * > + * New kernels write this node themselves. In that case it just > + * overwrites an existing node which is OK. > + */ > + if (type == LIBXL_DOMAIN_TYPE_PV) { > + protocol = xc_domain_get_native_protocol(ctx->xch, domid);You can declare protocol here instead of globally in the function. The NULL is also redundant.> + if (protocol) { > + strncpy(p, protocol, strlen(protocol)); > + p[strlen(protocol)] = 0; > + flexarray_append(front, "protocol"); > + flexarray_append(front, p);GCSPRINTF would be the right way to do this, but actually you can pass protocol directly to flexarray_append, that is fine since the flexarray never frees the values which it contains so they either need to be gc''d, string constants or managed manually. Ian.
On Fri, Apr 26, 2013 at 10:18:51AM +0100, Ian Campbell wrote:> On Thu, 2013-04-25 at 22:36 +0100, Wei Liu wrote: > > @@ -2156,6 +2166,24 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid, > > flexarray_append(front, "device-type"); > > flexarray_append(front, disk->is_cdrom ? "cdrom" : "disk"); > > > > + /* > > + * Old PV kernel disk frontends before 2.6.26 rely on tool stack to > > + * write disk native protocol to frontend node. Xend does this, port > > + * this behaviour to xl. > > + * > > + * New kernels write this node themselves. In that case it just > > + * overwrites an existing node which is OK. > > + */ > > + if (type == LIBXL_DOMAIN_TYPE_PV) { > > + protocol = xc_domain_get_native_protocol(ctx->xch, domid); > > You can declare protocol here instead of globally in the function. The > NULL is also redundant. > > > + if (protocol) { > > + strncpy(p, protocol, strlen(protocol)); > > + p[strlen(protocol)] = 0; > > + flexarray_append(front, "protocol"); > > + flexarray_append(front, p); > > GCSPRINTF would be the right way to do this, but actually you can pass > protocol directly to flexarray_append, that is fine since the flexarray > never frees the values which it contains so they either need to be gc''d, > string constants or managed manually. >Oh thanks for the hint. This saves serveral lines of code. The (void *) argument of flexarray_append misled me. ;-) Below is updated version of the patch. Casting "protocol" to (void *) is necessary to have it compiled. Otherwise gcc complains we discard the const quilifier. Wei. -----------8<-------- commit 7961cfe75b1282a2d27163a04ff268cedf4919b5 Author: Wei Liu <wei.liu2@citrix.com> Date: Wed Apr 24 19:00:01 2013 +0100 libxl: write IO ABI for disk frontends This is a patch to forward-port a Xend behaviour. Xend writes IO ABI used for all frontends. Blkfront before 2.6.26 relies on this behaviour otherwise guest cannot boot when running in 32-on-64 mode. Blkfront after 2.6.26 writes that node itself, in which case it''s just an overwrite to an existing node which should be OK. In fact Xend writes the ABI for all frontends including console and vif. But nowadays only old disk frontends rely on that behaviour so that we only write the ABI for disk frontends in libxl, minimizing the impact. Signed-off-by: Wei Liu <wei.liu2@citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com> Cc: Roger Pau Monne <roger.pau@citrix.com> Cc: Valtteri Kiviniemi <kiviniemi.valtteri@gmail.com> diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c index 041832e..aaf35ca 100644 --- a/tools/libxc/xc_dom_arm.c +++ b/tools/libxc/xc_dom_arm.c @@ -29,6 +29,13 @@ #define CONSOLE_PFN_OFFSET 0 #define XENSTORE_PFN_OFFSET 1 +/* get guest IO ABI protocol */ +const char *xc_domain_get_native_protocol(xc_interface *xch, + uint32_t domid) +{ + return XEN_IO_PROTO_ABI_ARM; +} + /* ------------------------------------------------------------------------ */ /* * arm guests are hybrid and start off with paging disabled, therefore no diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c index eb9ac07..84a8de6 100644 --- a/tools/libxc/xc_dom_x86.c +++ b/tools/libxc/xc_dom_x86.c @@ -47,6 +47,40 @@ #define round_down(addr, mask) ((addr) & ~(mask)) #define round_up(addr, mask) ((addr) | (mask)) +/* get guest IO ABI protocol */ +const char *xc_domain_get_native_protocol(xc_interface *xch, + uint32_t domid) +{ + int ret; + uint32_t guest_width; + const char *protocol; + DECLARE_DOMCTL; + + memset(&domctl, 0, sizeof(domctl)); + domctl.domain = domid; + domctl.cmd = XEN_DOMCTL_get_address_size; + + ret = do_domctl(xch, &domctl); + + if ( ret ) + return NULL; + + guest_width = domctl.u.address_size.size; + + switch (guest_width) { + case 32: /* 32 bit guest */ + protocol = XEN_IO_PROTO_ABI_X86_32; + break; + case 64: /* 64 bit guest */ + protocol = XEN_IO_PROTO_ABI_X86_64; + break; + default: + protocol = NULL; + } + + return protocol; +} + static unsigned long nr_page_tables(struct xc_dom_image *dom, xen_vaddr_t start, xen_vaddr_t end, unsigned long bits) diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h index 50853af..2f32151 100644 --- a/tools/libxc/xenctrl.h +++ b/tools/libxc/xenctrl.h @@ -637,6 +637,16 @@ int xc_domain_hvm_setcontext(xc_interface *xch, uint32_t size); /** + * This function will return guest IO ABI protocol + * + * @parm xch a handle to an open hypervisor interface + * @parm domid the domain to get IO ABI protocol for + * @return guest protocol on success, NULL on failure + */ +const char *xc_domain_get_native_protocol(xc_interface *xch, + uint32_t domid); + +/** * This function returns information about the execution context of a * particular vcpu of a domain. * diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index 572c2c6..9013932 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -2038,6 +2038,12 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid, libxl_ctx *ctx = gc->owner; xs_transaction_t t = XBT_NULL; + libxl_domain_type type = libxl__domain_type(gc, domid); + if (type == LIBXL_DOMAIN_TYPE_INVALID) { + rc = ERROR_FAIL; + goto out; + } + for (;;) { rc = libxl__xs_transaction_start(gc, &t); if (rc) goto out; @@ -2156,6 +2162,23 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid, flexarray_append(front, "device-type"); flexarray_append(front, disk->is_cdrom ? "cdrom" : "disk"); + /* + * Old PV kernel disk frontends before 2.6.26 rely on tool stack to + * write disk native protocol to frontend node. Xend does this, port + * this behaviour to xl. + * + * New kernels write this node themselves. In that case it just + * overwrites an existing node which is OK. + */ + if (type == LIBXL_DOMAIN_TYPE_PV) { + const char *protocol + xc_domain_get_native_protocol(ctx->xch, domid); + if (protocol) { + flexarray_append(front, "protocol"); + flexarray_append(front, (void *)protocol); + } + } + libxl__device_generic_add(gc, t, device, libxl__xs_kvs_of_flexarray(gc, back, back->count), libxl__xs_kvs_of_flexarray(gc, front, front->count));
On Fri, 2013-04-26 at 10:44 +0100, Wei Liu wrote:> On Fri, Apr 26, 2013 at 10:18:51AM +0100, Ian Campbell wrote: > > On Thu, 2013-04-25 at 22:36 +0100, Wei Liu wrote: > > > @@ -2156,6 +2166,24 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid, > > > flexarray_append(front, "device-type"); > > > flexarray_append(front, disk->is_cdrom ? "cdrom" : "disk"); > > > > > > + /* > > > + * Old PV kernel disk frontends before 2.6.26 rely on tool stack to > > > + * write disk native protocol to frontend node. Xend does this, port > > > + * this behaviour to xl. > > > + * > > > + * New kernels write this node themselves. In that case it just > > > + * overwrites an existing node which is OK. > > > + */ > > > + if (type == LIBXL_DOMAIN_TYPE_PV) { > > > + protocol = xc_domain_get_native_protocol(ctx->xch, domid); > > > > You can declare protocol here instead of globally in the function. The > > NULL is also redundant. > > > > > + if (protocol) { > > > + strncpy(p, protocol, strlen(protocol)); > > > + p[strlen(protocol)] = 0; > > > + flexarray_append(front, "protocol"); > > > + flexarray_append(front, p); > > > > GCSPRINTF would be the right way to do this, but actually you can pass > > protocol directly to flexarray_append, that is fine since the flexarray > > never frees the values which it contains so they either need to be gc''d, > > string constants or managed manually. > > > > Oh thanks for the hint. This saves serveral lines of code. The (void *) > argument of flexarray_append misled me. ;-) > > Below is updated version of the patch. Casting "protocol" to (void *) is > necessary to have it compiled. Otherwise gcc complains we discard the > const quilifier.Oh, sorry, if this is the case the libxl__strdup(gc,protocol) is better than the cast. Sorry for misleading :-( Now you mention it I do seem to recall trying to correctly constify flexarray''s at one point and not quite being able to make it work. Ian.> > > Wei. > > -----------8<-------- > commit 7961cfe75b1282a2d27163a04ff268cedf4919b5 > Author: Wei Liu <wei.liu2@citrix.com> > Date: Wed Apr 24 19:00:01 2013 +0100 > > libxl: write IO ABI for disk frontends > > This is a patch to forward-port a Xend behaviour. Xend writes IO ABI used for > all frontends. Blkfront before 2.6.26 relies on this behaviour otherwise guest > cannot boot when running in 32-on-64 mode. Blkfront after 2.6.26 writes that > node itself, in which case it''s just an overwrite to an existing node which > should be OK. > > In fact Xend writes the ABI for all frontends including console and vif. But > nowadays only old disk frontends rely on that behaviour so that we only write > the ABI for disk frontends in libxl, minimizing the impact. > > Signed-off-by: Wei Liu <wei.liu2@citrix.com> > Cc: Ian Campbell <ian.campbell@citrix.com> > Cc: Roger Pau Monne <roger.pau@citrix.com> > Cc: Valtteri Kiviniemi <kiviniemi.valtteri@gmail.com> > > diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c > index 041832e..aaf35ca 100644 > --- a/tools/libxc/xc_dom_arm.c > +++ b/tools/libxc/xc_dom_arm.c > @@ -29,6 +29,13 @@ > #define CONSOLE_PFN_OFFSET 0 > #define XENSTORE_PFN_OFFSET 1 > > +/* get guest IO ABI protocol */ > +const char *xc_domain_get_native_protocol(xc_interface *xch, > + uint32_t domid) > +{ > + return XEN_IO_PROTO_ABI_ARM; > +} > + > /* ------------------------------------------------------------------------ */ > /* > * arm guests are hybrid and start off with paging disabled, therefore no > diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c > index eb9ac07..84a8de6 100644 > --- a/tools/libxc/xc_dom_x86.c > +++ b/tools/libxc/xc_dom_x86.c > @@ -47,6 +47,40 @@ > #define round_down(addr, mask) ((addr) & ~(mask)) > #define round_up(addr, mask) ((addr) | (mask)) > > +/* get guest IO ABI protocol */ > +const char *xc_domain_get_native_protocol(xc_interface *xch, > + uint32_t domid) > +{ > + int ret; > + uint32_t guest_width; > + const char *protocol; > + DECLARE_DOMCTL; > + > + memset(&domctl, 0, sizeof(domctl)); > + domctl.domain = domid; > + domctl.cmd = XEN_DOMCTL_get_address_size; > + > + ret = do_domctl(xch, &domctl); > + > + if ( ret ) > + return NULL; > + > + guest_width = domctl.u.address_size.size; > + > + switch (guest_width) { > + case 32: /* 32 bit guest */ > + protocol = XEN_IO_PROTO_ABI_X86_32; > + break; > + case 64: /* 64 bit guest */ > + protocol = XEN_IO_PROTO_ABI_X86_64; > + break; > + default: > + protocol = NULL; > + } > + > + return protocol; > +} > + > static unsigned long > nr_page_tables(struct xc_dom_image *dom, > xen_vaddr_t start, xen_vaddr_t end, unsigned long bits) > diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h > index 50853af..2f32151 100644 > --- a/tools/libxc/xenctrl.h > +++ b/tools/libxc/xenctrl.h > @@ -637,6 +637,16 @@ int xc_domain_hvm_setcontext(xc_interface *xch, > uint32_t size); > > /** > + * This function will return guest IO ABI protocol > + * > + * @parm xch a handle to an open hypervisor interface > + * @parm domid the domain to get IO ABI protocol for > + * @return guest protocol on success, NULL on failure > + */ > +const char *xc_domain_get_native_protocol(xc_interface *xch, > + uint32_t domid); > + > +/** > * This function returns information about the execution context of a > * particular vcpu of a domain. > * > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c > index 572c2c6..9013932 100644 > --- a/tools/libxl/libxl.c > +++ b/tools/libxl/libxl.c > @@ -2038,6 +2038,12 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid, > libxl_ctx *ctx = gc->owner; > xs_transaction_t t = XBT_NULL; > > + libxl_domain_type type = libxl__domain_type(gc, domid); > + if (type == LIBXL_DOMAIN_TYPE_INVALID) { > + rc = ERROR_FAIL; > + goto out; > + } > + > for (;;) { > rc = libxl__xs_transaction_start(gc, &t); > if (rc) goto out; > @@ -2156,6 +2162,23 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid, > flexarray_append(front, "device-type"); > flexarray_append(front, disk->is_cdrom ? "cdrom" : "disk"); > > + /* > + * Old PV kernel disk frontends before 2.6.26 rely on tool stack to > + * write disk native protocol to frontend node. Xend does this, port > + * this behaviour to xl. > + * > + * New kernels write this node themselves. In that case it just > + * overwrites an existing node which is OK. > + */ > + if (type == LIBXL_DOMAIN_TYPE_PV) { > + const char *protocol > + xc_domain_get_native_protocol(ctx->xch, domid); > + if (protocol) { > + flexarray_append(front, "protocol"); > + flexarray_append(front, (void *)protocol); > + } > + } > + > libxl__device_generic_add(gc, t, device, > libxl__xs_kvs_of_flexarray(gc, back, back->count), > libxl__xs_kvs_of_flexarray(gc, front, front->count));
On Fri, Apr 26, 2013 at 10:52:35AM +0100, Ian Campbell wrote: [...]> > > > + flexarray_append(front, p); > > > > > > GCSPRINTF would be the right way to do this, but actually you can pass > > > protocol directly to flexarray_append, that is fine since the flexarray > > > never frees the values which it contains so they either need to be gc''d, > > > string constants or managed manually. > > > > > > > Oh thanks for the hint. This saves serveral lines of code. The (void *) > > argument of flexarray_append misled me. ;-) > > > > Below is updated version of the patch. Casting "protocol" to (void *) is > > necessary to have it compiled. Otherwise gcc complains we discard the > > const quilifier. > > Oh, sorry, if this is the case the libxl__strdup(gc,protocol) is better > than the cast. Sorry for misleading :-( > > Now you mention it I do seem to recall trying to correctly constify > flexarray''s at one point and not quite being able to make it work. >Another version with libxl__strdup(). :-) Wei. -------8<-------- commit 60bfc42e4e4f5e0f7f025e4e67b32c61fef396d4 Author: Wei Liu <wei.liu2@citrix.com> Date: Wed Apr 24 19:00:01 2013 +0100 libxl: write IO ABI for disk frontends This is a patch to forward-port a Xend behaviour. Xend writes IO ABI used for all frontends. Blkfront before 2.6.26 relies on this behaviour otherwise guest cannot boot when running in 32-on-64 mode. Blkfront after 2.6.26 writes that node itself, in which case it''s just an overwrite to an existing node which should be OK. In fact Xend writes the ABI for all frontends including console and vif. But nowadays only old disk frontends rely on that behaviour so that we only write the ABI for disk frontends in libxl, minimizing the impact. Signed-off-by: Wei Liu <wei.liu2@citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com> Cc: Roger Pau Monne <roger.pau@citrix.com> Cc: Valtteri Kiviniemi <kiviniemi.valtteri@gmail.com> diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c index 041832e..aaf35ca 100644 --- a/tools/libxc/xc_dom_arm.c +++ b/tools/libxc/xc_dom_arm.c @@ -29,6 +29,13 @@ #define CONSOLE_PFN_OFFSET 0 #define XENSTORE_PFN_OFFSET 1 +/* get guest IO ABI protocol */ +const char *xc_domain_get_native_protocol(xc_interface *xch, + uint32_t domid) +{ + return XEN_IO_PROTO_ABI_ARM; +} + /* ------------------------------------------------------------------------ */ /* * arm guests are hybrid and start off with paging disabled, therefore no diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c index eb9ac07..84a8de6 100644 --- a/tools/libxc/xc_dom_x86.c +++ b/tools/libxc/xc_dom_x86.c @@ -47,6 +47,40 @@ #define round_down(addr, mask) ((addr) & ~(mask)) #define round_up(addr, mask) ((addr) | (mask)) +/* get guest IO ABI protocol */ +const char *xc_domain_get_native_protocol(xc_interface *xch, + uint32_t domid) +{ + int ret; + uint32_t guest_width; + const char *protocol; + DECLARE_DOMCTL; + + memset(&domctl, 0, sizeof(domctl)); + domctl.domain = domid; + domctl.cmd = XEN_DOMCTL_get_address_size; + + ret = do_domctl(xch, &domctl); + + if ( ret ) + return NULL; + + guest_width = domctl.u.address_size.size; + + switch (guest_width) { + case 32: /* 32 bit guest */ + protocol = XEN_IO_PROTO_ABI_X86_32; + break; + case 64: /* 64 bit guest */ + protocol = XEN_IO_PROTO_ABI_X86_64; + break; + default: + protocol = NULL; + } + + return protocol; +} + static unsigned long nr_page_tables(struct xc_dom_image *dom, xen_vaddr_t start, xen_vaddr_t end, unsigned long bits) diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h index 50853af..2f32151 100644 --- a/tools/libxc/xenctrl.h +++ b/tools/libxc/xenctrl.h @@ -637,6 +637,16 @@ int xc_domain_hvm_setcontext(xc_interface *xch, uint32_t size); /** + * This function will return guest IO ABI protocol + * + * @parm xch a handle to an open hypervisor interface + * @parm domid the domain to get IO ABI protocol for + * @return guest protocol on success, NULL on failure + */ +const char *xc_domain_get_native_protocol(xc_interface *xch, + uint32_t domid); + +/** * This function returns information about the execution context of a * particular vcpu of a domain. * diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index 572c2c6..d371b88 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -2038,6 +2038,12 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid, libxl_ctx *ctx = gc->owner; xs_transaction_t t = XBT_NULL; + libxl_domain_type type = libxl__domain_type(gc, domid); + if (type == LIBXL_DOMAIN_TYPE_INVALID) { + rc = ERROR_FAIL; + goto out; + } + for (;;) { rc = libxl__xs_transaction_start(gc, &t); if (rc) goto out; @@ -2156,6 +2162,23 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid, flexarray_append(front, "device-type"); flexarray_append(front, disk->is_cdrom ? "cdrom" : "disk"); + /* + * Old PV kernel disk frontends before 2.6.26 rely on tool stack to + * write disk native protocol to frontend node. Xend does this, port + * this behaviour to xl. + * + * New kernels write this node themselves. In that case it just + * overwrites an existing node which is OK. + */ + if (type == LIBXL_DOMAIN_TYPE_PV) { + const char *protocol + xc_domain_get_native_protocol(ctx->xch, domid); + if (protocol) { + flexarray_append(front, "protocol"); + flexarray_append(front, libxl__strdup(gc, protocol)); + } + } + libxl__device_generic_add(gc, t, device, libxl__xs_kvs_of_flexarray(gc, back, back->count), libxl__xs_kvs_of_flexarray(gc, front, front->count));
On Fri, 2013-04-26 at 11:11 +0100, Wei Liu wrote:> On Fri, Apr 26, 2013 at 10:52:35AM +0100, Ian Campbell wrote: > [...] > > > > > + flexarray_append(front, p); > > > > > > > > GCSPRINTF would be the right way to do this, but actually you can pass > > > > protocol directly to flexarray_append, that is fine since the flexarray > > > > never frees the values which it contains so they either need to be gc''d, > > > > string constants or managed manually. > > > > > > > > > > Oh thanks for the hint. This saves serveral lines of code. The (void *) > > > argument of flexarray_append misled me. ;-) > > > > > > Below is updated version of the patch. Casting "protocol" to (void *) is > > > necessary to have it compiled. Otherwise gcc complains we discard the > > > const quilifier. > > > > Oh, sorry, if this is the case the libxl__strdup(gc,protocol) is better > > than the cast. Sorry for misleading :-( > > > > Now you mention it I do seem to recall trying to correctly constify > > flexarray''s at one point and not quite being able to make it work. > > > > Another version with libxl__strdup(). :-)Thanks! George, this is on your tracking list and is a bug fix and/or a regression from Xend. Are you happy to give it a freeze exception?> -------8<-------- > commit 60bfc42e4e4f5e0f7f025e4e67b32c61fef396d4 > Author: Wei Liu <wei.liu2@citrix.com> > Date: Wed Apr 24 19:00:01 2013 +0100 > > libxl: write IO ABI for disk frontends > > This is a patch to forward-port a Xend behaviour. Xend writes IO ABI used for > all frontends. Blkfront before 2.6.26 relies on this behaviour otherwise guest > cannot boot when running in 32-on-64 mode. Blkfront after 2.6.26 writes that > node itself, in which case it''s just an overwrite to an existing node which > should be OK. > > In fact Xend writes the ABI for all frontends including console and vif. But > nowadays only old disk frontends rely on that behaviour so that we only write > the ABI for disk frontends in libxl, minimizing the impact. > > Signed-off-by: Wei Liu <wei.liu2@citrix.com> > Cc: Ian Campbell <ian.campbell@citrix.com>Acked-by: Ian Campbell <ian.campbell@citrix.com> Ian.
On 26/04/13 11:22, Ian Campbell wrote:> On Fri, 2013-04-26 at 11:11 +0100, Wei Liu wrote: >> On Fri, Apr 26, 2013 at 10:52:35AM +0100, Ian Campbell wrote: >> [...] >>>>>> + flexarray_append(front, p); >>>>> GCSPRINTF would be the right way to do this, but actually you can pass >>>>> protocol directly to flexarray_append, that is fine since the flexarray >>>>> never frees the values which it contains so they either need to be gc''d, >>>>> string constants or managed manually. >>>>> >>>> Oh thanks for the hint. This saves serveral lines of code. The (void *) >>>> argument of flexarray_append misled me. ;-) >>>> >>>> Below is updated version of the patch. Casting "protocol" to (void *) is >>>> necessary to have it compiled. Otherwise gcc complains we discard the >>>> const quilifier. >>> Oh, sorry, if this is the case the libxl__strdup(gc,protocol) is better >>> than the cast. Sorry for misleading :-( >>> >>> Now you mention it I do seem to recall trying to correctly constify >>> flexarray''s at one point and not quite being able to make it work. >>> >> Another version with libxl__strdup(). :-) > Thanks! > > George, this is on your tracking list and is a bug fix and/or a > regression from Xend. Are you happy to give it a freeze exception?Yes, I think this is fine to go in. -George
> > George, this is on your tracking list and is a bug fix and/or a > > regression from Xend. Are you happy to give it a freeze exception? > > Yes, I think this is fine to go in.Applied. Thanks
Valtteri Kiviniemi
2013-May-16 12:43 UTC
Re: [PATCH V3] libxl: write IO ABI for disk frontends
Hi, I will test this patch now. Is this patch already included in xen-unstable? - Valtteri 2013/4/26 Ian Campbell <Ian.Campbell@citrix.com>> > > > George, this is on your tracking list and is a bug fix and/or a > > > regression from Xend. Are you happy to give it a freeze exception? > > > > Yes, I think this is fine to go in. > > Applied. Thanks > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
On Thu, May 16, 2013 at 03:43:02PM +0300, Valtteri Kiviniemi wrote:> Hi, > > I will test this patch now. Is this patch already included in xen-unstable? >Yes, it''s in xen-4.3-rc1 now. I would encourage you help testing the up coming Xen-4.3-rcX. Wei.
Valtteri Kiviniemi
2013-May-16 14:01 UTC
Re: [PATCH V3] libxl: write IO ABI for disk frontends
2013/5/16 Wei Liu <wei.liu2@citrix.com>> On Thu, May 16, 2013 at 03:43:02PM +0300, Valtteri Kiviniemi wrote: > > Hi, > > > > I will test this patch now. Is this patch already included in > xen-unstable? > > > > Yes, it''s in xen-4.3-rc1 now. I would encourage you help testing the > up coming Xen-4.3-rcX. > > > Wei. >Hi, Ok, thanks. I''m building 4.3.0-rc1 now on my test server and I will post results within 24 hours. - Valtteri _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Valtteri Kiviniemi
2013-May-17 07:36 UTC
Re: [PATCH V3] libxl: write IO ABI for disk frontends
2013/5/16 Valtteri Kiviniemi <kiviniemi.valtteri@gmail.com>> 2013/5/16 Wei Liu <wei.liu2@citrix.com> > >> On Thu, May 16, 2013 at 03:43:02PM +0300, Valtteri Kiviniemi wrote: >> > Hi, >> > >> > I will test this patch now. Is this patch already included in >> xen-unstable? >> > >> >> Yes, it''s in xen-4.3-rc1 now. I would encourage you help testing the >> up coming Xen-4.3-rcX. >> >> >> Wei. >> > > Hi, > > Ok, thanks. I''m building 4.3.0-rc1 now on my test server and I will post > results within 24 hours. > > - Valtteri >Hi, My 32bit debian etch with kernel 2.6.16.33-xen is now booting successfully with Xen 4.3-rc1 and with xl tool stack. So the patch is working as intented. I also tested various xl commands and functions with my legacy domU and all of them seemed to be working (reboot, pause, unpause, shutdown etc.) except suspend. Suspend crashes with "Internal error" and the guest console shows error message "Can''t suspend SMP guests without CONFIG_HOTPLUG_CPU". 2.6.16.33 kernel does not seem to have cpu hotplug support, or then I just can find it in the kernel menuconfig. - Valtteri _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
On Fri, May 17, 2013 at 10:36:34AM +0300, Valtteri Kiviniemi wrote:> 2013/5/16 Valtteri Kiviniemi <kiviniemi.valtteri@gmail.com> > > > 2013/5/16 Wei Liu <wei.liu2@citrix.com> > > > >> On Thu, May 16, 2013 at 03:43:02PM +0300, Valtteri Kiviniemi wrote: > >> > Hi, > >> > > >> > I will test this patch now. Is this patch already included in > >> xen-unstable? > >> > > >> > >> Yes, it''s in xen-4.3-rc1 now. I would encourage you help testing the > >> up coming Xen-4.3-rcX. > >> > >> > >> Wei. > >> > > > > Hi, > > > > Ok, thanks. I''m building 4.3.0-rc1 now on my test server and I will post > > results within 24 hours. > > > > - Valtteri > > > > Hi, > > My 32bit debian etch with kernel 2.6.16.33-xen is now booting successfully > with Xen 4.3-rc1 and with xl tool stack. So the patch is working as > intented. I also tested various xl commands and functions with my legacy > domU and all of them seemed to be working (reboot, pause, unpause, shutdownThanks for testing.> etc.) except suspend. Suspend crashes with "Internal error" and the guest > console shows error message "Can''t suspend SMP guests without > CONFIG_HOTPLUG_CPU". > > 2.6.16.33 kernel does not seem to have cpu hotplug support, or then I just > can find it in the kernel menuconfig. >George, should this one be added to your list? (Just found out you''re already in CC ;-) ) Wei.
On 05/17/2013 09:33 AM, Wei Liu wrote:> On Fri, May 17, 2013 at 10:36:34AM +0300, Valtteri Kiviniemi wrote: >> 2013/5/16 Valtteri Kiviniemi <kiviniemi.valtteri@gmail.com> >> >>> 2013/5/16 Wei Liu <wei.liu2@citrix.com> >>> >>>> On Thu, May 16, 2013 at 03:43:02PM +0300, Valtteri Kiviniemi wrote: >>>>> Hi, >>>>> >>>>> I will test this patch now. Is this patch already included in >>>> xen-unstable? >>>>> >>>> >>>> Yes, it''s in xen-4.3-rc1 now. I would encourage you help testing the >>>> up coming Xen-4.3-rcX. >>>> >>>> >>>> Wei. >>>> >>> >>> Hi, >>> >>> Ok, thanks. I''m building 4.3.0-rc1 now on my test server and I will post >>> results within 24 hours. >>> >>> - Valtteri >>> >> >> Hi, >> >> My 32bit debian etch with kernel 2.6.16.33-xen is now booting successfully >> with Xen 4.3-rc1 and with xl tool stack. So the patch is working as >> intented. I also tested various xl commands and functions with my legacy >> domU and all of them seemed to be working (reboot, pause, unpause, shutdown > > Thanks for testing. > >> etc.) except suspend. Suspend crashes with "Internal error" and the guest >> console shows error message "Can''t suspend SMP guests without >> CONFIG_HOTPLUG_CPU". >> >> 2.6.16.33 kernel does not seem to have cpu hotplug support, or then I just >> can find it in the kernel menuconfig. >> > > George, should this one be added to your list? (Just found out you''re > already in CC ;-) )...add "Linux 2.6.16.33 HOTPLUG config option is hard to find" to my list? I''m not sure that would be a 4.3 release blocker... :-) -George
On Fri, May 17, 2013 at 09:56:38AM +0100, George Dunlap wrote: [...]> >>with Xen 4.3-rc1 and with xl tool stack. So the patch is working as > >>intented. I also tested various xl commands and functions with my legacy > >>domU and all of them seemed to be working (reboot, pause, unpause, shutdown > > > >Thanks for testing. > > > >>etc.) except suspend. Suspend crashes with "Internal error" and the guest > >>console shows error message "Can''t suspend SMP guests without > >>CONFIG_HOTPLUG_CPU". > >> > >>2.6.16.33 kernel does not seem to have cpu hotplug support, or then I just > >>can find it in the kernel menuconfig. > >> > > > >George, should this one be added to your list? (Just found out you''re > >already in CC ;-) ) > > ...add "Linux 2.6.16.33 HOTPLUG config option is hard to find" to my > list? I''m not sure that would be a 4.3 release blocker... :-) >I mean ''xl suspend'' crashes with "Internal Error". Should not be a blocker, but at least worthy of making its way to known issues list? (if there is one). Wei.> -George
On 05/17/2013 10:07 AM, Wei Liu wrote:> On Fri, May 17, 2013 at 09:56:38AM +0100, George Dunlap wrote: > [...] >>>> with Xen 4.3-rc1 and with xl tool stack. So the patch is working as >>>> intented. I also tested various xl commands and functions with my legacy >>>> domU and all of them seemed to be working (reboot, pause, unpause, shutdown >>> >>> Thanks for testing. >>> >>>> etc.) except suspend. Suspend crashes with "Internal error" and the guest >>>> console shows error message "Can''t suspend SMP guests without >>>> CONFIG_HOTPLUG_CPU". >>>> >>>> 2.6.16.33 kernel does not seem to have cpu hotplug support, or then I just >>>> can find it in the kernel menuconfig. >>>> >>> >>> George, should this one be added to your list? (Just found out you''re >>> already in CC ;-) ) >> >> ...add "Linux 2.6.16.33 HOTPLUG config option is hard to find" to my >> list? I''m not sure that would be a 4.3 release blocker... :-) >> > > I mean ''xl suspend'' crashes with "Internal Error". Should not be a > blocker, but at least worthy of making its way to known issues list? (if > there is one).Well if it *fails* with "Internal Error" because there''s an internal error, I''m not sure that''s a bug. :-) If Valtteri wants to post a proper bug report, with the listed output and a suggestion of what would be a better output, we can consider putting it on the list. -George
On Fri, May 17, 2013 at 10:18:05AM +0100, George Dunlap wrote:> On 05/17/2013 10:07 AM, Wei Liu wrote: > >On Fri, May 17, 2013 at 09:56:38AM +0100, George Dunlap wrote: > >[...] > >>>>with Xen 4.3-rc1 and with xl tool stack. So the patch is working as > >>>>intented. I also tested various xl commands and functions with my legacy > >>>>domU and all of them seemed to be working (reboot, pause, unpause, shutdown > >>> > >>>Thanks for testing. > >>> > >>>>etc.) except suspend. Suspend crashes with "Internal error" and the guest > >>>>console shows error message "Can''t suspend SMP guests without > >>>>CONFIG_HOTPLUG_CPU". > >>>> > >>>>2.6.16.33 kernel does not seem to have cpu hotplug support, or then I just > >>>>can find it in the kernel menuconfig. > >>>> > >>> > >>>George, should this one be added to your list? (Just found out you''re > >>>already in CC ;-) ) > >> > >>...add "Linux 2.6.16.33 HOTPLUG config option is hard to find" to my > >>list? I''m not sure that would be a 4.3 release blocker... :-) > >> > > > >I mean ''xl suspend'' crashes with "Internal Error". Should not be a > >blocker, but at least worthy of making its way to known issues list? (if > >there is one). > > Well if it *fails* with "Internal Error" because there''s an internal > error, I''m not sure that''s a bug. :-) >The "crash" bugs me. If it is a normal exit that''s fine.> If Valtteri wants to post a proper bug report, with the listed > output and a suggestion of what would be a better output, we can > consider putting it on the list. >Valtteri now it is up to you. :-) Wei.
Valtteri Kiviniemi
2013-May-17 10:53 UTC
Re: [PATCH V3] libxl: write IO ABI for disk frontends
2013/5/17 Wei Liu <wei.liu2@citrix.com>> On Fri, May 17, 2013 at 10:18:05AM +0100, George Dunlap wrote: > > On 05/17/2013 10:07 AM, Wei Liu wrote: > > >On Fri, May 17, 2013 at 09:56:38AM +0100, George Dunlap wrote: > > >[...] > > >>>>with Xen 4.3-rc1 and with xl tool stack. So the patch is working as > > >>>>intented. I also tested various xl commands and functions with my > legacy > > >>>>domU and all of them seemed to be working (reboot, pause, unpause, > shutdown > > >>> > > >>>Thanks for testing. > > >>> > > >>>>etc.) except suspend. Suspend crashes with "Internal error" and the > guest > > >>>>console shows error message "Can''t suspend SMP guests without > > >>>>CONFIG_HOTPLUG_CPU". > > >>>> > > >>>>2.6.16.33 kernel does not seem to have cpu hotplug support, or then > I just > > >>>>can find it in the kernel menuconfig. > > >>>> > > >>> > > >>>George, should this one be added to your list? (Just found out you''re > > >>>already in CC ;-) ) > > >> > > >>...add "Linux 2.6.16.33 HOTPLUG config option is hard to find" to my > > >>list? I''m not sure that would be a 4.3 release blocker... :-) > > >> > > > > > >I mean ''xl suspend'' crashes with "Internal Error". Should not be a > > >blocker, but at least worthy of making its way to known issues list? (if > > >there is one). > > > > Well if it *fails* with "Internal Error" because there''s an internal > > error, I''m not sure that''s a bug. :-) > > > > The "crash" bugs me. If it is a normal exit that''s fine. > > > If Valtteri wants to post a proper bug report, with the listed > > output and a suggestion of what would be a better output, we can > > consider putting it on the list. > > > > Valtteri now it is up to you. :-) > > > Wei. >Hi, I''m now at holiday couple of weeks but after that I will double check my kernel config and post a proper bug report if needed. I just had this feeling that the pre 4.0 xen versions (for example xen 3.4 with 2.6.18 xenified kernel) handled the suspend differently and figured that maybe there is still something missing from xl toolstack. That 2.6.16 kernel is so old that I just assumed that it does not have cpu hotplug support and figured that maybe it is something that xl toolstack does not handle. But you are probably right, I''m just blind and missed the option in the menuconfig :) Its hard to remember all the changes that has happened in Xen and Linux kernels and which options need to be enabled etc. But I will test it more thoroughly after my holiday. - Valteri _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Maybe Matching Threads
- [PATCH v2 00/14] xen: arm: 64-bit guest support and domU FDT autogeneration
- [PATCH v6 00/16] xen: arm: 64-bit guest support and domU FDT autogeneration
- (XEN) traps.c:3156: GPF messages in xen dmesg
- Xen 4.0.4, kernel 3.5.0 HVM crash and kernel BUG
- [PATCH RFC v13 00/20] Introduce PVH domU support