xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2] libxl: enabling upstream qemu as pv backend
@ 2011-06-10 11:27 Wei Liu
  2011-06-10 14:03 ` Stefano Stabellini
  2011-06-21 15:46 ` Ian Jackson
  0 siblings, 2 replies; 12+ messages in thread
From: Wei Liu @ 2011-06-10 11:27 UTC (permalink / raw)
  To: xen-devel@lists.xensource.com; +Cc: Ian Campbell, Stefano Stabellini

This is a modified version of the original patch.

Stefano and Ian, please review.

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

commit b471938f13f2fa118776c563d7cf3b440c308492
Author: Wei Liu <liuw@liuw.name>
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@liuw.name>

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..fb0e0b1 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,14 @@ 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 = info->device_model_version;
+    xenpv_dm_info.type = LIBXL_DOMAIN_TYPE_PV;
+    xenpv_dm_info.device_model = info->device_model;
+    if (libxl__create_xenpv_qemu(gc, domid,
+                                 &xenpv_dm_info,
+                                 vfb, &dm_starting) < 0) {
         ret = ERROR_FAIL;
         goto out_free;
     }
@@ -909,7 +926,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 +943,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 +998,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..2546c0b 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");
 
-        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 (c_info->hvm == 1 && !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 ?

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] libxl: enabling upstream qemu as pv backend
  2011-06-10 11:27 [PATCH V2] libxl: enabling upstream qemu as pv backend Wei Liu
@ 2011-06-10 14:03 ` Stefano Stabellini
  2011-06-14 14:39   ` Ian Campbell
  2011-06-21 15:46 ` Ian Jackson
  1 sibling, 1 reply; 12+ messages in thread
From: Stefano Stabellini @ 2011-06-10 14:03 UTC (permalink / raw)
  To: Wei Liu; +Cc: Ian Campbell, xen-devel@lists.xensource.com, Stabellini

On Fri, 10 Jun 2011, Wei Liu wrote:
> This is a modified version of the original patch.
> 
> Stefano and Ian, please review.
> 

It is fine by me

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] libxl: enabling upstream qemu as pv backend
  2011-06-10 14:03 ` Stefano Stabellini
@ 2011-06-14 14:39   ` Ian Campbell
  0 siblings, 0 replies; 12+ messages in thread
From: Ian Campbell @ 2011-06-14 14:39 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: xen-devel@lists.xensource.com, Wei Liu

On Fri, 2011-06-10 at 15:03 +0100, Stefano Stabellini wrote:
> On Fri, 10 Jun 2011, Wei Liu wrote:
> > This is a modified version of the original patch.
> > 
> > Stefano and Ian, please review.
> > 
> 
> It is fine by me

Me too.

Ian.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] libxl: enabling upstream qemu as pv backend
  2011-06-10 11:27 [PATCH V2] libxl: enabling upstream qemu as pv backend Wei Liu
  2011-06-10 14:03 ` Stefano Stabellini
@ 2011-06-21 15:46 ` Ian Jackson
  2011-06-23  2:17   ` Wei Liu
  1 sibling, 1 reply; 12+ messages in thread
From: Ian Jackson @ 2011-06-21 15:46 UTC (permalink / raw)
  To: Wei Liu; +Cc: Ian Campbell, xen-devel@lists.xensource.com, Stefano Stabellini

Wei Liu writes ("[Xen-devel] [PATCH V2] libxl: enabling upstream qemu as pv backend"):
> This is a modified version of the original patch.

Thanks.  This looked reasonable so I tried to apply it, fixing up the
long lines myself.  But:

 xl_cmdimpl.c: In function 'parse_config_data':
 xl_cmdimpl.c:1181: error: 'libxl_device_model_info' has no member named 'extra_pv'
 xl_cmdimpl.c:1182: error: 'libxl_device_model_info' has no member named 'extra_pv'
 [etc]

Can you check that this patch works when applied to xen-unstable tip ?

