On Tue, 2011-07-19 at 09:47 +0100, Ian Campbell wrote:
> On Tue, 2011-07-19 at 06:53 +0100, Wei Liu wrote:
> > diff -r 809882b6f1f8 -r fd24899c2558 tools/libxl/libxl_create.c
> > --- a/tools/libxl/libxl_create.c Mon Jul 18 14:52:30 2011 +0100
> > +++ b/tools/libxl/libxl_create.c Tue Jul 19 13:51:39 2011 +0800
> > @@ -530,6 +530,7 @@
> > {
> > int need_qemu = 0;
> > libxl_device_console console;
> > + libxl_device_model_info xenpv_dm_info;
> >
> > for (i = 0; i < d_config->num_vfbs; i++) {
> > libxl_device_vfb_add(ctx, domid, &d_config->vfbs[i]);
> > @@ -550,8 +551,19 @@
>
> Please can you add to your ~/.hgrc:
> [diff]
> showfunc = True
>
Done. I'm not a hg fan TBH...
> I've updated http://wiki.xen.org/xenwiki/SubmittingXenPatches with it.
>
> > libxl__device_console_add(gc, domid, &console, &state);
> > libxl_device_console_destroy(&console);
> >
> > + /* only copy those useful configs */
> > + memset((void*)&xenpv_dm_info, 0x00,
> > sizeof(libxl_device_model_info));
> > + xenpv_dm_info.device_model_version =
> > + d_config->dm_info.device_model_version;
> > + xenpv_dm_info.type = d_config->dm_info.type;
> > + xenpv_dm_info.device_model = d_config->dm_info.device_model;
> > + xenpv_dm_info.extra = d_config->dm_info.extra;
> > + xenpv_dm_info.extra_pv = d_config->dm_info.extra_pv;
> > + xenpv_dm_info.extra_hvm = d_config->dm_info.extra_hvm;
>
> All this could be under the following if ?
>
OK, that's fine.
> > if (need_qemu)
> > - libxl__create_xenpv_qemu(gc, domid, d_config->vfbs,
> > &dm_starting);
> > + libxl__create_xenpv_qemu(gc, domid, &xenpv_dm_info,
> > + d_config->vfbs, &dm_starting);
> >
> > @@ -742,7 +751,14 @@
> > if (ret)
> > goto out_free;
> > }
> > - if (libxl__create_xenpv_qemu(gc, domid, vfb, &dm_starting) < 0) {
> > +
> > + memset((void*)&xenpv_dm_info, 0x00, sizeof(libxl_device_model_info));
> > + xenpv_dm_info.device_model_version = info->device_model_version;
> > + xenpv_dm_info.type = LIBXL_DOMAIN_TYPE_PV;
> > + xenpv_dm_info.device_model = info->device_model;
>
> Why don't extra_* get copied here?
>
Because they are useless to stubdom at the moment.
However, adding three more lines won't hurt.
> Perhaps this and the other similar hunk above could be split into a
> common libxl__init_xenpv_dm_info(&xenpv_dm_info, info)?
>
> If there is a reason for the extra_* difference then add an "int
> is_stubdom" to the init function and gate accordingly?
>
> That said, I've no objection to this patch going in as is and addressing
> comments later as necessary.
>
I think I would just leave it as it is at the moment.
> Ian.
>
>
Patch revised according to your comments.
Wei.
------8<------
# HG changeset patch
# User Wei Liu <liuw@xxxxxxxxx>
# Date 1311067941 -28800
# Node ID db9937db859a634f1aa6361bdba41bdcb9906224
# Parent 809882b6f1f84a5673d0ea18dfd392bb35774741
libxl: enabling upstream qemu as pure pv backend.
This patch makes device_model_{version,override} work for pure pv
guest, so that users can specify upstream qemu as pure pv backend
other than traditional qemu-xen.
This patch also adds device_model_args_{pv,hvm} options for pv and
hvm guest respectively.
Internally, original libxl__create_xenpv_qemu allocates a new empty
dm_info (struct libxl_device_model_info) for every xenpv qemu created.
Now the caller of libxl__create_xenpv_qemu is responsible for
allocating this dm_info.
Signed-off-by: Wei Liu <liuw@xxxxxxxxx>
diff -r 809882b6f1f8 -r db9937db859a tools/libxl/libxl.idl
--- a/tools/libxl/libxl.idl Mon Jul 18 14:52:30 2011 +0100
+++ b/tools/libxl/libxl.idl Tue Jul 19 17:32:21 2011 +0800
@@ -238,6 +238,8 @@ The password never expires"""),
("vcpu_avail", integer, False, "vcpus actually available"),
("xen_platform_pci", bool, False, "enable/disable the xen
platform pci device"),
("extra", libxl_string_list, False, "extra parameters pass
directly to qemu, NULL terminated"),
+ ("extra_pv", libxl_string_list, False, "extra parameters pass
directly to qemu for PV guest, NULL terminated"),
+ ("extra_hvm", libxl_string_list, False, "extra parameters pass
directly to qemu for HVM guest, NULL terminated"),
],
comment=
"""Device Model information.
diff -r 809882b6f1f8 -r db9937db859a tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c Mon Jul 18 14:52:30 2011 +0100
+++ b/tools/libxl/libxl_create.c Tue Jul 19 17:32:21 2011 +0800
@@ -530,6 +530,7 @@ static int do_domain_create(libxl__gc *g
{
int need_qemu = 0;
libxl_device_console console;
+ libxl_device_model_info xenpv_dm_info;
for (i = 0; i < d_config->num_vfbs; i++) {
libxl_device_vfb_add(ctx, domid, &d_config->vfbs[i]);
@@ -550,8 +551,20 @@ static int do_domain_create(libxl__gc *g
libxl__device_console_add(gc, domid, &console, &state);
libxl_device_console_destroy(&console);
- if (need_qemu)
- libxl__create_xenpv_qemu(gc, domid, d_config->vfbs, &dm_starting);
+ if (need_qemu) {
+ /* only copy those useful configs */
+ memset((void*)&xenpv_dm_info, 0, sizeof(libxl_device_model_info));
+ xenpv_dm_info.device_model_version =
+ d_config->dm_info.device_model_version;
+ xenpv_dm_info.type = d_config->dm_info.type;
+ xenpv_dm_info.device_model = d_config->dm_info.device_model;
+ xenpv_dm_info.extra = d_config->dm_info.extra;
+ xenpv_dm_info.extra_pv = d_config->dm_info.extra_pv;
+ xenpv_dm_info.extra_hvm = d_config->dm_info.extra_hvm;
+
+ libxl__create_xenpv_qemu(gc, domid, &xenpv_dm_info,
+ d_config->vfbs, &dm_starting);
+ }
break;
}
default:
diff -r 809882b6f1f8 -r db9937db859a tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c Mon Jul 18 14:52:30 2011 +0100
+++ b/tools/libxl/libxl_dm.c Tue Jul 19 17:32:21 2011 +0800
@@ -206,9 +206,13 @@ static char ** libxl__build_device_model
switch (info->type) {
case LIBXL_DOMAIN_TYPE_PV:
flexarray_append(dm_args, "xenpv");
+ for (i = 0; info->extra_pv && info->extra_pv[i] != NULL; i++)
+ flexarray_append(dm_args, info->extra_pv[i]);
break;
case LIBXL_DOMAIN_TYPE_HVM:
flexarray_append(dm_args, "xenfv");
+ for (i = 0; info->extra_hvm && info->extra_hvm[i] != NULL; i++)
+ flexarray_append(dm_args, info->extra_hvm[i]);
break;
}
flexarray_append(dm_args, NULL);
@@ -403,9 +407,13 @@ static char ** libxl__build_device_model
switch (info->type) {
case LIBXL_DOMAIN_TYPE_PV:
flexarray_append(dm_args, "xenpv");
+ for (i = 0; info->extra_pv && info->extra_pv[i] != NULL; i++)
+ flexarray_append(dm_args, info->extra_pv[i]);
break;
case LIBXL_DOMAIN_TYPE_HVM:
flexarray_append(dm_args, "xenfv");
+ for (i = 0; info->extra_hvm && info->extra_hvm[i] != NULL; i++)
+ flexarray_append(dm_args, info->extra_hvm[i]);
break;
}
@@ -614,6 +622,7 @@ static int libxl__create_stubdom(libxl__
struct xs_permissions perm[2];
xs_transaction_t t;
libxl__device_model_starting *dm_starting = 0;
+ libxl_device_model_info xenpv_dm_info;
if (info->device_model_version !=
LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL) {
ret = ERROR_INVAL;
@@ -647,7 +656,7 @@ static int libxl__create_stubdom(libxl__
b_info.u.pv.features = "";
/* fixme: this function can leak the stubdom if it fails */
-
+ domid = 0;
ret = libxl__domain_make(gc, &c_info, &domid);
if (ret)
goto out_free;
@@ -742,7 +751,18 @@ retry_transaction:
if (ret)
goto out_free;
}
- if (libxl__create_xenpv_qemu(gc, domid, vfb, &dm_starting) < 0) {
+
+ memset((void*)&xenpv_dm_info, 0, sizeof(libxl_device_model_info));
+ xenpv_dm_info.device_model_version = info->device_model_version;
+ xenpv_dm_info.type = LIBXL_DOMAIN_TYPE_PV;
+ xenpv_dm_info.device_model = info->device_model;
+ xenpv_dm_info.extra = info->extra;
+ xenpv_dm_info.extra_pv = info->extra_pv;
+ xenpv_dm_info.extra_hvm = info->extra_hvm;
+
+ if (libxl__create_xenpv_qemu(gc, domid,
+ &xenpv_dm_info,
+ vfb, &dm_starting) < 0) {
ret = ERROR_FAIL;
goto out_free;
}
@@ -951,7 +971,6 @@ static int libxl__build_xenpv_qemu_args(
libxl_device_model_info *info)
{
libxl_ctx *ctx = libxl__gc_owner(gc);
- memset(info, 0x00, sizeof(libxl_device_model_info));
if (vfb != NULL) {
info->vnc = vfb->vnc;
@@ -969,9 +988,6 @@ static int libxl__build_xenpv_qemu_args(
info->nographic = 1;
info->domid = domid;
info->dom_name = libxl_domid_to_name(ctx, domid);
- info->device_model_version =
LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
- info->device_model = NULL;
- info->type = LIBXL_DOMAIN_TYPE_PV;
return 0;
}
@@ -1012,12 +1028,12 @@ out:
return ret;
}
-int libxl__create_xenpv_qemu(libxl__gc *gc, uint32_t domid, libxl_device_vfb
*vfb,
+int libxl__create_xenpv_qemu(libxl__gc *gc, uint32_t domid,
+ libxl_device_model_info *info,
+ libxl_device_vfb *vfb,
libxl__device_model_starting **starting_r)
{
- libxl_device_model_info info;
-
- libxl__build_xenpv_qemu_args(gc, domid, vfb, &info);
- libxl__create_device_model(gc, &info, NULL, 0, NULL, 0, starting_r);
+ libxl__build_xenpv_qemu_args(gc, domid, vfb, info);
+ libxl__create_device_model(gc, info, NULL, 0, NULL, 0, starting_r);
return 0;
}
diff -r 809882b6f1f8 -r db9937db859a tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h Mon Jul 18 14:52:30 2011 +0100
+++ b/tools/libxl/libxl_internal.h Tue Jul 19 17:32:21 2011 +0800
@@ -267,8 +267,10 @@ _hidden int libxl__create_device_model(l
libxl_device_disk *disk, int num_disks,
libxl_device_nic *vifs, int num_vifs,
libxl__device_model_starting **starting_r);
-_hidden int libxl__create_xenpv_qemu(libxl__gc *gc, uint32_t domid,
libxl_device_vfb *vfb,
- libxl__device_model_starting **starting_r);
+_hidden int libxl__create_xenpv_qemu(libxl__gc *gc, uint32_t domid,
+ libxl_device_model_info *dm_info,
+ libxl_device_vfb *vfb,
+ libxl__device_model_starting **starting_r);
_hidden int libxl__need_xenpv_qemu(libxl__gc *gc,
int nr_consoles, libxl_device_console *consoles,
int nr_vfbs, libxl_device_vfb *vfbs,
diff -r 809882b6f1f8 -r db9937db859a tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Mon Jul 18 14:52:30 2011 +0100
+++ b/tools/libxl/xl_cmdimpl.c Tue Jul 19 17:32:21 2011 +0800
@@ -529,6 +529,13 @@ static void parse_config_data(const char
int pci_msitranslate = 1;
int e;
+ XLU_ConfigList *dmargs;
+ int nr_dmargs = 0;
+ XLU_ConfigList *dmargs_hvm;
+ int nr_dmargs_hvm = 0;
+ XLU_ConfigList *dmargs_pv;
+ int nr_dmargs_pv = 0;
+
libxl_domain_create_info *c_info = &d_config->c_info;
libxl_domain_build_info *b_info = &d_config->b_info;
@@ -552,7 +559,7 @@ static void parse_config_data(const char
&c_info->ssidref);
if (e) {
if (errno == ENOSYS) {
- fprintf(stderr, "XSM Disabled: seclabel not supported\n");
+ fprintf(stderr, "XSM Disabled: seclabel not supported\n");
} else {
fprintf(stderr, "Invalid seclabel: %s\n", buf);
exit(1);
@@ -1014,41 +1021,72 @@ skip_vfb:
break;
}
+ /* init dm from c and b */
+ if (libxl_init_dm_info(ctx, dm_info, c_info, b_info))
+ exit(1);
+ /* parse device model arguments, this works for pv, hvm and stubdom */
+ if (!xlu_cfg_get_string (config, "device_model", &buf)) {
+ fprintf(stderr,
+ "WARNING: ignoring device_model directive.\n"
+ "WARNING: Use \"device_model_override\" instead if you"
+ " really want a non-default device_model\n");
+ if (strstr(buf, "stubdom-dm")) {
+ if (c_info->type == LIBXL_DOMAIN_TYPE_HVM)
+ fprintf(stderr, "WARNING: Or use"
+ " \"device_model_stubdomain_override\" if you "
+ " want to enable stubdomains\n");
+ else
+ fprintf(stderr, "WARNING: ignoring"
+ " \"device_model_stubdomain_override\" directive"
+ " for pv guest\n");
+ }
+ }
+
+
+ xlu_cfg_replace_string (config, "device_model_override",
+ &dm_info->device_model);
+ if (!xlu_cfg_get_string (config, "device_model_version", &buf)) {
+ if (!strcmp(buf, "qemu-xen-traditional")) {
+ dm_info->device_model_version
+ = LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
+ } else if (!strcmp(buf, "qemu-xen")) {
+ dm_info->device_model_version
+ = LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
+ } else {
+ fprintf(stderr,
+ "Unknown device_model_version \"%s\" specified\n", buf);
+ exit(1);
+ }
+ } else if (dm_info->device_model)
+ fprintf(stderr, "WARNING: device model override given without specific
DM version\n");
+ if (!xlu_cfg_get_long (config, "device_model_stubdomain_override", &l))
+ dm_info->device_model_stubdomain = l;
+
+#define parse_extra_args(type) \
+ if (!xlu_cfg_get_list(config, "device_model_args"#type, \
+ &dmargs##type, &nr_dmargs##type, 0)) \
+ { \
+ int i; \
+ dm_info->extra##type = \
+ xmalloc(sizeof(char*)*(nr_dmargs##type + 1)); \
+ dm_info->extra##type[nr_dmargs##type] = NULL; \
+ for (i=0; i<nr_dmargs##type; i++) { \
+ const char *a = xlu_cfg_get_listitem(dmargs##type, i); \
+ dm_info->extra##type[i] = a ? strdup(a) : NULL; \
+ } \
+ } \
+ /* parse extra args for qemu, common to both pv, hvm */
+ parse_extra_args();
+
+ /* parse extra args dedicated to pv */
+ parse_extra_args(_pv);
+
+ /* parse extra args dedicated to hvm */
+ parse_extra_args(_hvm);
+
+#undef parse_extra_args
+
if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) {
- XLU_ConfigList *dmargs;
- int nr_dmargs = 0;
-
- /* init dm from c and b */
- if (libxl_init_dm_info(ctx, dm_info, c_info, b_info))
- exit(1);
-
- /* then process config related to dm */
- if (!xlu_cfg_get_string (config, "device_model", &buf)) {
- fprintf(stderr,
- "WARNING: ignoring device_model directive.\n"
- "WARNING: Use \"device_model_override\" instead if you
really want a non-default device_model\n");
- if (strstr(buf, "stubdom-dm"))
- fprintf(stderr, "WARNING: Or use
\"device_model_stubdomain_override\" if you want to enable stubdomains\n");
- }
-
- xlu_cfg_replace_string (config, "device_model_override",
- &dm_info->device_model);
- if (!xlu_cfg_get_string (config, "device_model_version", &buf)) {
- if (!strcmp(buf, "qemu-xen-traditional")) {
- dm_info->device_model_version
- = LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
- } else if (!strcmp(buf, "qemu-xen")) {
- dm_info->device_model_version
- = LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
- } else {
- fprintf(stderr,
- "Unknown device_model_version \"%s\" specified\n",
buf);
- exit(1);
- }
- } else if (dm_info->device_model)
- fprintf(stderr, "WARNING: device model override given without
specific DM version\n");
- if (!xlu_cfg_get_long (config, "device_model_stubdomain_override", &l))
- dm_info->device_model_stubdomain = l;
if (!xlu_cfg_get_long (config, "stdvga", &l))
dm_info->stdvga = l;
if (!xlu_cfg_get_long (config, "vnc", &l))
@@ -1090,17 +1128,6 @@ skip_vfb:
xlu_cfg_replace_string (config, "soundhw", &dm_info->soundhw);
if (!xlu_cfg_get_long (config, "xen_platform_pci", &l))
dm_info->xen_platform_pci = l;
-
- if (!xlu_cfg_get_list(config, "device_model_args", &dmargs,
&nr_dmargs, 0))
- {
- int i;
- dm_info->extra = xmalloc(sizeof(char *) * (nr_dmargs + 1));
- dm_info->extra[nr_dmargs] = NULL;
- for (i=0; i<nr_dmargs; i++) {
- const char *a = xlu_cfg_get_listitem(dmargs, i);
- dm_info->extra[i] = a ? strdup(a) : NULL;
- }
- }
}
dm_info->type = c_info->type;
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home