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] libxl: enabling upstream qemu as pure pv backend

The dm creating logic is as followed:

if hvm
  libxl__create_device_model
    if stubdom
      libxl__create_stubdom -> libxl__create_xenpv_qemu
                                |
                                --> libxl__create_device_model
    else
      spawn and exec qemu
else /* pv */
  if need_qemu
    libxl__create_xenpv_qemu
     |
     --> libxl__create_device_model

*
I think adding device_model_args_{pv,fv} is a good idea.

*
Since libxl__create_stubdom receives a dm_info structure, I think it is
ok for libxl__create_xenpv_qemu to receive one, too. This dm_info is key
structure to indicate xenpv qemu's type (traditional or upstream). But
once we enter libxl__create_xenpv_qemu, we lost knowledge of whether we
are creating a stubdom/qemu-xen or upstream qemu. So the caller should
be responsible for filling in a new dm_info for
libxl__create_xenpv_qemu.

*
vfb is derive from d_config (libxl_domain_config), which is a domain
property. Currently, the existing code either use
libxl__vfb_and_vkb_from_device_model_info (if we are using stubdom) or
direct parsing (if we are using xenpv_qemu). Though the code is
redundent, the parsing is just the same essentially. What's the point of
moving vnc and sdl out of vfb?

*
Configure two DMs for one domain? Haven't thought about that. I doubt
that if it really necessary if we are moving towards a unified DM --
upstream qemu -- wouldn't that be sufficient in the long run?

*
To my understanding, stubdom is minios+qemu-xen. If I (the user) am
using stubdom and specify device model args, these args should go to
xenpv qemu, not xenfv in stubdom, right? What I see in the code is that
we only need a few args (e.g. disks, vifs) to start stubdom. The
internal setup to connect to domU is done within stubdom.

To summarize, I give a second prototype of my patch. Please review.

libxl__build_xenpv_qemu_args handles common options to both xenpv qemu
and upstream qemu, while libxl__build_device_model_args distinguish
between old and new qemu's and build args respectively.

libxl__create_xenpv_qemu is not allocating a struct
libxl_device_model_info anymore, because at this point, it doesn't know
if it is building a stubdom/qemu-xen (traditional type) or upstream
qemu. The allocating and filling becomes caller's responsibility.

This patch has been tested with pv guest creating, fv guest creating and
fv-stubdom guest creating.

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

commit e7ca429c34bd1f306f0819d447456dbe48e53e6e
Author: Wei Liu <liuw@xxxxxxxxx>
Date:   Wed Jun 8 11:13:25 2011 +0800

    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 add device_model_args_{pv,fv} options for pv and
    fv 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 --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 62294b2..92550bb 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -486,6 +486,7 @@ static int do_domain_create(libxl__gc *gc, 
libxl_domain_config *d_config,
     } else {
         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]);
