* [PATCH] xl: Don't require a config file for cpupools
@ 2012-04-03 18:29 George Dunlap
2012-04-04 9:12 ` Ian Campbell
2012-04-23 15:25 ` Ian Jackson
0 siblings, 2 replies; 4+ messages in thread
From: George Dunlap @ 2012-04-03 18:29 UTC (permalink / raw)
To: xen-devel; +Cc: george.dunlap
Since the key information can be fairly simply put on the command-line,
there's no need to require an actual config file.
Also improve the help to cross-reference the xlcpupool.cfg manpage.
Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
diff -r 30cc13e25e01 -r 0fb728d56bae docs/man/xl.pod.1
--- a/docs/man/xl.pod.1 Tue Apr 03 19:02:19 2012 +0100
+++ b/docs/man/xl.pod.1 Tue Apr 03 19:28:04 2012 +0100
@@ -848,11 +848,13 @@ Cpu-pools can be specified either by nam
=over 4
-=item B<cpupool-create> [I<OPTIONS>] I<ConfigFile> [I<Variable=Value> ...]
+=item B<cpupool-create> [I<OPTIONS>] [I<ConfigFile>] [I<Variable=Value> ...]
-Create a cpu pool based an I<ConfigFile>.
+Create a cpu pool based an config from a I<ConfigFile> or command-line parameters.
Variable settings from the I<ConfigFile> may be altered by specifying new
-or additional assignments on the command line.
+or additional assignments on the command line.
+
+See the xlcpupool.cfg manpage for more information.
B<OPTIONS>
diff -r 30cc13e25e01 -r 0fb728d56bae tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Tue Apr 03 19:02:19 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c Tue Apr 03 19:28:04 2012 +0100
@@ -5407,7 +5407,7 @@ int main_tmem_freeable(int argc, char **
int main_cpupoolcreate(int argc, char **argv)
{
- const char *filename = NULL;
+ const char *filename = NULL, *config_src=NULL;
const char *p;
char extra_config[1024];
int opt;
@@ -5471,23 +5471,26 @@ int main_cpupoolcreate(int argc, char **
optind++;
}
- if (!filename) {
- help("cpupool-create");
- return -ERROR_FAIL;
- }
-
- if (libxl_read_file_contents(ctx, filename, (void **)&config_data, &config_len)) {
- fprintf(stderr, "Failed to read config file: %s: %s\n",
- filename, strerror(errno));
- return -ERROR_FAIL;
- }
+ if (filename)
+ {
+ if (libxl_read_file_contents(ctx, filename, (void **)&config_data,
+ &config_len)) {
+ fprintf(stderr, "Failed to read config file: %s: %s\n",
+ filename, strerror(errno));
+ return -ERROR_FAIL;
+ }
+ config_src=filename;
+ }
+ else
+ config_src="command line";
+
if (strlen(extra_config)) {
if (config_len > INT_MAX - (strlen(extra_config) + 2)) {
fprintf(stderr, "Failed to attach extra configration\n");
return -ERROR_FAIL;
}
config_data = xrealloc(config_data,
- config_len + strlen(extra_config) + 2);
+ config_len + strlen(extra_config) + 2);
if (!config_data) {
fprintf(stderr, "Failed to realloc config_data\n");
return -ERROR_FAIL;
@@ -5498,7 +5501,7 @@ int main_cpupoolcreate(int argc, char **
config_len += strlen(extra_config) + 1;
}
- config = xlu_cfg_init(stderr, filename);
+ config = xlu_cfg_init(stderr, config_src);
if (!config) {
fprintf(stderr, "Failed to allocate for configuration\n");
return -ERROR_FAIL;
@@ -5512,8 +5515,12 @@ int main_cpupoolcreate(int argc, char **
if (!xlu_cfg_get_string (config, "name", &buf, 0))
name = strdup(buf);
- else
+ else if (filename)
name = libxl_basename(filename);
+ else {
+ fprintf(stderr, "Missing cpupool name!\n");
+ return -ERROR_FAIL;
+ }
if (!libxl_name_to_cpupoolid(ctx, name, &poolid)) {
fprintf(stderr, "Pool name \"%s\" already exists\n", name);
return -ERROR_FAIL;
@@ -5595,7 +5602,7 @@ int main_cpupoolcreate(int argc, char **
libxl_uuid_generate(&uuid);
- printf("Using config file \"%s\"\n", filename);
+ printf("Using config file \"%s\"\n", config_src);
printf("cpupool name: %s\n", name);
printf("scheduler: %s\n", libxl_scheduler_to_string(sched));
printf("number of cpus: %d\n", n_cpus);
diff -r 30cc13e25e01 -r 0fb728d56bae tools/libxl/xl_cmdtable.c
--- a/tools/libxl/xl_cmdtable.c Tue Apr 03 19:02:19 2012 +0100
+++ b/tools/libxl/xl_cmdtable.c Tue Apr 03 19:28:04 2012 +0100
@@ -364,12 +364,14 @@ struct cmd_spec cmd_table[] = {
},
{ "cpupool-create",
&main_cpupoolcreate, 1,
- "Create a CPU pool based an ConfigFile",
- "[options] <ConfigFile> [vars]",
+ "Create a new CPU pool",
+ "[options] [<ConfigFile>] [Variable=value ...]",
"-h, --help Print this help.\n"
"-f FILE, --defconfig=FILE Use the given configuration file.\n"
"-n, --dryrun Dry run - prints the resulting configuration.\n"
- " (deprecated in favour of global -N option)."
+ " (deprecated in favour of global -N option).\n"
+ "\nSee the xlcpupool.cfg manpage for more information.",
+
},
{ "cpupool-list",
&main_cpupoollist, 0,
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xl: Don't require a config file for cpupools
2012-04-03 18:29 [PATCH] xl: Don't require a config file for cpupools George Dunlap
@ 2012-04-04 9:12 ` Ian Campbell
2012-04-23 15:25 ` Ian Jackson
1 sibling, 0 replies; 4+ messages in thread
From: Ian Campbell @ 2012-04-04 9:12 UTC (permalink / raw)
To: George Dunlap; +Cc: xen-devel
On Tue, 2012-04-03 at 19:29 +0100, George Dunlap wrote:
> Since the key information can be fairly simply put on the command-line,
> there's no need to require an actual config file.
>
> Also improve the help to cross-reference the xlcpupool.cfg manpage.
>
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
Code looks good to me, couple of comments on the docs aspects below.
> diff -r 30cc13e25e01 -r 0fb728d56bae docs/man/xl.pod.1
> --- a/docs/man/xl.pod.1 Tue Apr 03 19:02:19 2012 +0100
> +++ b/docs/man/xl.pod.1 Tue Apr 03 19:28:04 2012 +0100
> @@ -848,11 +848,13 @@ Cpu-pools can be specified either by nam
>
> =over 4
>
> -=item B<cpupool-create> [I<OPTIONS>] I<ConfigFile> [I<Variable=Value> ...]
> +=item B<cpupool-create> [I<OPTIONS>] [I<ConfigFile>] [I<Variable=Value> ...]
>
> -Create a cpu pool based an I<ConfigFile>.
> +Create a cpu pool based an config from a I<ConfigFile> or command-line parameters.
> Variable settings from the I<ConfigFile> may be altered by specifying new
> -or additional assignments on the command line.
> +or additional assignments on the command line.
Could do with rewrapping? Or a blank line between the paras if that's
what they are meant to be (that's prexisting in that case)
> +
> +See the xlcpupool.cfg manpage for more information.
I think this should be "L<xlcpupool.cfg(N)>" in here. (N for whichever
section the manpage is in).
> diff -r 30cc13e25e01 -r 0fb728d56bae tools/libxl/xl_cmdtable.c
> --- a/tools/libxl/xl_cmdtable.c Tue Apr 03 19:02:19 2012 +0100
> +++ b/tools/libxl/xl_cmdtable.c Tue Apr 03 19:28:04 2012 +0100
> @@ -364,12 +364,14 @@ struct cmd_spec cmd_table[] = {
> },
> { "cpupool-create",
> &main_cpupoolcreate, 1,
> - "Create a CPU pool based an ConfigFile",
> - "[options] <ConfigFile> [vars]",
> + "Create a new CPU pool",
> + "[options] [<ConfigFile>] [Variable=value ...]",
> "-h, --help Print this help.\n"
> "-f FILE, --defconfig=FILE Use the given configuration file.\n"
> "-n, --dryrun Dry run - prints the resulting configuration.\n"
> - " (deprecated in favour of global -N option)."
> + " (deprecated in favour of global -N option).\n"
> + "\nSee the xlcpupool.cfg manpage for more information.",
xlcpupool.cfg(N) here too.
> +
> },
> { "cpupool-list",
> &main_cpupoollist, 0,
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xl: Don't require a config file for cpupools
2012-04-03 18:29 [PATCH] xl: Don't require a config file for cpupools George Dunlap
2012-04-04 9:12 ` Ian Campbell
@ 2012-04-23 15:25 ` Ian Jackson
2012-04-25 10:50 ` George Dunlap
1 sibling, 1 reply; 4+ messages in thread
From: Ian Jackson @ 2012-04-23 15:25 UTC (permalink / raw)
To: George Dunlap; +Cc: xen-devel
George Dunlap writes ("[Xen-devel] [PATCH] xl: Don't require a config file for cpupools"):
> Since the key information can be fairly simply put on the command-line,
> there's no need to require an actual config file.
Thanks. Just a few tiny coding style nits. I agree with Ian
Campbell's comments, and also:
> - const char *filename = NULL;
> + const char *filename = NULL, *config_src=NULL;
This has inconsistent use of whitespace.
> + if (filename)
> + {
This should have { on the same line as the if.
> + if (libxl_read_file_contents(ctx, filename, (void **)&config_data,
> + &config_len)) {
> + fprintf(stderr, "Failed to read config file: %s: %s\n",
> + filename, strerror(errno));
> + return -ERROR_FAIL;
> + }
> + config_src=filename;
We put spaces around "=". (Here and a few lines further on.)
> + }
> + else
And on the same line as the else. And having { } for the if, I think
putting { } for the else block would be more conventional.
> - printf("Using config file \"%s\"\n", filename);
> + printf("Using config file \"%s\"\n", config_src);
This will print
Using config file "command line"
which is rather an odd message. Perhaps change the string to
`<command line>' and remove the quotes ?
Ian.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xl: Don't require a config file for cpupools
2012-04-23 15:25 ` Ian Jackson
@ 2012-04-25 10:50 ` George Dunlap
0 siblings, 0 replies; 4+ messages in thread
From: George Dunlap @ 2012-04-25 10:50 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel
On Mon, Apr 23, 2012 at 4:25 PM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
>> - printf("Using config file \"%s\"\n", filename);
>> + printf("Using config file \"%s\"\n", config_src);
>
> This will print
> Using config file "command line"
> which is rather an odd message. Perhaps change the string to
> `<command line>' and remove the quotes ?
Changing the string to <command line> makes sense. I think the point
of putting quotes there is to avoid getting strange results if your
filenames have spaces. But I suppose if you're strange enough to be
using spaces in your filenames, you should be able to figure out
what's going on yourself. :-)
New patch on the way...
-George
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-04-25 10:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-03 18:29 [PATCH] xl: Don't require a config file for cpupools George Dunlap
2012-04-04 9:12 ` Ian Campbell
2012-04-23 15:25 ` Ian Jackson
2012-04-25 10:50 ` George Dunlap
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.