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