| Message ID | 20190805144910.20223-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tools/xl: Make extra= usable in combination with cmdline= | expand |
On Mon, Aug 05, 2019 at 03:49:10PM +0100, Andrew Cooper wrote: > c/s 986aea7fbe "xl.cfg: add 'cmdline' in config file" introduced cmdline= and > make extra= and root= unusable if cmdline= was present. > > For the vm.cfg file itself, this makes sense. However, for development > purposes it is very convenient to have a cmdline= in the cfg file, and specify > extra= on the `xl create` command line. > > While updating the manpage, correct the entry for cmdline=. There is never > any appending which goes on with this option, but after this change extra= may > be appended to cmdline=. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > CC: Ian Jackson <Ian.Jackson@citrix.com> > CC: Wei Liu <wl@xen.org> > CC: Anthony PERARD <anthony.perard@citrix.com> > > In particular, this is useful for testing Xen as a PVH guest, where switching > between a PV and PVH dom0 involves adding "dom0=pvh dom0-iommu=none" to > command line. Now, given a single build of Xen under test, two tests can be > run with just > > # xl create shim.cfg ramdisk=\"test-pv64-example\" > > and > > # xl create shim.cfg ramdisk=\"test-hvm64-example\" extra=\"dom0=pvh\ dom0-iommu=none\" > > without needing to edit shim.cfg inbetween. :(, this feels like the wrong approach. It is time to invent += ? So we can do cmdline+="x", disk+=['xvdz'], ... :-). The `cmdline' option have been there for sometime, so hopefully no one has both `cmdline' and `extra' in there config file and changing the meaning of extra would be ok. With that in mind, the patch looks fine, Reviewed-by: Anthony PERARD <anthony.perard@citrix.com> Thanks,
Anthony PERARD writes ("Re: [PATCH] tools/xl: Make extra= usable in combination with cmdline="):
> On Mon, Aug 05, 2019 at 03:49:10PM +0100, Andrew Cooper wrote:
> > # xl create shim.cfg ramdisk=\"test-hvm64-example\" extra=\"dom0=pvh\ dom0-iommu=none\"
> >
> > without needing to edit shim.cfg inbetween.
>
> :(, this feels like the wrong approach. It is time to invent += ? So we
> can do cmdline+="x", disk+=['xvdz'], ... :-).
+1
Ian.
diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in index c99d40307e..ca34f3c623 100644 --- a/docs/man/xl.cfg.5.pod.in +++ b/docs/man/xl.cfg.5.pod.in @@ -431,18 +431,24 @@ Load the specified file as the ramdisk. =item B<cmdline="STRING"> -Append B<STRING> to the kernel command line. (Note: the meaning of -this is guest specific). It can replace B<root="STRING"> -along with B<extra="STRING"> and is preferred. When B<cmdline="STRING"> is set, -B<root="STRING"> and B<extra="STRING"> will be ignored. +Use B<STRING> as the kernel command line. (Note: the meaning of this is guest +specific). + +When this option is specified, B<root="STRING"> will be ignored. =item B<root="STRING"> +This option is deprecated and its use is discouraged. It will be ignored if +B<cmdline=> is specified. + Append B<root=STRING> to the kernel command line (Note: the meaning of this is guest specific). =item B<extra="STRING"> +This option is deprecated and its use in configuration files is discouraged. +It may however be useful to use on the command line for B<xl create>. + Append B<STRING> to the kernel command line. (Note: the meaning of this is guest specific). diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c index e105bda2bb..2cad5c6e08 100644 --- a/tools/xl/xl_parse.c +++ b/tools/xl/xl_parse.c @@ -287,10 +287,14 @@ static char *parse_cmdline(XLU_Config *config) xlu_cfg_get_string (config, "extra", &extra, 0); if (buf) { - cmdline = strdup(buf); - if (root || extra) - fprintf(stderr, "Warning: ignoring root= and extra= " - "in favour of cmdline=\n"); + if (root) + fprintf(stderr, "Warning: ignoring root= in favour of cmdline=\n"); + + if (extra) { + xasprintf(&cmdline, "%s %s", buf, extra); + } else { + cmdline = strdup(buf); + } } else { if (root && extra) { xasprintf(&cmdline, "root=%s %s", root, extra);
c/s 986aea7fbe "xl.cfg: add 'cmdline' in config file" introduced cmdline= and make extra= and root= unusable if cmdline= was present. For the vm.cfg file itself, this makes sense. However, for development purposes it is very convenient to have a cmdline= in the cfg file, and specify extra= on the `xl create` command line. While updating the manpage, correct the entry for cmdline=. There is never any appending which goes on with this option, but after this change extra= may be appended to cmdline=. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Ian Jackson <Ian.Jackson@citrix.com> CC: Wei Liu <wl@xen.org> CC: Anthony PERARD <anthony.perard@citrix.com> In particular, this is useful for testing Xen as a PVH guest, where switching between a PV and PVH dom0 involves adding "dom0=pvh dom0-iommu=none" to command line. Now, given a single build of Xen under test, two tests can be run with just # xl create shim.cfg ramdisk=\"test-pv64-example\" and # xl create shim.cfg ramdisk=\"test-hvm64-example\" extra=\"dom0=pvh\ dom0-iommu=none\" without needing to edit shim.cfg inbetween. --- docs/man/xl.cfg.5.pod.in | 14 ++++++++++---- tools/xl/xl_parse.c | 12 ++++++++---- 2 files changed, 18 insertions(+), 8 deletions(-)