qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* Re: [Qemu-devel] Environment variables for user-mode QEMU
@ 2013-04-25 12:22 Riku Voipio
  2013-04-30 12:17 ` Thomas Schwinge
  0 siblings, 1 reply; 15+ messages in thread
From: Riku Voipio @ 2013-04-25 12:22 UTC (permalink / raw)
  To: Thomas Schwinge, j.schauer
  Cc: peter.maydell, aliguori, sw, qemu-devel, agraf, paul

Bcc: 
Subject: Re: [Qemu-devel] Environment variables for user-mode QEMU
Reply-To: 
In-Reply-To: <87txmwoyqc.fsf@schwinge.name>
X-message-flag: Warning: message not sent with a DRM-Certified client 

On Wed, Apr 24, 2013 at 03:16:27PM +0200, Thomas Schwinge wrote:
> We have a need to pass environment variable assignments containing commas
> to user-mode QEMU.  The usage information currently says:
 
>     You can use -E and -U options or the QEMU_SET_ENV and
>     QEMU_UNSET_ENV environment variables to set and unset
>     environment variables for the target process.
>     It is possible to provide several variables by separating them
>     by commas in getsubopt(3) style. Additionally it is possible to
>     provide the -E and -U options multiple times.
 
>     $ env -i x86_64-linux-user/qemu-x86_64 -E x=y,y=z /usr/bin/printenv
>     y=z
>     x=y
> 
> Instead of this, we'd like to see:
> 
>     $ env -i x86_64-linux-user/qemu-x86_64 -E x=y,y=z /usr/bin/printenv
>     x=y,y=z
> 
> Due to the tokenization based on comma in
> linux-user/main.c:handle_arg_set_env, there is currently no way to
> achieve this -- other than pre-setting environment variables before
> running user-mode QEMU, which obviously isn't always desirable/possible
> (LD_LIBRARY_PATH, etc.).
> 
> Assuming there is a consensus, how would you like this implemented?
 
> Is it OK to change the semantics of -E (as well as -U, for symmetry?) to
> not handle multiple environment variable assignments (preliminary patch
> below), or does that functionality need to be preserved?  Something needs
> to be done about QEMU_SET_ENV and QEMU_UNSET_ENV then (if anyone is using
> these at all, which we can't disprove), as these are not very useful if
> they can handle only one environment variable.

We do not want to break backwards compatability. Also, adding an new
option to support passing commas would be ugly - and
we would have to support both options for a long time.

The only reason why we need the comma parsing is for the
QEMU_SET_ENV env variable, where the only way to set multiple env
variables is with commas.

However, it can be argued, that adding support for env variables 
in commit fc9c54124d134dbd76338a92a91804dab2df8166 broke passing commas
using the traditional "-E " parameters. 

So we could weasel ourself out of this mess by removing comma parsing when
setting env variables from "-E", while keep parsing when using QEMU_SET_ENV.

Riku

^ permalink raw reply	[flat|nested] 15+ messages in thread
* [Qemu-devel] Environment variables for user-mode QEMU
@ 2013-04-24 13:16 Thomas Schwinge
  2013-04-24 16:11 ` Thomas Schwinge
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Schwinge @ 2013-04-24 13:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: paul