@@ -506,8 +507,15 @@ static int do_domain_create(libxl__gc *gc, 
libxl_domain_config *d_config,
         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;
         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);
     }
 
     if (dm_starting) {
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 47a51c8..473e3e4 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -207,9 +207,13 @@ static char ** 
libxl__build_device_model_args_old(libxl__gc *gc,
     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_FV:
         flexarray_append(dm_args, "xenfv");
+        for (i = 0; info->extra_fv && info->extra_fv[i] != NULL; i++)
+            flexarray_append(dm_args, info->extra_fv[i]);
         break;
     }
     flexarray_append(dm_args, NULL);
@@ -364,9 +368,13 @@ static char ** 
libxl__build_device_model_args_new(libxl__gc *gc,
     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_FV:
         flexarray_append(dm_args, "xenfv");
+        for (i = 0; info->extra_fv && info->extra_fv[i] != NULL; i++)
+            flexarray_append(dm_args, info->extra_fv[i]);
         break;
     }
 
@@ -575,6 +583,7 @@ static int libxl__create_stubdom(libxl__gc *gc,
     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;
@@ -608,6 +617,7 @@ static int libxl__create_stubdom(libxl__gc *gc,
 
     /* 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;
@@ -702,7 +712,15 @@ retry_transaction:
         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 =
+        LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
+    xenpv_dm_info.type = LIBXL_DOMAIN_TYPE_PV;
+    xenpv_dm_info.device_model = NULL;
+    if (libxl__create_xenpv_qemu(gc, domid,
+                                 &xenpv_dm_info,
+                                 vfb, &dm_starting) < 0) {
         ret = ERROR_FAIL;
         goto out_free;
     }
@@ -909,7 +927,6 @@ static int libxl__build_xenpv_qemu_args(libxl__gc *gc,
                                         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;
@@ -927,9 +944,6 @@ static int libxl__build_xenpv_qemu_args(libxl__gc *gc,
         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;
 }
 
@@ -985,12 +999,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 --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 64e1d56..55e9f7f 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -256,7 +256,9 @@ _hidden int libxl__create_device_model(libxl__gc *gc,
                               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,
+_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,
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 5415bc5..ced6dfb 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -627,6 +627,13 @@ static void parse_config_data(const char 
*configfile_filename_report,
     int pci_msitranslate = 1;
     int e;
 
+    XLU_ConfigList *dmargs;
+    int nr_dmargs = 0;
+    XLU_ConfigList *dmargs_fv;
+    int nr_dmargs_fv = 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;
 
@@ -1075,40 +1082,76 @@ skip_vfb:
         break;
     }
 
-    if (c_info->hvm == 1) {
-        XLU_ConfigList *dmargs;
-        int nr_dmargs = 0;
-
-        /* init dm from c and b */
-        libxl_init_dm_info(dm_info, c_info, b_info);
+    /* init dm from c and b */
+    libxl_init_dm_info(dm_info, c_info, b_info);
+    /* parse device model arguments, this works for pv, fv 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->hvm == 1) {
+                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");
+            }
+        }
+    }
 
-        /* then process config related to dm */
-        if (!xlu_cfg_get_string (config, "device_model", &buf)) {
+    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,
-                    "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");
+                    "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;
 
-        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;
+    /* parse extra args for qemu, common to both pv, fv */
+    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;
+        }
+    }
+    /* parse extra args dedicated to pv */
+    if (!xlu_cfg_get_list(config, "device_model_args_pv", &dmargs_pv, 
&nr_dmargs_pv, 0))
+    {
+        int i;
+        dm_info->extra_pv = xmalloc(sizeof(char *) * (nr_dmargs_pv + 1));
+        dm_info->extra_pv[nr_dmargs_pv] = NULL;
+        for (i = 0; i<nr_dmargs_pv; i++) {
+            const char *a = xlu_cfg_get_listitem(dmargs_pv, i);
+            dm_info->extra_pv[i] = a ? strdup(a) : NULL;
+        }
+    }
+    /* parse extra args dedicated to fv */
+    if (!xlu_cfg_get_list(config, "device_model_args_fv", &dmargs_fv, 
&nr_dmargs_fv, 0))
+    {
+        int i;
+        dm_info->extra_fv = xmalloc(sizeof(char *) * (nr_dmargs_fv + 1));
+        dm_info->extra_fv[nr_dmargs_fv] = NULL;
+        for (i = 0; i<nr_dmargs_fv; i++) {
+            const char *a = xlu_cfg_get_listitem(dmargs_fv, i);
+            dm_info->extra_fv[i] = a ? strdup(a) : NULL;
+        }
+    }
+
+    if (c_info->hvm == 1) {
         if (!xlu_cfg_get_long (config, "stdvga", &l))
             dm_info->stdvga = l;
         if (!xlu_cfg_get_long (config, "vnc", &l))
@@ -1136,17 +1179,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->hvm ?



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

<Prev in Thread] Current Thread [Next in Thread>