WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] Re: [PATCH UPDATED] libxl: enabling upstream qemu as pure pv

On Tue, 2011-07-19 at 09:47 +0100, Ian Campbell wrote: 
> On Tue, 2011-07-19 at 06:53 +0100, Wei Liu wrote:
> > diff -r 809882b6f1f8 -r fd24899c2558 tools/libxl/libxl_create.c
> > --- a/tools/libxl/libxl_create.c        Mon Jul 18 14:52:30 2011 +0100
> > +++ b/tools/libxl/libxl_create.c        Tue Jul 19 13:51:39 2011 +0800
> > @@ -530,6 +530,7 @@
> >      {
> >          int need_qemu = 0;
> >          libxl_device_console console;
> > +        libxl_device_model_info xenpv_dm_info;
> > 
> >          for (i = 0; i < d_config->num_vfbs; i++) {
> >              libxl_device_vfb_add(ctx, domid, &d_config->vfbs[i]);
> > @@ -550,8 +551,19 @@
> 
> Please can you add to your ~/.hgrc:
>         [diff]
>         showfunc = True
> 

Done. I'm not a hg fan TBH...

> I've updated http://wiki.xen.org/xenwiki/SubmittingXenPatches with it.
> 
> >          libxl__device_console_add(gc, domid, &console, &state);
> >          libxl_device_console_destroy(&console);
> > 
> > +        /* only copy those useful configs */
> > +        memset((void*)&xenpv_dm_info, 0x00, 
> > sizeof(libxl_device_model_info));
> > +        xenpv_dm_info.device_model_version =
> > +            d_config->dm_info.device_model_version;
> > +        xenpv_dm_info.type = d_config->dm_info.type;
> > +        xenpv_dm_info.device_model = d_config->dm_info.device_model;
> > +        xenpv_dm_info.extra = d_config->dm_info.extra;
> > +        xenpv_dm_info.extra_pv = d_config->dm_info.extra_pv;
> > +        xenpv_dm_info.extra_hvm = d_config->dm_info.extra_hvm;
> 
> All this could be under the following if ?
> 

OK, that's fine.

> >          if (need_qemu)
> > -            libxl__create_xenpv_qemu(gc, domid, d_config->vfbs, 
> > &dm_starting);
> > +            libxl__create_xenpv_qemu(gc, domid, &xenpv_dm_info,
> > +                                     d_config->vfbs, &dm_starting);
> > 
> > @@ -742,7 +751,14 @@
> >          if (ret)
> >              goto out_free;
> >      }
> > -    if (libxl__create_xenpv_qemu(gc, domid, vfb, &dm_starting) < 0) {
> > +
> > +    memset((void*)&xenpv_dm_info, 0x00, sizeof(libxl_device_model_info));
> > +    xenpv_dm_info.device_model_version = info->device_model_version;
> > +    xenpv_dm_info.type = LIBXL_DOMAIN_TYPE_PV;
> > +    xenpv_dm_info.device_model = info->device_model;
> 
> Why don't extra_* get copied here?
> 

Because they are useless to stubdom at the moment.

However, adding three more lines won't hurt.

> Perhaps this and the other similar hunk above could be split into a
> common libxl__init_xenpv_dm_info(&xenpv_dm_info, info)?
> 
> If there is a reason for the extra_* difference then add an "int
> is_stubdom" to the init function and gate accordingly?
> 
> That said, I've no objection to this patch going in as is and addressing
> comments later as necessary.
> 

I think I would just leave it as it is at the moment.

> Ian.
> 
> 

Patch revised according to your comments.

Wei.

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

# HG changeset patch
# User Wei Liu <liuw@xxxxxxxxx>
# Date 1311067941 -28800
# Node ID db9937db859a634f1aa6361bdba41bdcb9906224
# Parent  809882b6f1f84a5673d0ea18dfd392bb35774741
libxl: enabling upstream qemu as pure pv backend.

This patch makes device_model_{version,override} work for pure pv
guest, so that users can specify upstream qemu as pure pv backend
other than traditional qemu-xen.

This patch also adds device_model_args_{pv,hvm} options for pv and
hvm guest respectively.

Internally, original libxl__create_xenpv_qemu allocates a new empty
dm_info (struct libxl_device_model_info) for every xenpv qemu created.
Now the caller of libxl__create_xenpv_qemu is responsible for
allocating this dm_info.

Signed-off-by: Wei Liu <liuw@xxxxxxxxx>

diff -r 809882b6f1f8 -r db9937db859a tools/libxl/libxl.idl
--- a/tools/libxl/libxl.idl     Mon Jul 18 14:52:30 2011 +0100
+++ b/tools/libxl/libxl.idl     Tue Jul 19 17:32:21 2011 +0800
@@ -238,6 +238,8 @@ The password never expires"""),
     ("vcpu_avail",       integer,           False, "vcpus actually available"),
     ("xen_platform_pci", bool,              False, "enable/disable the xen 
platform pci device"),
     ("extra",            libxl_string_list, False, "extra parameters pass 
directly to qemu, NULL terminated"),
+    ("extra_pv",         libxl_string_list, False, "extra parameters pass 
directly to qemu for PV guest, NULL terminated"),
+    ("extra_hvm",        libxl_string_list, False, "extra parameters pass 
directly to qemu for HVM guest, NULL terminated"),
     ],
     comment=
 """Device Model information.
diff -r 809882b6f1f8 -r db9937db859a tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c        Mon Jul 18 14:52:30 2011 +0100
+++ b/tools/libxl/libxl_create.c        Tue Jul 19 17:32:21 2011 +0800
@@ -530,6 +530,7 @@ static int do_domain_create(libxl__gc *g
     {
         int need_qemu = 0;
         libxl_device_console console;
+        libxl_device_model_info xenpv_dm_info;
 
         for (i = 0; i < d_config->num_vfbs; i++) {
             libxl_device_vfb_add(ctx, domid, &d_config->vfbs[i]);
@@ -550,8 +551,20 @@ static int do_domain_create(libxl__gc *g
         libxl__device_console_add(gc, domid, &console, &state);
         libxl_device_console_destroy(&console);
 
-        if (need_qemu)
-            libxl__create_xenpv_qemu(gc, domid, d_config->vfbs, &dm_starting);
+        if (need_qemu) {
+            /* only copy those useful configs */
+            memset((void*)&xenpv_dm_info, 0, sizeof(libxl_device_model_info));
+            xenpv_dm_info.device_model_version =
+                d_config->dm_info.device_model_version;
+            xenpv_dm_info.type = d_config->dm_info.type;
+            xenpv_dm_info.device_model = d_config->dm_info.device_model;
+            xenpv_dm_info.extra = d_config->dm_info.extra;
+            xenpv_dm_info.extra_pv = d_config->dm_info.extra_pv;
+            xenpv_dm_info.extra_hvm = d_config->dm_info.extra_hvm;
+
+            libxl__create_xenpv_qemu(gc, domid, &xenpv_dm_info,
+                                     d_config->vfbs, &dm_starting);
+        }
         break;
     }
     default:
diff -r 809882b6f1f8 -r db9937db859a tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c    Mon Jul 18 14:52:30 2011 +0100
+++ b/tools/libxl/libxl_dm.c    Tue Jul 19 17:32:21 2011 +0800
@@ -206,9 +206,13 @@ static char ** libxl__build_device_model
     switch (info->type) {
     case LIBXL_DOMAIN_TYPE_PV:
         flexarray_append(dm_args, "xenpv");
+        for (i = 0; info->extra_pv && info->extra_pv[i] != NULL; i++)
+            flexarray_append(dm_args, info->extra_pv[i]);
         break;
     case LIBXL_DOMAIN_TYPE_HVM:
         flexarray_append(dm_args, "xenfv");
+        for (i = 0; info->extra_hvm && info->extra_hvm[i] != NULL; i++)
+            flexarray_append(dm_args, info->extra_hvm[i]);
         break;
     }
     flexarray_append(dm_args, NULL);
@@ -403,9 +407,13 @@ static char ** libxl__build_device_model
     switch (info->type) {
     case LIBXL_DOMAIN_TYPE_PV:
         flexarray_append(dm_args, "xenpv");
+        for (i = 0; info->extra_pv && info->extra_pv[i] != NULL; i++)
+            flexarray_append(dm_args, info->extra_pv[i]);
         break;
     case LIBXL_DOMAIN_TYPE_HVM:
         flexarray_append(dm_args, "xenfv");
+        for (i = 0; info->extra_hvm && info->extra_hvm[i] != NULL; i++)
+            flexarray_append(dm_args, info->extra_hvm[i]);
         break;
     }
 
@@ -614,6 +622,7 @@ static int libxl__create_stubdom(libxl__
     struct xs_permissions perm[2];
     xs_transaction_t t;
     libxl__device_model_starting *dm_starting = 0;
+    libxl_device_model_info xenpv_dm_info;
 
     if (info->device_model_version != 
LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL) {
         ret = ERROR_INVAL;
@@ -647,7 +656,7 @@ static int libxl__create_stubdom(libxl__
     b_info.u.pv.features = "";
 
     /* fixme: this function can leak the stubdom if it fails */
-
+    domid = 0;
     ret = libxl__domain_make(gc, &c_info, &domid);
     if (ret)
         goto out_free;
@@ -742,7 +751,18 @@ retry_transaction:
         if (ret)
             goto out_free;
     }
-    if (libxl__create_xenpv_qemu(gc, domid, vfb, &dm_starting) < 0) {
+
+    memset((void*)&xenpv_dm_info, 0, sizeof(libxl_device_model_info));
+    xenpv_dm_info.device_model_version = info->device_model_version;
+    xenpv_dm_info.type = LIBXL_DOMAIN_TYPE_PV;
+    xenpv_dm_info.device_model = info->device_model;
+    xenpv_dm_info.extra = info->extra;
+    xenpv_dm_info.extra_pv = info->extra_pv;
+    xenpv_dm_info.extra_hvm = info->extra_hvm;
+
+    if (libxl__create_xenpv_qemu(gc, domid,
+                                 &xenpv_dm_info,
+                                 vfb, &dm_starting) < 0) {
         ret = ERROR_FAIL;
         goto out_free;
     }
@@ -951,7 +971,6 @@ static int libxl__build_xenpv_qemu_args(
                                         libxl_device_model_info *info)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
-    memset(info, 0x00, sizeof(libxl_device_model_info));
 
     if (vfb != NULL) {
         info->vnc = vfb->vnc;
@@ -969,9 +988,6 @@ static int libxl__build_xenpv_qemu_args(
         info->nographic = 1;
     info->domid = domid;
     info->dom_name = libxl_domid_to_name(ctx, domid);
-    info->device_model_version = 
LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
-    info->device_model = NULL;
-    info->type = LIBXL_DOMAIN_TYPE_PV;
     return 0;
 }
 
@@ -1012,12 +1028,12 @@ out:
     return ret;
 }
 
-int libxl__create_xenpv_qemu(libxl__gc *gc, uint32_t domid, libxl_device_vfb 
*vfb,
+int libxl__create_xenpv_qemu(libxl__gc *gc, uint32_t domid,
+                             libxl_device_model_info *info,
+                             libxl_device_vfb *vfb,
                              libxl__device_model_starting **starting_r)
 {
-    libxl_device_model_info info;
-
-    libxl__build_xenpv_qemu_args(gc, domid, vfb, &info);
-    libxl__create_device_model(gc, &info, NULL, 0, NULL, 0, starting_r);
+    libxl__build_xenpv_qemu_args(gc, domid, vfb, info);
+    libxl__create_device_model(gc, info, NULL, 0, NULL, 0, starting_r);
     return 0;
 }
diff -r 809882b6f1f8 -r db9937db859a tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h      Mon Jul 18 14:52:30 2011 +0100
+++ b/tools/libxl/libxl_internal.h      Tue Jul 19 17:32:21 2011 +0800
@@ -267,8 +267,10 @@ _hidden int libxl__create_device_model(l
                               libxl_device_disk *disk, int num_disks,
                               libxl_device_nic *vifs, int num_vifs,
                               libxl__device_model_starting **starting_r);
-_hidden int libxl__create_xenpv_qemu(libxl__gc *gc, uint32_t domid, 
libxl_device_vfb *vfb,
-                            libxl__device_model_starting **starting_r);
+_hidden int libxl__create_xenpv_qemu(libxl__gc *gc, uint32_t domid,
+                              libxl_device_model_info *dm_info,
+                              libxl_device_vfb *vfb,
+                              libxl__device_model_starting **starting_r);
 _hidden int libxl__need_xenpv_qemu(libxl__gc *gc,
         int nr_consoles, libxl_device_console *consoles,
         int nr_vfbs, libxl_device_vfb *vfbs,
diff -r 809882b6f1f8 -r db9937db859a tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c  Mon Jul 18 14:52:30 2011 +0100
+++ b/tools/libxl/xl_cmdimpl.c  Tue Jul 19 17:32:21 2011 +0800
@@ -529,6 +529,13 @@ static void parse_config_data(const char
     int pci_msitranslate = 1;
     int e;
 
+    XLU_ConfigList *dmargs;
+    int nr_dmargs = 0;
+    XLU_ConfigList *dmargs_hvm;
+    int nr_dmargs_hvm = 0;
+    XLU_ConfigList *dmargs_pv;
+    int nr_dmargs_pv = 0;
+
     libxl_domain_create_info *c_info = &d_config->c_info;
     libxl_domain_build_info *b_info = &d_config->b_info;
 
@@ -552,7 +559,7 @@ static void parse_config_data(const char
                                     &c_info->ssidref);
         if (e) {
             if (errno == ENOSYS) {
-                fprintf(stderr, "XSM Disabled: seclabel not supported\n");    
+                fprintf(stderr, "XSM Disabled: seclabel not supported\n");
             } else {
                 fprintf(stderr, "Invalid seclabel: %s\n", buf);
                 exit(1);
@@ -1014,41 +1021,72 @@ skip_vfb:
         break;
     }
 
+    /* init dm from c and b */
+    if (libxl_init_dm_info(ctx, dm_info, c_info, b_info))
+        exit(1);
+    /* parse device model arguments, this works for pv, hvm and stubdom */
+    if (!xlu_cfg_get_string (config, "device_model", &buf)) {
+        fprintf(stderr,
+                "WARNING: ignoring device_model directive.\n"
+                "WARNING: Use \"device_model_override\" instead if you"
+                " really want a non-default device_model\n");
+        if (strstr(buf, "stubdom-dm")) {
+            if (c_info->type == LIBXL_DOMAIN_TYPE_HVM)
+                fprintf(stderr, "WARNING: Or use"
+                        " \"device_model_stubdomain_override\" if you "
+                        " want to enable stubdomains\n");
+            else
+                fprintf(stderr, "WARNING: ignoring"
+                        " \"device_model_stubdomain_override\" directive"
+                        " for pv guest\n");
+        }
+    }
+
+
+    xlu_cfg_replace_string (config, "device_model_override",
+                            &dm_info->device_model);
+    if (!xlu_cfg_get_string (config, "device_model_version", &buf)) {
+        if (!strcmp(buf, "qemu-xen-traditional")) {
+            dm_info->device_model_version
+                = LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
+        } else if (!strcmp(buf, "qemu-xen")) {
+            dm_info->device_model_version
+                = LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
+        } else {
+            fprintf(stderr,
+                    "Unknown device_model_version \"%s\" specified\n", buf);
+            exit(1);
+        }
+    } else if (dm_info->device_model)
+        fprintf(stderr, "WARNING: device model override given without specific 
DM version\n");
+    if (!xlu_cfg_get_long (config, "device_model_stubdomain_override", &l))
+        dm_info->device_model_stubdomain = l;
+
+#define parse_extra_args(type)                                          \
+    if (!xlu_cfg_get_list(config, "device_model_args"#type,             \
+                          &dmargs##type, &nr_dmargs##type, 0))          \
+    {                                                                   \
+        int i;                                                          \
+        dm_info->extra##type =                                          \
+            xmalloc(sizeof(char*)*(nr_dmargs##type + 1));               \
+        dm_info->extra##type[nr_dmargs##type] = NULL;                   \
+        for (i=0; i<nr_dmargs##type; i++) {                             \
+            const char *a = xlu_cfg_get_listitem(dmargs##type, i);      \
+            dm_info->extra##type[i] = a ? strdup(a) : NULL;             \
+        }                                                               \
+    }                                                                   \
+    /* parse extra args for qemu, common to both pv, hvm */
+    parse_extra_args();
+
+    /* parse extra args dedicated to pv */
+    parse_extra_args(_pv);
+
+    /* parse extra args dedicated to hvm */
+    parse_extra_args(_hvm);
+
+#undef parse_extra_args
+
     if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) {
-        XLU_ConfigList *dmargs;
-        int nr_dmargs = 0;
-
-        /* init dm from c and b */
-        if (libxl_init_dm_info(ctx, dm_info, c_info, b_info))
-            exit(1);
-
-        /* then process config related to dm */
-        if (!xlu_cfg_get_string (config, "device_model", &buf)) {
-            fprintf(stderr,
-                    "WARNING: ignoring device_model directive.\n"
-                    "WARNING: Use \"device_model_override\" instead if you 
really want a non-default device_model\n");
-            if (strstr(buf, "stubdom-dm"))
-                fprintf(stderr, "WARNING: Or use 
\"device_model_stubdomain_override\" if you want to enable stubdomains\n");
-        }
-
-        xlu_cfg_replace_string (config, "device_model_override",
-                                &dm_info->device_model);
-        if (!xlu_cfg_get_string (config, "device_model_version", &buf)) {
-            if (!strcmp(buf, "qemu-xen-traditional")) {
-                dm_info->device_model_version
-                    = LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
-            } else if (!strcmp(buf, "qemu-xen")) {
-                dm_info->device_model_version
-                    = LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
-            } else {
-                fprintf(stderr,
-                        "Unknown device_model_version \"%s\" specified\n", 
buf);
-                exit(1);
-            }
-        } else if (dm_info->device_model)
-            fprintf(stderr, "WARNING: device model override given without 
specific DM version\n");
-        if (!xlu_cfg_get_long (config, "device_model_stubdomain_override", &l))
-            dm_info->device_model_stubdomain = l;
         if (!xlu_cfg_get_long (config, "stdvga", &l))
             dm_info->stdvga = l;
         if (!xlu_cfg_get_long (config, "vnc", &l))
@@ -1090,17 +1128,6 @@ skip_vfb:
         xlu_cfg_replace_string (config, "soundhw", &dm_info->soundhw);
         if (!xlu_cfg_get_long (config, "xen_platform_pci", &l))
             dm_info->xen_platform_pci = l;
-
-        if (!xlu_cfg_get_list(config, "device_model_args", &dmargs, 
&nr_dmargs, 0))
-        {
-            int i;
-            dm_info->extra = xmalloc(sizeof(char *) * (nr_dmargs + 1));
-            dm_info->extra[nr_dmargs] = NULL;
-            for (i=0; i<nr_dmargs; i++) {
-                const char *a = xlu_cfg_get_listitem(dmargs, i);
-                dm_info->extra[i] = a ? strdup(a) : NULL;
-            }
-        }
     }
 
     dm_info->type = c_info->type;




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel