Message ID | 20170907101642.15782-5-roger.pau@citrix.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Roger Pau Monne writes ("[PATCH v2 04/21] xl: introduce a domain type option"): > Introduce a new type option to xl configuration files in order to > specify the domain type. This supersedes the current builder option. ...> > Th > libxl_defbool_set(&c_info->run_hotplug_scripts, run_hotplug_scripts); > c_info->type = LIBXL_DOMAIN_TYPE_PV; > - if (!xlu_cfg_get_string (config, "builder", &buf, 0) && > - !strncmp(buf, "hvm", strlen(buf))) > - c_info->type = LIBXL_DOMAIN_TYPE_HVM; > + if (!xlu_cfg_get_string (config, "builder", &buf, 0)) { > + fprintf(stderr, > + "The builder option is being deprecated, please use type instead.\n"); Line length. Probably best to shuffle the message to the left, rather than linewrapping, for ease of grepping. The error message would benefit from some `quotes' around the parameter names. It would be nice if you did this in a way that meant that a config file which specified both `type="hvm"' and `builder="hvm"' did not generate a warning. And it ought to be an error to specify `type="pv"' with `builder="hvm"' surely. Ian.
On Wed, Sep 20, 2017 at 03:50:48PM +0100, Ian Jackson wrote: > Roger Pau Monne writes ("[PATCH v2 04/21] xl: introduce a domain type option"): > > Introduce a new type option to xl configuration files in order to > > specify the domain type. This supersedes the current builder option. > ...> > > Th > > libxl_defbool_set(&c_info->run_hotplug_scripts, run_hotplug_scripts); > > c_info->type = LIBXL_DOMAIN_TYPE_PV; > > - if (!xlu_cfg_get_string (config, "builder", &buf, 0) && > > - !strncmp(buf, "hvm", strlen(buf))) > > - c_info->type = LIBXL_DOMAIN_TYPE_HVM; > > + if (!xlu_cfg_get_string (config, "builder", &buf, 0)) { > > + fprintf(stderr, > > + "The builder option is being deprecated, please use type instead.\n"); > > Line length. Probably best to shuffle the message to the left, rather > than linewrapping, for ease of grepping. > > The error message would benefit from some `quotes' around the > parameter names. > > It would be nice if you did this in a way that meant that a config > file which specified both `type="hvm"' and `builder="hvm"' did not > generate a warning. Hm, really? I can do that but it seems weird because it doesn't promote deprecation. Thanks, Roger.
Roger Pau Monné writes ("Re: [PATCH v2 04/21] xl: introduce a domain type option"): > On Wed, Sep 20, 2017 at 03:50:48PM +0100, Ian Jackson wrote: > > It would be nice if you did this in a way that meant that a config > > file which specified both `type="hvm"' and `builder="hvm"' did not > > generate a warning. > > Hm, really? I can do that but it seems weird because it doesn't > promote deprecation. It does promote deprecation, surely. Such a config file is compatible with all past and all planned future versions of Xen. In particular, if we stop honouring builder= at all, it will still work. Ian.
diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in index 79cb2eaea7..ab53436da2 100644 --- a/docs/man/xl.cfg.pod.5.in +++ b/docs/man/xl.cfg.pod.5.in @@ -54,9 +54,9 @@ Pairs may be separated either by a newline or a semicolon. Both of the following are valid: name="h0" - builder="hvm" + type="hvm" - name="h0"; builder="hvm" + name="h0"; type="hvm" =head1 OPTIONS @@ -77,6 +77,24 @@ single host must be unique. =over 4 +=item B<type="pv"> + +Specifies that this is to be a PV domain, suitable for hosting Xen-aware guest +operating systems. This is the default. + +=item B<type="hvm"> + +Specifies that this is to be an HVM domain. That is, a fully virtualised +computer with emulated BIOS, disk and network peripherals, etc. + +=back + +=head3 Deprecated guest type selection + +Note that the builder option is being deprecated in favor of the type option. + +=over 4 + =item B<builder="generic"> Specifies that this is to be a PV domain, suitable for hosting Xen-aware guest diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c index 61f9a38573..65297352bd 100644 --- a/tools/xl/xl_parse.c +++ b/tools/xl/xl_parse.c @@ -854,9 +854,23 @@ void parse_config_data(const char *config_source, libxl_defbool_set(&c_info->run_hotplug_scripts, run_hotplug_scripts); c_info->type = LIBXL_DOMAIN_TYPE_PV; - if (!xlu_cfg_get_string (config, "builder", &buf, 0) && - !strncmp(buf, "hvm", strlen(buf))) - c_info->type = LIBXL_DOMAIN_TYPE_HVM; + if (!xlu_cfg_get_string (config, "builder", &buf, 0)) { + fprintf(stderr, + "The builder option is being deprecated, please use type instead.\n"); + if (!strncmp(buf, "hvm", strlen(buf))) + c_info->type = LIBXL_DOMAIN_TYPE_HVM; + } + + if (!xlu_cfg_get_string (config, "type", &buf, 0)) { + if (!strncmp(buf, "hvm", strlen(buf))) { + c_info->type = LIBXL_DOMAIN_TYPE_HVM; + } else if (!strncmp(buf, "pv", strlen(buf))) { + c_info->type = LIBXL_DOMAIN_TYPE_PV; + } else { + fprintf(stderr, "Invalid domain type %s.\n", buf); + exit(1); + } + } xlu_cfg_get_defbool(config, "hap", &c_info->hap, 0);