[libvirt] [PATCH] Split contents of <cmdline>...</cmdline> and set LXC init argv

[Daniel P. Berrange]

On Wed, Mar 14, 2012 at 11:22:35AM -0600, Eric Blake wrote:
> On 03/14/2012 09:24 AM, Daniel P. Berrange wrote:
> > From: "Daniel P. Berrange" <berrange at redhat.com>
> > 
> > Currently for LXC we set LIBVIRT_LXC_CMDLINE to contain the
> > contents of <cmdline>...</cmdline>. It is more convenient
> > if we just set the argv[] of the init binary directly though.
> > 
> > * lxc/lxc_container.c: Set init argv from cmdline
> > ---
> >  src/lxc/lxc_container.c |   12 ++++++++++++
> >  1 files changed, 12 insertions(+), 0 deletions(-)
> > 
> > diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
> > index d827b35..93dfb86 100644
> > --- a/src/lxc/lxc_container.c
> > +++ b/src/lxc/lxc_container.c
> > @@ -117,11 +117,19 @@ static virCommandPtr lxcContainerBuildInitCmd(virDomainDefPtr vmDef)
> >  {
> >      char uuidstr[VIR_UUID_STRING_BUFLEN];
> >      virCommandPtr cmd;
> > +    char **args = NULL;
> > +    size_t i;
> > +
> > +    if (vmDef->os.cmdline &&
> > +        !(args = virStrSplitQuoted(vmDef->os.cmdline, " \t")))
> > +        return NULL;
> 
> This part may change, depending on what we do in the previous patch
> (yeah, this should have been sent as a two-part series) - for example,
> we may decide to add a virStrSplitShell(), then we don't even have to
> pass in the " \t" string of separators, but can instead rely on
> virStrSplitShell that takes a single flat arg and reliably splits it out
> in the same way as the shell and virsh does it.  But once you have the
> split args,
> 
> ACK to the rest of the patch.

Hmm, examining things more closely, I think this patch is terminally
flawed.

 - SystemD parses /proc/cmdline splitting on whitespace, allowing
   quoting using ' or ". Any arg from /proc/cmdline can be passed
   to init via argv[] too

 - SysVInit parses /proc/cmdline, splitting on whitespace, with
   no quoting method. It only looks for 'console=' args though.
   It does not accept of the args via argv[] either

   It relies on rc.sysinit to process other /proc/cmdline args
   using  'for ARG in $(cat /proc/cmdline)' which relies on
   the shell IFS

 - upstart does not parse /proc/cmdline at all, and ignores any
   argv{]

   It relies on rc-sysinit.conf to process other /proc/cmdline
   args using  'for ARG in $(cat /proc/cmdline)' which relies on
   the shell IFS


In other words, I don't think it is acceptable to automagically
split  '<cmdline>' and pass them as argv[]. We should only
use it for setting LIBVIRT_LXC_CMDLINE.

We should add a separate <initarg> element for setting init's
argv[] I think

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|