Re: [Xen-devel] [PATCH] xl: Don't require a config file for cpupools

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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@xxxxxxxxxxxxx>

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@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel