[libvirt] [PATCH] Split contents of <cmdline>...</cmdline> and set LXC init argv
From: "Daniel P. Berrange" <berrange@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; virUUIDFormat(vmDef->uuid, uuidstr); cmd = virCommandNew(vmDef->os.init); + virCommandAddArgSet(cmd, (const char **)args); + virCommandAddEnvString(cmd, "PATH=/bin:/sbin"); virCommandAddEnvString(cmd, "TERM=linux"); virCommandAddEnvString(cmd, "container=lxc-libvirt"); @@ -131,6 +139,10 @@ static virCommandPtr lxcContainerBuildInitCmd(virDomainDefPtr vmDef) if (vmDef->os.cmdline) virCommandAddEnvPair(cmd, "LIBVIRT_LXC_CMDLINE", vmDef->os.cmdline); + for (i = 0 ; args[i] ; i++) + VIR_FREE(args[i]); + VIR_FREE(args); + return cmd; } -- 1.7.7.6
On 03/14/2012 09:24 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@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. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
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@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 :|
On 03/14/2012 11:53 AM, Daniel P. Berrange wrote:
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
as in: <cmdline>complex value that we won't split</cmdline> <initarg>arg1</initarg> <initarg>arg2, with funky '"\ stuff</initarg> Yes, that sounds like a more sure-fire way of letting the user specify exactly what they wanted, and nicer for XPath queries too. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Wed, Mar 14, 2012 at 12:00:04PM -0600, Eric Blake wrote:
On 03/14/2012 11:53 AM, Daniel P. Berrange wrote:
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
as in:
<cmdline>complex value that we won't split</cmdline> <initarg>arg1</initarg> <initarg>arg2, with funky '"\ stuff</initarg>
Yes, that sounds like a more sure-fire way of letting the user specify exactly what they wanted, and nicer for XPath queries too.
Yes, exactly. 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 :|
participants (2)
-
Daniel P. Berrange -
Eric Blake