[-- Attachment #1: Type: text/plain, Size: 3089 bytes --]

Hi!

We have a need to pass environment variable assignments containing commas
to user-mode QEMU.  The usage information currently says:

    You can use -E and -U options or the QEMU_SET_ENV and
    QEMU_UNSET_ENV environment variables to set and unset
    environment variables for the target process.
    It is possible to provide several variables by separating them
    by commas in getsubopt(3) style. Additionally it is possible to
    provide the -E and -U options multiple times.

    $ env -i x86_64-linux-user/qemu-x86_64 -E x=y,y=z /usr/bin/printenv
    y=z
    x=y

Instead of this, we'd like to see:

    $ env -i x86_64-linux-user/qemu-x86_64 -E x=y,y=z /usr/bin/printenv
    x=y,y=z

Due to the tokenization based on comma in
linux-user/main.c:handle_arg_set_env, there is currently no way to
achieve this -- other than pre-setting environment variables before
running user-mode QEMU, which obviously isn't always desirable/possible
(LD_LIBRARY_PATH, etc.).

Assuming there is a consensus, how would you like this implemented?

Is it OK to change the semantics of -E (as well as -U, for symmetry?) to
not handle multiple environment variable assignments (preliminary patch
below), or does that functionality need to be preserved?  Something needs
to be done about QEMU_SET_ENV and QEMU_UNSET_ENV then (if anyone is using
these at all, which we can't disprove), as these are not very useful if
they can handle only one environment variable.

Or, should we perhaps have a new -env option that causes any later
non-option arguments, that contain an equal sign, to be handled as
environment variable assignments, just like the env command does?

    $ env -i x86_64-linux-user/qemu-x86_64 [some options] -env [more options] a=b,c d=e,f=a /usr/bin/printenv
    a=b,c
    d=e,f=a

I think I might prefer that solution.  As it is a new option, it also
won't interfere with anyone's usage/expectations of the current behavior.
Anyway, here is the incomplete patch, changing the behavior of -E and -U
(as well as the corresponding QEMU_SET_ENV and QEMU_UNSET_ENV):

diff --git linux-user/main.c linux-user/main.c
index 4e92a0b..62ff963 100644
--- linux-user/main.c
+++ linux-user/main.c
@@ -3204,26 +3204,16 @@ static void handle_arg_log_filename(const char *arg)
 
 static void handle_arg_set_env(const char *arg)
 {
-    char *r, *p, *token;
-    r = p = strdup(arg);
-    while ((token = strsep(&p, ",")) != NULL) {
-        if (envlist_setenv(envlist, token) != 0) {
-            usage();
-        }
+    if (envlist_setenv(envlist, arg) != 0) {
+        usage();
     }
-    free(r);
 }
 
 static void handle_arg_unset_env(const char *arg)
 {
-    char *r, *p, *token;
-    r = p = strdup(arg);
-    while ((token = strsep(&p, ",")) != NULL) {
-        if (envlist_unsetenv(envlist, token) != 0) {
-            usage();
-        }
+    if (envlist_unsetenv(envlist, arg) != 0) {
+        usage();
     }
-    free(r);
 }
 
 static void handle_arg_argv0(const char *arg)


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]

^ permalink raw reply related	[flat|nested] 15+ messages in thread
end of thread, other threads:[~2013-04-30 12:17 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-25 12:22 [Qemu-devel] Environment variables for user-mode QEMU Riku Voipio
2013-04-30 12:17 ` Thomas Schwinge
  -- strict thread matches above, loose matches on Subject: below --
2013-04-24 13:16 Thomas Schwinge
2013-04-24 16:11 ` Thomas Schwinge
2013-04-25 15:06   ` [Qemu-devel] [PATCH 1/4] util/envlist: Properly forward a callback's error in envlist_parse Thomas Schwinge
2013-04-25 15:06     ` [Qemu-devel] [PATCH 2/4] linux-user: Use existing envlist_parse_set/envlist_parse_unset interface Thomas Schwinge
2013-04-25 15:06     ` [Qemu-devel] [PATCH 3/4] linux-user: Tell handler_arg_* which context they're invoked from Thomas Schwinge
2013-04-25 15:06     ` [Qemu-devel] [PATCH 4/4] linux-user: Restore original behavior of the -E and -U command-line options Thomas Schwinge
2013-04-25 15:52       ` Peter Maydell
2013-04-25 16:18         ` Thomas Schwinge
2013-04-25 16:21           ` Peter Maydell
2013-04-25 16:41             ` [Qemu-devel] [PATCH] qemu-doc: Option -ignore-environment removed Thomas Schwinge
2013-04-25 17:00               ` Peter Maydell
2013-04-26 10:52               ` Stefan Hajnoczi
2013-04-25 16:37       ` [Qemu-devel] [PATCH v2] linux-user: Restore original behavior of the -E and -U command-line options Thomas Schwinge

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).