Thanks,
Ian.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] libxl: enabling upstream qemu as pv backend
  2011-06-21 15:46 ` Ian Jackson
@ 2011-06-23  2:17   ` Wei Liu
  2011-06-23  2:53     ` Wei Liu
  0 siblings, 1 reply; 12+ messages in thread
From: Wei Liu @ 2011-06-23  2:17 UTC (permalink / raw)
  To: Ian Jackson
  Cc: Ian Campbell, xen-devel@lists.xensource.com, Stefano Stabellini

Sorry I missed some lines in libxl.idl .

Attached is the correct version.

commit 4150ea233438bf3fa837e5bc5746b01859fbbbb7
Author: Wei Liu <liuw@liuw.name>
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 adds 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@liuw.name>

diff --git a/tools/libxl/libxl.idl b/tools/libxl/libxl.idl
index f0fb22f..b5c81a3 100644
--- a/tools/libxl/libxl.idl
+++ b/tools/libxl/libxl.idl
@@ -222,6 +222,8 @@ libxl_device_model_info = Struct("device_model_info",[
     ("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_fv",         libxl_string_list, False, "extra parameters
pass directly to qemu for FV guest, NULL terminated"),
     ],
     comment=
 """Device Model information.
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..fb0e0b1 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,14 @@ 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 = info->device_model_version;
+    xenpv_dm_info.type = LIBXL_DOMAIN_TYPE_PV;
+    xenpv_dm_info.device_model = info->device_model;
+    if (libxl__create_xenpv_qemu(gc, domid,
+                                 &xenpv_dm_info,
+                                 vfb, &dm_starting) < 0) {
         ret = ERROR_FAIL;
         goto out_free;
     }
@@ -909,7 +926,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 +943,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 +998,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..2546c0b 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");

-        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 (c_info->hvm == 1 && !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 ?

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] libxl: enabling upstream qemu as pv backend
  2011-06-23  2:17   ` Wei Liu
@ 2011-06-23  2:53     ` Wei Liu
  2011-06-27 14:14       ` Ian Jackson
  0 siblings, 1 reply; 12+ messages in thread
From: Wei Liu @ 2011-06-23  2:53 UTC (permalink / raw)
  To: Ian Jackson
  Cc: Ian Campbell, xen-devel@lists.xensource.com, Stefano Stabellini

[-- Attachment #1: Type: text/plain, Size: 101 bytes --]

Resend.

Gmail is really bad for submitting patch, but it is the only choice
for the moment...

Wei.

[-- Attachment #2: libxl-upstream-qemu.patch --]
[-- Type: application/octet-stream, Size: 13984 bytes --]

commit 4150ea233438bf3fa837e5bc5746b01859fbbbb7
Author: Wei Liu <liuw@liuw.name>
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 adds 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@liuw.name>

diff --git a/tools/libxl/libxl.idl b/tools/libxl/libxl.idl
index f0fb22f..b5c81a3 100644
--- a/tools/libxl/libxl.idl
+++ b/tools/libxl/libxl.idl
@@ -222,6 +222,8 @@ libxl_device_model_info = Struct("device_model_info",[
     ("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_fv",         libxl_string_list, False, "extra parameters pass directly to qemu for FV guest, NULL terminated"),
     ],
     comment=
 """Device Model information.
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..fb0e0b1 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,14 @@ 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 = info->device_model_version;
+    xenpv_dm_info.type = LIBXL_DOMAIN_TYPE_PV;
+    xenpv_dm_info.device_model = info->device_model;
+    if (libxl__create_xenpv_qemu(gc, domid,
+                                 &xenpv_dm_info,
+                                 vfb, &dm_starting) < 0) {
         ret = ERROR_FAIL;
         goto out_free;
     }
@@ -909,7 +926,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 +943,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 +998,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..2546c0b 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");
 
-        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 (c_info->hvm == 1 && !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 ?

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] libxl: enabling upstream qemu as pv backend
  2011-06-23  2:53     ` Wei Liu
@ 2011-06-27 14:14       ` Ian Jackson
  2011-06-28  3:01         ` Wei Liu
  0 siblings, 1 reply; 12+ messages in thread
From: Ian Jackson @ 2011-06-27 14:14 UTC (permalink / raw)
  To: Wei Liu; +Cc: Ian Campbell, xen-devel@lists.xensource.com, Stefano Stabellini

Wei Liu writes ("Re: [Xen-devel] [PATCH V2] libxl: enabling upstream qemu as pv backend"):
> Resend.

Thanks.

> Gmail is really bad for submitting patch, but it is the only choice
> for the moment...

The patch seems mostly OK to me but I have two small complaints:


Firstly we have three copies of essentially this:

+    /* 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;
+        }
+    }

Can you please find a way to do this that doesn't involve open-coding
the same 13 lines for each case ?  A small helper function or a macro
would do.


Secondly, there are still some overly long lines.  Can you please,
before you resend, check that there are no added lines in your diff
which are over 80 (preferably, 75) characters ?  This includes lines
containing message strings.  Long lines look like this to me:

+                "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_ovr
eride\" if you want to enable stubdomains\n");
+            else
+                fprintf(stderr, "WARNING: ignoring \"device_model_stubdomain_o
verride\" directive for pv guest\n");
+        }
+    }

This makes it very hard to see the structure.


Thanks,
Ian.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] libxl: enabling upstream qemu as pv backend
  2011-06-27 14:14       ` Ian Jackson
@ 2011-06-28  3:01         ` Wei Liu
  2011-06-28  8:26           ` Ian Campbell
  0 siblings, 1 reply; 12+ messages in thread
From: Wei Liu @ 2011-06-28  3:01 UTC (permalink / raw)
  To: Ian Jackson
  Cc: Ian Campbell, xen-devel@lists.xensource.com, Stefano Stabellini

[-- Attachment #1: Type: text/plain, Size: 118 bytes --]

Revised.

No added lines are longer than 80 in C code.

A macro is used to avoid open-coding the same 13 lines.

Wei.

[-- Attachment #2: libxl-upstream-qemu.patch --]
[-- Type: application/octet-stream, Size: 13984 bytes --]

commit 4150ea233438bf3fa837e5bc5746b01859fbbbb7
Author: Wei Liu <liuw@liuw.name>
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 adds 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@liuw.name>

diff --git a/tools/libxl/libxl.idl b/tools/libxl/libxl.idl
index f0fb22f..b5c81a3 100644
--- a/tools/libxl/libxl.idl
+++ b/tools/libxl/libxl.idl
@@ -222,6 +222,8 @@ libxl_device_model_info = Struct("device_model_info",[
     ("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_fv",         libxl_string_list, False, "extra parameters pass directly to qemu for FV guest, NULL terminated"),
     ],
     comment=
 """Device Model information.
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..fb0e0b1 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,14 @@ 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 = info->device_model_version;
+    xenpv_dm_info.type = LIBXL_DOMAIN_TYPE_PV;
+    xenpv_dm_info.device_model = info->device_model;
+    if (libxl__create_xenpv_qemu(gc, domid,
+                                 &xenpv_dm_info,
+                                 vfb, &dm_starting) < 0) {
         ret = ERROR_FAIL;
         goto out_free;
     }
@@ -909,7 +926,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 +943,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 +998,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..2546c0b 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");
 
-        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 (c_info->hvm == 1 && !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 ?

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] libxl: enabling upstream qemu as pv backend
  2011-06-28  3:01         ` Wei Liu
@ 2011-06-28  8:26           ` Ian Campbell
  2011-06-28  8:54             ` Wei Liu
  0 siblings, 1 reply; 12+ messages in thread
From: Ian Campbell @ 2011-06-28  8:26 UTC (permalink / raw)
  To: Wei Liu; +Cc: xen-devel@lists.xensource.com, Ian Jackson, Stefano Stabellini

On Tue, 2011-06-28 at 04:01 +0100, Wei Liu wrote:
> Revised.

The attached patch was identical to the previous post -- I think you
attached the wrong one.

Ian.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] libxl: enabling upstream qemu as pv backend
  2011-06-28  8:26           ` Ian Campbell
@ 2011-06-28  8:54             ` Wei Liu
  2011-07-15 12:38               ` Ian Campbell
  0 siblings, 1 reply; 12+ messages in thread
From: Wei Liu @ 2011-06-28  8:54 UTC (permalink / raw)
  To: Ian Campbell
  Cc: xen-devel@lists.xensource.com, Ian Jackson, Stefano Stabellini

[-- Attachment #1: Type: text/plain, Size: 265 bytes --]

On Tue, Jun 28, 2011 at 4:26 PM, Ian Campbell
<Ian.Campbell@eu.citrix.com> wrote:
> On Tue, 2011-06-28 at 04:01 +0100, Wei Liu wrote:
>> Revised.
>
> The attached patch was identical to the previous post -- I think you
> attached the wrong one.
>

Sorry. :-(

Wei.

[-- Attachment #2: libxl-upstream-qemu.patch --]
[-- Type: application/octet-stream, Size: 13927 bytes --]

commit 0b9aa4140ff54e6052624774b285a9b08db9c161
Author: Wei Liu <liuw@liuw.name>
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 adds 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@liuw.name>

diff --git a/tools/libxl/libxl.idl b/tools/libxl/libxl.idl
index f0fb22f..b5c81a3 100644
--- a/tools/libxl/libxl.idl
+++ b/tools/libxl/libxl.idl
@@ -222,6 +222,8 @@ libxl_device_model_info = Struct("device_model_info",[
     ("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_fv",         libxl_string_list, False, "extra parameters pass directly to qemu for FV guest, NULL terminated"),
     ],
     comment=
 """Device Model information.
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..fb0e0b1 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,14 @@ 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 = info->device_model_version;
+    xenpv_dm_info.type = LIBXL_DOMAIN_TYPE_PV;
+    xenpv_dm_info.device_model = info->device_model;
+    if (libxl__create_xenpv_qemu(gc, domid,
+                                 &xenpv_dm_info,
+                                 vfb, &dm_starting) < 0) {
         ret = ERROR_FAIL;
         goto out_free;
     }
@@ -909,7 +926,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 +943,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 +998,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..430a09b 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,73 @@ 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 (c_info->hvm == 1 &&
+        !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 + 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;             \
+        }                                                               \
+    }                                                                   \
 
-        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 */
+    parse_extra_args();
+
+    /* parse extra args dedicated to pv */
+    parse_extra_args(_pv);
+
+    /* parse extra args dedicated to fv */
+    parse_extra_args(_fv);
+
+#undef parse_extra_args
+
+    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 +1176,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 ?

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] libxl: enabling upstream qemu as pv backend
  2011-06-28  8:54             ` Wei Liu
@ 2011-07-15 12:38               ` Ian Campbell
  2011-07-15 13:20                 ` Wei Liu
  0 siblings, 1 reply; 12+ messages in thread
From: Ian Campbell @ 2011-07-15 12:38 UTC (permalink / raw)
  To: Wei Liu; +Cc: xen-devel@lists.xensource.com, Ian Jackson, Stefano Stabellini

On Tue, 2011-06-28 at 09:54 +0100, Wei Liu wrote:
> On Tue, Jun 28, 2011 at 4:26 PM, Ian Campbell
> <Ian.Campbell@eu.citrix.com> wrote:
> > On Tue, 2011-06-28 at 04:01 +0100, Wei Liu wrote:
> >> Revised.
> >
> > The attached patch was identical to the previous post -- I think you
> > attached the wrong one.
> >
> 
> Sorry. :-(

This appears to have gotten dropped.

Wei would you mind reposting (rebasing if necessary)?

Ian.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2] libxl: enabling upstream qemu as pv backend
  2011-07-15 12:38               ` Ian Campbell
@ 2011-07-15 13:20                 ` Wei Liu
  0 siblings, 0 replies; 12+ messages in thread
From: Wei Liu @ 2011-07-15 13:20 UTC (permalink / raw)
  To: Ian Campbell
  Cc: xen-devel@lists.xensource.com, Ian Jackson, Stefano Stabellini

On Fri, Jul 15, 2011 at 8:38 PM, Ian Campbell
<Ian.Campbell@eu.citrix.com> wrote:
> On Tue, 2011-06-28 at 09:54 +0100, Wei Liu wrote:
>> On Tue, Jun 28, 2011 at 4:26 PM, Ian Campbell
>> <Ian.Campbell@eu.citrix.com> wrote:
>> > On Tue, 2011-06-28 at 04:01 +0100, Wei Liu wrote:
>> >> Revised.
>> >
>> > The attached patch was identical to the previous post -- I think you
>> > attached the wrong one.
>> >
>>
>> Sorry. :-(
>
> This appears to have gotten dropped.
>
> Wei would you mind reposting (rebasing if necessary)?
>
> Ian.
>
>
>

OK, I will post a newer version tomorrow.

Please don't apply this version. I've updated it a bit.

Wei.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2011-07-15 13:20 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-10 11:27 [PATCH V2] libxl: enabling upstream qemu as pv backend Wei Liu
2011-06-10 14:03 ` Stefano Stabellini
2011-06-14 14:39   ` Ian Campbell
2011-06-21 15:46 ` Ian Jackson
2011-06-23  2:17   ` Wei Liu
2011-06-23  2:53     ` Wei Liu
2011-06-27 14:14       ` Ian Jackson
2011-06-28  3:01         ` Wei Liu
2011-06-28  8:26           ` Ian Campbell
2011-06-28  8:54             ` Wei Liu
2011-07-15 12:38               ` Ian Campbell
2011-07-15 13:20                 ` Wei Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).