[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.