Re: [Xen-devel] [PATCH V3] libxl: write IO ABI for disk frontends

[Wei Liu <wei.liu2@xxxxxxxxxx>]

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@xxxxxxxxxx>
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@xxxxxxxxxx>
    Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
    Cc: Roger Pau Monne <roger.pau@xxxxxxxxxx>
    Cc: Valtteri Kiviniemi <kiviniemi.valtteri@xxxxxxxxx>

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));

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