Debian Bug report logs - #632192
qemu: get elf interpreter prefix from environment variable

version graph

Package: qemu; Maintainer for qemu is Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>; Source for qemu is src:qemu.

Reported by: Johannes Schauer <j.schauer@email.de>

Date: Thu, 30 Jun 2011 13:06:02 UTC

Severity: wishlist

Tags: patch

Found in version qemu/0.14.0+dfsg-5.1

Fixed in version 1.0~rc4+dfsg-1

Done: Vagrant Cascadian <vagrant@freegeek.org>

Bug is archived. No further changes may be made.

Toggle useless messages

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to debian-bugs-dist@lists.debian.org, Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>:
Bug#632192; Package qemu. (Thu, 30 Jun 2011 13:06:05 GMT) Full text and rfc822 format available.

Acknowledgement sent to Johannes Schauer <j.schauer@email.de>:
New Bug report received and forwarded. Copy sent to Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>. (Thu, 30 Jun 2011 13:06:05 GMT) Full text and rfc822 format available.

Message #5 received at submit@bugs.debian.org (full text, mbox):

From: Johannes Schauer <j.schauer@email.de>
To: Debian Bug Tracking System <submit@bugs.debian.org>
Subject: qemu: get elf interpreter prefix from environment variable
Date: Thu, 30 Jun 2011 15:02:55 +0200
Package: qemu
Version: 0.14.0+dfsg-5.1
Severity: wishlist

Using qemu-user-static, it is possible to build a whole foreign debian rootfs.

With the help of fakeroot and fakechroot this is even possible without
having to resort to becoming the superuser.

To execute foreign binaries, shared libraries in /etc/qemu-binfmt are
necessary.

A problem is that, one has to change those libraries according to what
binaries one is executing: for example the debian architectures arm,
armel and armhf are all using the /etc/qemu-binfmt/arm directory for
shared libraries but they require different library versions (arm,
armel, armhf) according to what code is executed. Changing those
libraries to whatever one wants to execute is cumbersome and requires
superuser permissions and by that obliterating above advantage.

One could supply the -L parameter to supply a custom elf interpreter
prefix but during the creation of a rootfs, qemu-user-static binaries
are called in a transparent manner by the binfmt mechanism. This makes
it impossible to hand over commandline options to qemu-user-static.

Adding support of an environment variable to specify the elf interpreter
prefix would solve all above issues.

Even more so, during the creation of a foreign rootfs, the fitting
libraries would already have been unpacked before any binary needs to be
executed.

This is how it would ideally look like for an armel rootfs:

export QEMU_LD_PREFIX=/tmp/debian-sid-armel/
fakeroot fakechroot chroot /tmp/debian-armel/ dpkg --configure -a

I hope it is clear what I tried to point out :)

cheers, josch




Information forwarded to debian-bugs-dist@lists.debian.org, Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>:
Bug#632192; Package qemu. (Thu, 30 Jun 2011 21:24:16 GMT) Full text and rfc822 format available.

Acknowledgement sent to Geert Stappers <stappers@stappers.nl>:
Extra info received and forwarded to list. Copy sent to Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>. (Thu, 30 Jun 2011 21:24:16 GMT) Full text and rfc822 format available.

Message #10 received at 632192@bugs.debian.org (full text, mbox):

From: Geert Stappers <stappers@stappers.nl>
To: Johannes Schauer <j.schauer@email.de>
Cc: 632192@bugs.debian.org, debian-embedded@lists.debian.org
Subject: FWIW debian/qemu-user-static.README.Debian
Date: Thu, 30 Jun 2011 23:15:09 +0200
[Message part 1 (text/plain, inline)]
For What it's Worth:
In the file debian/qemu-user-static.README.Debian
is this text
* Configuring qemu-user-static to use in a foreign chroot
  When used with binfmt, qemu-user-ARCH-static can be configured to run
  a foreign chroot. The qemu-user-ARCH-static binary should be copied 
  (or better hard linked) into the chroot in /usr/bin. Then the chroot 
  can be entered and used like a normal chroot.

I haven't played with it, but it seems to help for "polystrap"


Cheers
Geert Stappers

[signature.asc (application/pgp-signature, inline)]

Information forwarded to debian-bugs-dist@lists.debian.org, Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>:
Bug#632192; Package qemu. (Thu, 30 Jun 2011 22:42:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Geert Stappers <stappers@stappers.nl>:
Extra info received and forwarded to list. Copy sent to Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>. (Thu, 30 Jun 2011 22:42:03 GMT) Full text and rfc822 format available.

Message #15 received at 632192@bugs.debian.org (full text, mbox):

From: Geert Stappers <stappers@stappers.nl>
To: Marcus Osdoba <marcus.osdoba@googlemail.com>
Cc: debian-embedded@lists.debian.org, 632192@bugs.debian.org
Subject: Re: FWIW debian/qemu-user-static.README.Debian
Date: Fri, 1 Jul 2011 00:38:27 +0200
On Thu, Jun 30, 2011 at 11:33:27PM +0200, Marcus Osdoba wrote:
> Am 30.06.2011 23:15, schrieb Geert Stappers:
> >   When used with binfmt, qemu-user-ARCH-static can be configured to run
> >   a foreign chroot. The qemu-user-ARCH-static binary should be copied
> >   (or better hard linked) into the chroot in /usr/bin. Then the chroot
> >   can be entered and used like a normal chroot.
> Am I missing something? That was already done in polystrap:
> <36073d0c9a389048a7f6a4cd4e9cd28ed1bc32a9>
> # copy qemu usermode binary
> if [ $ARCH != "`dpkg --print-architecture`" ]; then
>         case $ARCH in
> 
> alpha|arm|armeb|i386|m68k|mips|mipsel|ppc64|sh4|sh4eb|sparc|sparc64)
>                 cp `which qemu-$ARCH-static` $ROOTDIR/usr/bin;;
>                 amd64) cp `which qemu-x86_64-static` $ROOTDIR/usr/bin;;
>                 armel) cp `which qemu-arm-static` $ROOTDIR/usr/bin;;
>                 lpia) cp `which qemu-i386-static` $ROOTDIR/usr/bin;;
>                 powerpc) cp `which qemu-ppc-static` $ROOTDIR/usr/bin;;
>                 *) echo "unknown architecture: $ARCH"; exit 1;;
>         esac
> fi
> </36073d0c9a389048a7f6a4cd4e9cd28ed1bc32a9>

You are right, that copying is already done in polystrap.


Stappers
who found meanwhile the variable 'interp_prefix'





Information forwarded to debian-bugs-dist@lists.debian.org, Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>:
Bug#632192; Package qemu. (Fri, 01 Jul 2011 04:57:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Johannes Schauer <j.schauer@email.de>:
Extra info received and forwarded to list. Copy sent to Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>. (Fri, 01 Jul 2011 04:57:03 GMT) Full text and rfc822 format available.

Message #20 received at 632192@bugs.debian.org (full text, mbox):

From: Johannes Schauer <j.schauer@email.de>
To: Geert Stappers <stappers@stappers.nl>
Cc: 632192@bugs.debian.org, debian-embedded@lists.debian.org
Subject: Re: FWIW debian/qemu-user-static.README.Debian
Date: Fri, 1 Jul 2011 06:53:10 +0200
Hi,

On Thu, Jun 30, 2011 at 11:15:09PM +0200, Geert Stappers wrote:
> For What it's Worth:
> In the file debian/qemu-user-static.README.Debian
> is this text
> * Configuring qemu-user-static to use in a foreign chroot
>   When used with binfmt, qemu-user-ARCH-static can be configured to run
>   a foreign chroot. The qemu-user-ARCH-static binary should be copied 
>   (or better hard linked) into the chroot in /usr/bin. Then the chroot 
>   can be entered and used like a normal chroot.
> 
> I haven't played with it, but it seems to help for "polystrap"
no, this advise is only valid for a real chroot (which fakechroot does
not provide). With a real chroot, qemu-user-static will then use the
shared libraries it is supposed to.

With fakechroot, the qemu-user-static binary that was copied into the
root directory will never be executed (see next email)

cheers, josch




Information forwarded to debian-bugs-dist@lists.debian.org, Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>:
Bug#632192; Package qemu. (Fri, 01 Jul 2011 05:03:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Johannes Schauer <j.schauer@email.de>:
Extra info received and forwarded to list. Copy sent to Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>. (Fri, 01 Jul 2011 05:03:03 GMT) Full text and rfc822 format available.

Message #25 received at 632192@bugs.debian.org (full text, mbox):

From: Johannes Schauer <j.schauer@email.de>
To: Marcus Osdoba <marcus.osdoba@googlemail.com>
Cc: debian-embedded@lists.debian.org, 632192@bugs.debian.org
Subject: Re: FWIW debian/qemu-user-static.README.Debian
Date: Fri, 1 Jul 2011 07:01:12 +0200
Hi,

On Thu, Jun 30, 2011 at 11:33:27PM +0200, Marcus Osdoba wrote:
> Am I missing something? That was already done in polystrap:
> <36073d0c9a389048a7f6a4cd4e9cd28ed1bc32a9>
> # copy qemu usermode binary
> if [ $ARCH != "`dpkg --print-architecture`" ]; then
>         case $ARCH in
> 
> alpha|arm|armeb|i386|m68k|mips|mipsel|ppc64|sh4|sh4eb|sparc|sparc64)
>                 cp `which qemu-$ARCH-static` $ROOTDIR/usr/bin;;
>                 amd64) cp `which qemu-x86_64-static` $ROOTDIR/usr/bin;;
>                 armel) cp `which qemu-arm-static` $ROOTDIR/usr/bin;;
>                 lpia) cp `which qemu-i386-static` $ROOTDIR/usr/bin;;
>                 powerpc) cp `which qemu-ppc-static` $ROOTDIR/usr/bin;;
>                 *) echo "unknown architecture: $ARCH"; exit 1;;
>         esac
> fi
> </36073d0c9a389048a7f6a4cd4e9cd28ed1bc32a9>
> 
> I do this to in my multistrap wrapper, but it needs super user
> rights to get it work. I think, polystraps goal was to run in user
> space.
Just copying the qemu-user-static binary that matches the architecture
of the root file system does not require superuser rights, but executing
chroot to make it work does. Polystrap is supposed to run without
superuser rights and therefor uses fakeroot/fakechroot.

By a mistake of mine the piece of code you quoted above was still in the
polystrap code until yesterday when I removed it. It is some old cruft
that was still inside there from the times when I used a real superuser
chroot call to configure my rootfs. I have no idea how that piece of
code survived that long but since yesterday it is gone for good now.

With fakechroot, it will always be the host system's qemu-user-static
binary that will be called.

I wrote a tiny patch to supply the suggested functionality to qemu and
my system is since busy compiling the debian package. Will upload the
patch once it works.

cheers, josch




Information forwarded to debian-bugs-dist@lists.debian.org, Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>:
Bug#632192; Package qemu. (Fri, 01 Jul 2011 05:36:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Johannes Schauer <j.schauer@email.de>:
Extra info received and forwarded to list. Copy sent to Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>. (Fri, 01 Jul 2011 05:36:03 GMT) Full text and rfc822 format available.

Message #30 received at 632192@bugs.debian.org (full text, mbox):

From: Johannes Schauer <j.schauer@email.de>
To: 632192@bugs.debian.org
Subject: Re: qemu: get elf interpreter prefix from environment variable
Date: Fri, 1 Jul 2011 07:33:22 +0200
[Message part 1 (text/plain, inline)]
Hi,

I attached the very simple patch that will make the requested
functionality work in qemu. One can now do something like this:

QEMU_LD_PREFIX=/tmp/debian-sid-armel fakeroot fakechroot chroot /tmp/debian-sid-armel dpkg --configure -a

and it works! without anything in /etc/qemu-binfmt/arm :D

cheers, josch
[0001-add-QEMU_LD_PREFIX-environment-variable.patch (text/x-diff, attachment)]

Added tag(s) patch. Request was from Johannes Schauer <j.schauer@email.de> to control@bugs.debian.org. (Fri, 01 Jul 2011 05:39:04 GMT) Full text and rfc822 format available.

Information forwarded to debian-bugs-dist@lists.debian.org, Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>:
Bug#632192; Package qemu. (Tue, 26 Jul 2011 10:26:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Geert Stappers <stappers@stappers.nl>:
Extra info received and forwarded to list. Copy sent to Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>. (Tue, 26 Jul 2011 10:26:13 GMT) Full text and rfc822 format available.

Message #37 received at 632192@bugs.debian.org (full text, mbox):

From: Geert Stappers <stappers@stappers.nl>
To: 632192@bugs.debian.org
Cc: Geert Stappers <stappers@stappers.nl>
Subject: [PATCH 2/2] add QEMU_LD_PREFIX environment variable for BSD and Darwin
Date: Tue, 26 Jul 2011 12:21:14 +0200
---
 bsd-user/main.c    |    5 +++++
 darwin-user/main.c |    5 +++++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index 6018a41..ed639de 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -763,6 +763,11 @@ int main(int argc, char **argv)
     cpudef_setup(); /* parse cpu definitions in target config file (TBD) */
 #endif
 
+    /* read interp_prefix from environment variable */
+    if (getenv("QEMU_LD_PREFIX") != NULL) {
+        interp_prefix = getenv("QEMU_LD_PREFIX");
+    }
+
     optind = 1;
     for(;;) {
         if (optind >= argc)
diff --git a/darwin-user/main.c b/darwin-user/main.c
index 35196a1..f808521 100644
--- a/darwin-user/main.c
+++ b/darwin-user/main.c
@@ -751,6 +751,11 @@ int main(int argc, char **argv)
     if (argc <= 1)
         usage();
 
+    /* read interp_prefix from environment variable */
+    if (getenv("QEMU_LD_PREFIX") != NULL) {
+        interp_prefix = getenv("QEMU_LD_PREFIX");
+    }
+
     optind = 1;
     for(;;) {
         if (optind >= argc)
-- 
1.7.5.4





Information forwarded to debian-bugs-dist@lists.debian.org, Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>:
Bug#632192; Package qemu. (Tue, 26 Jul 2011 16:15:06 GMT) Full text and rfc822 format available.

Acknowledgement sent to Geert Stappers <stappers@stappers.nl>:
Extra info received and forwarded to list. Copy sent to Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>. (Tue, 26 Jul 2011 16:15:07 GMT) Full text and rfc822 format available.

Message #42 received at 632192@bugs.debian.org (full text, mbox):

From: Geert Stappers <stappers@stappers.nl>
To: 632192@bugs.debian.org
Cc: pkg-qemu-devel@lists.alioth.debian.org, Geert Stappers <stappers@stappers.nl>
Subject: [PATCH 0/3] Reworked patches for #632192
Date: Tue, 26 Jul 2011 18:12:51 +0200
Hello,

This E-mail introduces three E-mails with patches.
They are against the pkg-qemu git repository branch debian-unstable

The first patch should be from: Johannes Schauer <josch@pyneo.org>
the others are based upon it.


Cheers
Geert Stappers





Information forwarded to debian-bugs-dist@lists.debian.org, Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>:
Bug#632192; Package qemu. (Tue, 26 Jul 2011 16:15:08 GMT) Full text and rfc822 format available.

Acknowledgement sent to Geert Stappers <stappers@stappers.nl>:
Extra info received and forwarded to list. Copy sent to Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>. (Tue, 26 Jul 2011 16:15:08 GMT) Full text and rfc822 format available.

Message #47 received at 632192@bugs.debian.org (full text, mbox):

From: Geert Stappers <stappers@stappers.nl>
To: 632192@bugs.debian.org
Cc: pkg-qemu-devel@lists.alioth.debian.org, Johannes Schauer <josch@pyneo.org>
Subject: [PATCH 1/3] add QEMU_LD_PREFIX environment variable
Date: Tue, 26 Jul 2011 18:12:52 +0200
From: Johannes Schauer <josch@pyneo.org>

---
 linux-user/main.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 0d627d6..c802175 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -2758,6 +2758,11 @@ int main(int argc, char **argv, char **envp)
     cpudef_setup(); /* parse cpu definitions in target config file (TBD) */
 #endif
 
+    /* read interp_prefix from environment variable */
+    if (getenv("QEMU_LD_PREFIX") != NULL) {
+        interp_prefix = getenv("QEMU_LD_PREFIX");
+    }
+
     optind = 1;
     for(;;) {
         if (optind >= argc)
-- 
1.7.5.4





Information forwarded to debian-bugs-dist@lists.debian.org, Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>:
Bug#632192; Package qemu. (Tue, 26 Jul 2011 16:15:10 GMT) Full text and rfc822 format available.

Acknowledgement sent to Geert Stappers <stappers@stappers.nl>:
Extra info received and forwarded to list. Copy sent to Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>. (Tue, 26 Jul 2011 16:15:10 GMT) Full text and rfc822 format available.

Message #52 received at 632192@bugs.debian.org (full text, mbox):

From: Geert Stappers <stappers@stappers.nl>
To: 632192@bugs.debian.org
Cc: pkg-qemu-devel@lists.alioth.debian.org, Geert Stappers <stappers@stappers.nl>
Subject: [PATCH 3/3] debian/qemu-user.1: QEMU_LD_PREFIX environment documented
Date: Tue, 26 Jul 2011 18:12:54 +0200
---
 debian/qemu-user.1 |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/debian/qemu-user.1 b/debian/qemu-user.1
index 84c07aa..8a7ea50 100644
--- a/debian/qemu-user.1
+++ b/debian/qemu-user.1
@@ -1,5 +1,5 @@
-.\" $Id: qemu-user.1 234 2007-02-07 22:57:18Z guillem $
-.TH qemu\-user 1 2007-02-08 "0.9.0" Debian
+.\" nroff file, manually crafted (not generated from another source)
+.TH qemu\-user 1 2011-07-26 "0.14.1" Debian
 .SH NAME
 qemu\-user \- QEMU User Emulator
 .SH SYNOPSIS
@@ -31,6 +31,16 @@ Activate log (logfile=\fI/tmp/qemu.log\fP)
 .TP
 .BR \-p " \fI<pagesize>\fP"
 Set the host page size to 'pagesize'.
+.RE
+.SH "ENVIRONMENT"
+.PP
+\fBQEMU_LD_PREFIX\fR
+.RS 4
+If $QEMU_LD_PREFIX is set, it's value is used as prefix to \
+ the ELF interpreter. \
+This is doing the same as the option \fB\-L\fR, \
+but the command line option takes precedence.
+.RE
 .SH SEE ALSO
 .BR qemu (1),
 .BR qemu\-img (1).
-- 
1.7.5.4





Information forwarded to debian-bugs-dist@lists.debian.org, Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>:
Bug#632192; Package qemu. (Tue, 26 Jul 2011 16:15:12 GMT) Full text and rfc822 format available.

Acknowledgement sent to Geert Stappers <stappers@stappers.nl>:
Extra info received and forwarded to list. Copy sent to Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>. (Tue, 26 Jul 2011 16:15:12 GMT) Full text and rfc822 format available.

Message #57 received at 632192@bugs.debian.org (full text, mbox):

From: Geert Stappers <stappers@stappers.nl>
To: 632192@bugs.debian.org
Cc: pkg-qemu-devel@lists.alioth.debian.org, Geert Stappers <stappers@stappers.nl>
Subject: [PATCH 2/3] add QEMU_LD_PREFIX environment variable for BSD and Darwin
Date: Tue, 26 Jul 2011 18:12:53 +0200
---
 bsd-user/main.c    |    5 +++++
 darwin-user/main.c |    5 +++++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index 6b12f8b..cb98fa6 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -763,6 +763,11 @@ int main(int argc, char **argv)
     cpudef_setup(); /* parse cpu definitions in target config file (TBD) */
 #endif
 
+    /* read interp_prefix from environment variable */
+    if (getenv("QEMU_LD_PREFIX") != NULL) {
+        interp_prefix = getenv("QEMU_LD_PREFIX");
+    }
+
     optind = 1;
     for(;;) {
         if (optind >= argc)
diff --git a/darwin-user/main.c b/darwin-user/main.c
index 175e12f..5a67164 100644
--- a/darwin-user/main.c
+++ b/darwin-user/main.c
@@ -752,6 +752,11 @@ int main(int argc, char **argv)
     /* init debug */
     cpu_set_log_filename(DEBUG_LOGFILE);
 
+    /* read interp_prefix from environment variable */
+    if (getenv("QEMU_LD_PREFIX") != NULL) {
+        interp_prefix = getenv("QEMU_LD_PREFIX");
+    }
+
     optind = 1;
     for(;;) {
         if (optind >= argc)
-- 
1.7.5.4





Information forwarded to debian-bugs-dist@lists.debian.org, Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>:
Bug#632192; Package qemu. (Thu, 28 Jul 2011 08:42:19 GMT) Full text and rfc822 format available.

Acknowledgement sent to Riku Voipio <riku.voipio@iki.fi>:
Extra info received and forwarded to list. Copy sent to Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>. (Thu, 28 Jul 2011 08:42:20 GMT) Full text and rfc822 format available.

Message #62 received at 632192@bugs.debian.org (full text, mbox):

From: Riku Voipio <riku.voipio@iki.fi>
To: josch <josch@pyneo.org>
Cc: Riku Voipio <riku.voipio@iki.fi>, qemu-devel@nongnu.org, Johannes Schauer <j.schauer@email.de>, stappers@stappers.nl, 632192@bugs.debian.org
Subject: Re: [PATCH] add QEMU_LD_PREFIX environment variable
Date: Thu, 28 Jul 2011 11:41:09 +0300
On Sat, Jul 23, 2011 at 07:47:49AM +0200, josch wrote:
> This could be avoided by setting the proposed environment variable 
> QEMU_LD_PREFIX to the just
> created debian rootfs. As mentioned earlier, the usage of the -L option
> is not possible in this scenario because qemu-user is only implicitly
> called by the binfmt mechanism.

What worries me here is that we are beginning to add a enviroment variable
for each and every command line option of qemu linux-user. I think it would
be better to have a wrapper binary to be registered as the binfmt runner.
Alternatively we should have a generic setup for mapping enviroment variables
to command line options. Now we get special per-option code every time someone 
needs to setup a command line option from binfmt.

Riku




Information forwarded to debian-bugs-dist@lists.debian.org, Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>:
Bug#632192; Package qemu. (Thu, 28 Jul 2011 11:27:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Johannes Schauer <j.schauer@email.de>:
Extra info received and forwarded to list. Copy sent to Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>. (Thu, 28 Jul 2011 11:27:08 GMT) Full text and rfc822 format available.

Message #67 received at 632192@bugs.debian.org (full text, mbox):

From: Johannes Schauer <j.schauer@email.de>
To: Riku Voipio <riku.voipio@iki.fi>
Cc: josch <josch@pyneo.org>, qemu-devel@nongnu.org, stappers@stappers.nl, 632192@bugs.debian.org
Subject: Re: [PATCH] add QEMU_LD_PREFIX environment variable
Date: Thu, 28 Jul 2011 13:24:47 +0200
Hi,

On Thu, Jul 28, 2011 at 11:41:09AM +0300, Riku Voipio wrote:
> On Sat, Jul 23, 2011 at 07:47:49AM +0200, josch wrote:
> > This could be avoided by setting the proposed environment variable 
> > QEMU_LD_PREFIX to the just
> > created debian rootfs. As mentioned earlier, the usage of the -L option
> > is not possible in this scenario because qemu-user is only implicitly
> > called by the binfmt mechanism.
> 
> What worries me here is that we are beginning to add a enviroment
> variable for each and every command line option of qemu linux-user.

Are there other environment variables? I didnt see any? (well besides
QEMU_STRACE that is)

> I think it would be better to have a wrapper binary to be registered
> as the binfmt runner.

If you need help with writing something - dont hesitate to ask for help.
I'm very interested in having this functionality working because of the
reasons I gave in my initial mail. [1]

> Alternatively we should have a generic setup for mapping enviroment
> variables to command line options. Now we get special per-option code
> every time someone needs to setup a command line option from binfmt.

What other options are there that would be interesting for binfmt?

I was also sending this patch to qemu-devel list a month ago but got no
reply. [2]

As I said - if you would like a wrapper or a generic setup for mapping
env variables to commandline parameters and dont have time to do it
yourself, dont hesitate to delegate some work to me :)


@Geert Stappers:

you are patching bsd-user/main.c and darwin-user/main.c as well. I take
it that you did test your changes on those platforms? does it work there
as well? I have no clue of darwin but is it really useful there?

cheers, josch

[1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=632192#5
[2] http://lists.nongnu.org/archive/html/qemu-devel/2011-07/msg00459.html




Information forwarded to debian-bugs-dist@lists.debian.org, Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>:
Bug#632192; Package qemu. (Thu, 28 Jul 2011 16:54:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Geert Stappers <stappers@stappers.nl>:
Extra info received and forwarded to list. Copy sent to Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>. (Thu, 28 Jul 2011 16:54:03 GMT) Full text and rfc822 format available.

Message #72 received at 632192@bugs.debian.org (full text, mbox):

From: Geert Stappers <stappers@stappers.nl>
To: Johannes Schauer <j.schauer@email.de>, Riku Voipio <riku.voipio@iki.fi>, josch <josch@pyneo.org>, qemu-devel@nongnu.org, stappers@stappers.nl, 632192@bugs.debian.org
Subject: Re: [PATCH] add QEMU_LD_PREFIX environment variable
Date: Thu, 28 Jul 2011 18:50:26 +0200
On Thu, Jul 28, 2011 at 01:24:47PM +0200, Johannes Schauer wrote:
> 
> @Geert Stappers:
> 
> you are patching bsd-user/main.c and darwin-user/main.c as well. I take
> it that you did test your changes on those platforms? does it work there
> as well? I have no clue of darwin but is it really useful there?

They only check I did,
was checking if BSD and Darwing have getenv(), they do.

I consider the 
+    /* read interp_prefix from environment variable */
+    if (getenv("QEMU_LD_PREFIX") != NULL) {
+        interp_prefix = getenv("QEMU_LD_PREFIX");
to Darwin and BSD as harmless. In fact only reason for those additional
lines were to bring attention to those operating systems.

With the idea^Whope of better acception from upstream.


Cheers
Geert Stappers





Information forwarded to debian-bugs-dist@lists.debian.org, Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>:
Bug#632192; Package qemu. (Thu, 28 Jul 2011 17:30:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Alexander Graf <agraf@suse.de>:
Extra info received and forwarded to list. Copy sent to Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>. (Thu, 28 Jul 2011 17:30:03 GMT) Full text and rfc822 format available.

Message #77 received at 632192@bugs.debian.org (full text, mbox):

From: Alexander Graf <agraf@suse.de>
To: Geert Stappers <stappers@stappers.nl>
Cc: Johannes Schauer <j.schauer@email.de>, Riku Voipio <riku.voipio@iki.fi>, josch <josch@pyneo.org>, qemu-devel@nongnu.org, 632192@bugs.debian.org
Subject: Re: [Qemu-devel] [PATCH] add QEMU_LD_PREFIX environment variable
Date: Thu, 28 Jul 2011 19:28:23 +0200
On 28.07.2011, at 18:50, Geert Stappers wrote:

> On Thu, Jul 28, 2011 at 01:24:47PM +0200, Johannes Schauer wrote:
>> 
>> @Geert Stappers:
>> 
>> you are patching bsd-user/main.c and darwin-user/main.c as well. I take
>> it that you did test your changes on those platforms? does it work there
>> as well? I have no clue of darwin but is it really useful there?
> 
> They only check I did,
> was checking if BSD and Darwing have getenv(), they do.
> 
> I consider the 
> +    /* read interp_prefix from environment variable */
> +    if (getenv("QEMU_LD_PREFIX") != NULL) {
> +        interp_prefix = getenv("QEMU_LD_PREFIX");
> to Darwin and BSD as harmless. In fact only reason for those additional
> lines were to bring attention to those operating systems.

It's called DYLD_PRELOAD on Darwin :). However, I do think that if anything this should be an alias, so you can set either of the two. If anyone cares about darwin-user at all.


Alex





Information forwarded to debian-bugs-dist@lists.debian.org, Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>:
Bug#632192; Package qemu. (Fri, 29 Jul 2011 12:57:18 GMT) Full text and rfc822 format available.

Acknowledgement sent to Vagrant Cascadian <vagrant@freegeek.org>:
Extra info received and forwarded to list. Copy sent to Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>. (Fri, 29 Jul 2011 12:57:27 GMT) Full text and rfc822 format available.

Message #82 received at 632192@bugs.debian.org (full text, mbox):

From: Vagrant Cascadian <vagrant@freegeek.org>
To: Riku Voipio <riku.voipio@iki.fi>, 632192@bugs.debian.org
Cc: josch <josch@pyneo.org>, qemu-devel@nongnu.org, Johannes Schauer <j.schauer@email.de>
Subject: Re: Bug#632192: [PATCH] add QEMU_LD_PREFIX environment variable
Date: Fri, 29 Jul 2011 05:52:50 -0700
On Thu, Jul 28, 2011 at 11:41:09AM +0300, Riku Voipio wrote:
> On Sat, Jul 23, 2011 at 07:47:49AM +0200, josch wrote:
> > This could be avoided by setting the proposed environment variable 
> > QEMU_LD_PREFIX to the just
> > created debian rootfs. As mentioned earlier, the usage of the -L option
> > is not possible in this scenario because qemu-user is only implicitly
> > called by the binfmt mechanism.
> 
> What worries me here is that we are beginning to add a enviroment variable
> for each and every command line option of qemu linux-user. I think it would
> be better to have a wrapper binary to be registered as the binfmt runner.

setting up a wrapper makes trivial cross-architecture chroots harder as you'll 
have to copy two binaries into the chroot, and i'm not sure if it would work at 
all, as the wrapper will need to somehow emulate it's own interpreter...


> Alternatively we should have a generic setup for mapping enviroment variables
> to command line options. Now we get special per-option code every time someone 
> needs to setup a command line option from binfmt.

this would seem a bit better alternative to me...


live well,
  vagrant




Information forwarded to debian-bugs-dist@lists.debian.org, Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>:
Bug#632192; Package qemu. (Fri, 29 Jul 2011 15:27:12 GMT) Full text and rfc822 format available.

Acknowledgement sent to Johannes Schauer <j.schauer@email.de>:
Extra info received and forwarded to list. Copy sent to Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>. (Fri, 29 Jul 2011 15:27:12 GMT) Full text and rfc822 format available.

Message #87 received at 632192@bugs.debian.org (full text, mbox):

From: Johannes Schauer <j.schauer@email.de>
To: Vagrant Cascadian <vagrant@freegeek.org>
Cc: Riku Voipio <riku.voipio@iki.fi>, 632192@bugs.debian.org, qemu-devel@nongnu.org
Subject: Re: Bug#632192: [PATCH] add QEMU_LD_PREFIX environment variable
Date: Fri, 29 Jul 2011 17:21:59 +0200
On Fri, Jul 29, 2011 at 05:52:50AM -0700, Vagrant Cascadian wrote:
> setting up a wrapper makes trivial cross-architecture chroots harder
> as you'll have to copy two binaries into the chroot, and i'm not sure
> if it would work at all, as the wrapper will need to somehow emulate
> it's own interpreter...

Agreed - this makes a wrapper not an option. I didnt think of that.

> > Alternatively we should have a generic setup for mapping enviroment
> > variables to command line options. Now we get special per-option
> > code every time someone needs to setup a command line option from
> > binfmt.
> 
> this would seem a bit better alternative to me...

So if we agree on using environment variables to pass options to
qemu-user we next need to agree on how to name the options.

The following commandline arguments exist (in order as they are checked
in linux-user/main.c) and I shortly described and proposed a name for
the environment variable in the same line.

-d (activate log)             - QEMU_LOG
-D (logfile)                  - QEMU_LOGFILE
-E (set target env variabe)   - QEMU_SET_ENV
-U (unset target env variabe) - QEMU_UNSET_ENV
-0 (set target argv[0])       - QEMU_ARGV0
-s (stack size)               - QEMU_STACK_SIZE
-L (elf interpreter prefix)   - QEMU_LD_PREFIX
-p (page size)                - QEMU_PAGESIZE
-g (listen for gdb on port)   - QEMU_GDB
-r (uname)                    - QEMU_UNAME
-cpu (cpu model)              - QEMU_CPU
-B (guest base)               - QEMU_GUEST_BASE
-R (reserved virtual address) - QEMU_RESERVED_VA
-drop-ld-preload              - QEMU_DROP_LD_PRELOAD
-singlestep                   - QEMU_SINGLESTEP
-strace                       - QEMU_STRACE

also, there already is the QEMU_STRACE environment variable which could
be incorporated into the solution?

the -E and -U options can be specified several times so the environment
variables should be able to receive a list - maybe in the getsubopt(3)
style?

So what do you think?

cheers, josch




Information forwarded to debian-bugs-dist@lists.debian.org, Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>:
Bug#632192; Package qemu. (Sat, 30 Jul 2011 14:00:07 GMT) Full text and rfc822 format available.

Acknowledgement sent to Riku Voipio <riku.voipio@iki.fi>:
Extra info received and forwarded to list. Copy sent to Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>. (Sat, 30 Jul 2011 14:00:07 GMT) Full text and rfc822 format available.

Message #92 received at 632192@bugs.debian.org (full text, mbox):

From: Riku Voipio <riku.voipio@iki.fi>
To: Johannes Schauer <j.schauer@email.de>
Cc: Vagrant Cascadian <vagrant@freegeek.org>, Riku Voipio <riku.voipio@iki.fi>, 632192@bugs.debian.org, qemu-devel@nongnu.org
Subject: Re: Bug#632192: [PATCH] add QEMU_LD_PREFIX environment variable
Date: Sat, 30 Jul 2011 16:58:43 +0300
On Fri, Jul 29, 2011 at 05:21:59PM +0200, Johannes Schauer wrote:
> So if we agree on using environment variables to pass options to
> qemu-user we next need to agree on how to name the options.
 
> The following commandline arguments exist (in order as they are checked
> in linux-user/main.c) and I shortly described and proposed a name for
> the environment variable in the same line.
 
> -d (activate log)             - QEMU_LOG
> -D (logfile)                  - QEMU_LOGFILE
> -E (set target env variabe)   - QEMU_SET_ENV
> -U (unset target env variabe) - QEMU_UNSET_ENV
> -0 (set target argv[0])       - QEMU_ARGV0
> -s (stack size)               - QEMU_STACK_SIZE
> -L (elf interpreter prefix)   - QEMU_LD_PREFIX
> -p (page size)                - QEMU_PAGESIZE
> -g (listen for gdb on port)   - QEMU_GDB
> -r (uname)                    - QEMU_UNAME
> -cpu (cpu model)              - QEMU_CPU
> -B (guest base)               - QEMU_GUEST_BASE
> -R (reserved virtual address) - QEMU_RESERVED_VA
> -drop-ld-preload              - QEMU_DROP_LD_PRELOAD

This is a legacy option that could be removed already. The
-U LD_PRELOAD replaces this option. The only known user of this
option (scratchbox) has migrated -U LD_PRELOAD years ago.

> -singlestep                   - QEMU_SINGLESTEP
> -strace                       - QEMU_STRACE
 
> also, there already is the QEMU_STRACE environment variable which could
> be incorporated into the solution?

Else names look good to me.
 
> the -E and -U options can be specified several times so the environment
> variables should be able to receive a list - maybe in the getsubopt(3)
> style?

getsubopt would mean that passing enviroment variable contents with commas
wouldn't work. Perhaps that would still be an acceptable limitation.

Riku




Information forwarded to debian-bugs-dist@lists.debian.org, Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>:
Bug#632192; Package qemu. (Sun, 31 Jul 2011 11:54:11 GMT) Full text and rfc822 format available.

Acknowledgement sent to j.schauer@email.de:
Extra info received and forwarded to list. Copy sent to Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>. (Sun, 31 Jul 2011 11:54:12 GMT) Full text and rfc822 format available.

Message #97 received at 632192@bugs.debian.org (full text, mbox):

From: j.schauer@email.de
To: Riku Voipio <riku.voipio@iki.fi>, Vagrant Cascadian <vagrant@freegeek.org>, 632192@bugs.debian.org, qemu-devel@nongnu.org
Cc: Johannes Schauer <j.schauer@email.de>
Subject: [PATCH] introduce environment variables for all qemu-user options
Date: Sun, 31 Jul 2011 13:51:30 +0200
From: Johannes Schauer <j.schauer@email.de>

A first try to introduce a generic setup for mapping environment variables to
command line options.

I'm afraid to code something for platforms I can't do runtime tests on, so this 
is only for linux-user for now.

Signed-off-by: Johannes Schauer <j.schauer@email.de>
---
 linux-user/main.c |  147 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 144 insertions(+), 3 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index dbba8be..fb986e3 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -2640,26 +2640,51 @@ static void usage(void)
            "-E var=value      sets/modifies targets environment variable(s)\n"
            "-U var            unsets targets environment variable(s)\n"
            "-0 argv0          forces target process argv[0] to be argv0\n"
+           "-r uname          set qemu uname release string\n"
 #if defined(CONFIG_USE_GUEST_BASE)
            "-B address        set guest_base address to address\n"
            "-R size           reserve size bytes for guest virtual address space\n"
 #endif
            "\n"
+           "Standard environment variables:\n"
+           "QEMU_GDB          see the -g option\n"
+           "QEMU_LD_PREFIX    see the -L option\n"
+           "QEMU_STACK_SIZE   see the -s option\n"
+           "QEMU_CPU          see the -cpu option\n"
+           "QEMU_SET_ENV      see the -E option, comma separated list of arguments\n"
+           "QEMU_UNSET_ENV    see the -U option, comma separated list of arguments\n"
+           "QEMU_ARGV0        see the -0 option\n"
+           "QEMU_UNAME        see the -r option\n"
+#if defined(CONFIG_USE_GUEST_BASE)
+           "QEMU_GUEST_BASE   see the -B option\n"
+           "QEMU_RESERVED_VA  see the -R option\n"
+#endif
+           "\n"
            "Debug options:\n"
            "-d options   activate log (logfile=%s)\n"
            "-p pagesize  set the host page size to 'pagesize'\n"
            "-singlestep  always run in singlestep mode\n"
            "-strace      log system calls\n"
            "\n"
-           "Environment variables:\n"
-           "QEMU_STRACE       Print system calls and arguments similar to the\n"
-           "                  'strace' program.  Enable by setting to any value.\n"
+           "Debug environment variables:\n"
+           "QEMU_LOG          see the -d option\n"
+           "QEMU_PAGESIZE     see the -p option\n"
+           "QEMU_SINGLESTEP   see the -singlestep option\n"
+           "QEMU_STRACE       see the -strace option\n"
+           "\n"
            "You can use -E and -U options to set/unset environment variables\n"
            "for target process.  It is possible to provide several variables\n"
            "by repeating the option.  For example:\n"
            "    -E var1=val2 -E var2=val2 -U LD_PRELOAD -U LD_DEBUG\n"
            "Note that if you provide several changes to single variable\n"
            "last change will stay in effect.\n"
+           "Using the environment variables QEMU_SET_ENV and QEMU_UNSET_ENV\n"
+           "to set/unset environment variables for target process is\n"
+           "possible by a comma separated list of values in getsubopt(3)\n"
+           "style. For example:\n"
+           "    QEMU_SET_ENV=var1=val2,var2=val2 QEMU_UNSET_ENV=LD_PRELOAD,LD_DEBUG\n"
+           "Note that if you provide several changes to single variable\n"
+           "last change will stay in effect.\n"
            ,
            TARGET_ARCH,
            interp_prefix,
@@ -2758,6 +2783,122 @@ int main(int argc, char **argv, char **envp)
     cpudef_setup(); /* parse cpu definitions in target config file (TBD) */
 #endif
 
+    if ((r = getenv("QEMU_LOG")) != NULL) {
+        int mask;
+        const CPULogItem *item;
+        mask = cpu_str_to_log_mask(r);
+        if (!mask) {
+            printf("Log items (comma separated):\n");
+            for(item = cpu_log_items; item->mask != 0; item++) {
+                printf("%-10s %s\n", item->name, item->help);
+            }
+            exit(1);
+        }
+        cpu_set_log(mask);
+    }
+    if ((r = getenv("QEMU_SET_ENV")) != NULL) {
+        char *p, *token;
+        p = strdup(r);
+        while ((token = strsep(&p, ",")) != NULL) {
+            if (envlist_setenv(envlist, token) != 0)
+                usage();
+        }
+        free(p);
+    }
+    if ((r = getenv("QEMU_UNSET_ENV")) != NULL) {
+        char *p, *token;
+        p = strdup(r);
+        while ((token = strsep(&p, ",")) != NULL) {
+            if (envlist_unsetenv(envlist, token) != 0)
+                usage();
+        }
+        free(p);
+    }
+    if ((r = getenv("QEMU_ARGV0")) != NULL) {
+        argv0 = strdup(r);
+    }
+    if ((r = getenv("QEMU_STACK_SIZE")) != NULL) {
+        guest_stack_size = strtoul(r, (char **)&r, 0);
+        if (guest_stack_size == 0)
+            usage();
+        if (*r == 'M')
+            guest_stack_size *= 1024 * 1024;
+        else if (*r == 'k' || *r == 'K')
+            guest_stack_size *= 1024;
+    }
+    if ((r = getenv("QEMU_LD_PREFIX")) != NULL) {
+        interp_prefix = strdup(r);
+    }
+    if ((r = getenv("QEMU_PAGESIZE")) != NULL) {
+        qemu_host_page_size = atoi(r);
+        if (qemu_host_page_size == 0 ||
+            (qemu_host_page_size & (qemu_host_page_size - 1)) != 0) {
+            fprintf(stderr, "page size must be a power of two\n");
+            exit(1);
+        }
+    }
+    if ((r = getenv("QEMU_GDB")) != NULL) {
+        gdbstub_port = atoi(r);
+    }
+    if ((r = getenv("QEMU_UNAME")) != NULL) {
+        qemu_uname_release = strdup(r);
+    }
+    if ((r = getenv("QEMU_CPU")) != NULL) {
+        cpu_model = strdup(r);
+        if (cpu_model == NULL || strcmp(cpu_model, "?") == 0) {
+/* XXX: implement xxx_cpu_list for targets that still miss it */
+#if defined(cpu_list_id)
+            cpu_list_id(stdout, &fprintf, "");
+#elif defined(cpu_list)
+            cpu_list(stdout, &fprintf); /* deprecated */
+#endif
+            exit(1);
+        }
+    }
+#if defined(CONFIG_USE_GUEST_BASE)
+    if ((r = getenv("QEMU_GUEST_BASE")) != NULL) {
+       guest_base = strtol(r, NULL, 0);
+       have_guest_base = 1;
+    }
+    if ((r = getenv("QEMU_QEMU_RESERVED_VA")) != NULL) {
+        char *p;
+        int shift = 0;
+        reserved_va = strtoul(r, &p, 0);
+        switch (*p) {
+        case 'k':
+        case 'K':
+            shift = 10;
+            break;
+        case 'M':
+            shift = 20;
+            break;
+        case 'G':
+            shift = 30;
+            break;
+        }
+        if (shift) {
+            unsigned long unshifted = reserved_va;
+            p++;
+            reserved_va <<= shift;
+            if (((reserved_va >> shift) != unshifted)
+#if HOST_LONG_BITS > TARGET_VIRT_ADDR_SPACE_BITS
+                || (reserved_va > (1ul << TARGET_VIRT_ADDR_SPACE_BITS))
+#endif
+                ) {
+                fprintf(stderr, "Reserved virtual address too big\n");
+                exit(1);
+            }
+        }
+        if (*p) {
+            fprintf(stderr, "Unrecognised -R size suffix '%s'\n", p);
+            exit(1);
+        }
+    }
+#endif
+    if ((r = getenv("QEMU_SINGLESTEP")) != NULL) {
+        singlestep = 1;
+    }
+
     optind = 1;
     for(;;) {
         if (optind >= argc)
-- 
1.7.5.4





Information forwarded to debian-bugs-dist@lists.debian.org, Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>:
Bug#632192; Package qemu. (Sun, 31 Jul 2011 12:15:50 GMT) Full text and rfc822 format available.

Acknowledgement sent to Peter Maydell <peter.maydell@linaro.org>:
Extra info received and forwarded to list. Copy sent to Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>. (Sun, 31 Jul 2011 12:16:34 GMT) Full text and rfc822 format available.

Message #102 received at 632192@bugs.debian.org (full text, mbox):

From: Peter Maydell <peter.maydell@linaro.org>
To: j.schauer@email.de
Cc: Riku Voipio <riku.voipio@iki.fi>, Vagrant Cascadian <vagrant@freegeek.org>, 632192@bugs.debian.org, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] introduce environment variables for all qemu-user options
Date: Sun, 31 Jul 2011 13:12:02 +0100
On 31 July 2011 12:51,  <j.schauer@email.de> wrote:
> +    if ((r = getenv("QEMU_STACK_SIZE")) != NULL) {
> +        guest_stack_size = strtoul(r, (char **)&r, 0);
> +        if (guest_stack_size == 0)
> +            usage();
> +        if (*r == 'M')
> +            guest_stack_size *= 1024 * 1024;
> +        else if (*r == 'k' || *r == 'K')
> +            guest_stack_size *= 1024;
> +    }

[etc]

This is all basically duplicating the existing command line
argument parsing code, which (a) makes it very easy for the
two to drift out of sync in future and (b) means command line
options added in future might end up without a corresponding
environment variable.

I think it would be much nicer to have this be table-driven,
to avoid the code duplication. It ought to be possible to
derive most of the extra --help text from the table too.

-- PMM




Information forwarded to debian-bugs-dist@lists.debian.org, Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>:
Bug#632192; Package qemu. (Sun, 31 Jul 2011 21:45:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Johannes Schauer <j.schauer@email.de>:
Extra info received and forwarded to list. Copy sent to Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>. (Sun, 31 Jul 2011 21:45:03 GMT) Full text and rfc822 format available.

Message #107 received at 632192@bugs.debian.org (full text, mbox):

From: Johannes Schauer <j.schauer@email.de>
To: Peter Maydell <peter.maydell@linaro.org>, Riku Voipio <riku.voipio@iki.fi>, Vagrant Cascadian <vagrant@freegeek.org>, 632192@bugs.debian.org, qemu-devel@nongnu.org
Cc: Johannes Schauer <j.schauer@email.de>
Subject: [PATCH] introduce environment variables for all qemu-user options
Date: Sun, 31 Jul 2011 23:40:58 +0200
Rework option parsing code for linux-user in a table-driven manner to allow
environment variables for all commandline options.

Also generate usage() output from option table.



Signed-off-by: Johannes Schauer <j.schauer@email.de>
---
 linux-user/main.c |  518 ++++++++++++++++++++++++++++++++++-------------------
 1 files changed, 331 insertions(+), 187 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index dbba8be..95e8651 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -40,6 +40,10 @@
 char *exec_path;
 
 int singlestep;
+const char *filename;
+const char *argv0 = NULL;
+int gdbstub_port = 0;
+const char *cpu_model;
 unsigned long mmap_min_addr;
 #if defined(CONFIG_USE_GUEST_BASE)
 unsigned long guest_base;
@@ -47,6 +51,8 @@ int have_guest_base;
 unsigned long reserved_va;
 #endif
 
+static void usage(void);
+
 static const char *interp_prefix = CONFIG_QEMU_INTERP_PREFIX;
 const char *qemu_uname_release = CONFIG_UNAME_RELEASE;
 
@@ -2624,50 +2630,6 @@ void cpu_loop (CPUState *env)
 }
 #endif /* TARGET_ALPHA */
 
-static void usage(void)
-{
-    printf("qemu-" TARGET_ARCH " version " QEMU_VERSION QEMU_PKGVERSION ", Copyright (c) 2003-2008 Fabrice Bellard\n"
-           "usage: qemu-" TARGET_ARCH " [options] program [arguments...]\n"
-           "Linux CPU emulator (compiled for %s emulation)\n"
-           "\n"
-           "Standard options:\n"
-           "-h                print this help\n"
-           "-g port           wait gdb connection to port\n"
-           "-L path           set the elf interpreter prefix (default=%s)\n"
-           "-s size           set the stack size in bytes (default=%ld)\n"
-           "-cpu model        select CPU (-cpu ? for list)\n"
-           "-drop-ld-preload  drop LD_PRELOAD for target process\n"
-           "-E var=value      sets/modifies targets environment variable(s)\n"
-           "-U var            unsets targets environment variable(s)\n"
-           "-0 argv0          forces target process argv[0] to be argv0\n"
-#if defined(CONFIG_USE_GUEST_BASE)
-           "-B address        set guest_base address to address\n"
-           "-R size           reserve size bytes for guest virtual address space\n"
-#endif
-           "\n"
-           "Debug options:\n"
-           "-d options   activate log (logfile=%s)\n"
-           "-p pagesize  set the host page size to 'pagesize'\n"
-           "-singlestep  always run in singlestep mode\n"
-           "-strace      log system calls\n"
-           "\n"
-           "Environment variables:\n"
-           "QEMU_STRACE       Print system calls and arguments similar to the\n"
-           "                  'strace' program.  Enable by setting to any value.\n"
-           "You can use -E and -U options to set/unset environment variables\n"
-           "for target process.  It is possible to provide several variables\n"
-           "by repeating the option.  For example:\n"
-           "    -E var1=val2 -E var2=val2 -U LD_PRELOAD -U LD_DEBUG\n"
-           "Note that if you provide several changes to single variable\n"
-           "last change will stay in effect.\n"
-           ,
-           TARGET_ARCH,
-           interp_prefix,
-           guest_stack_size,
-           DEBUG_LOGFILE);
-    exit(1);
-}
-
 THREAD CPUState *thread_env;
 
 void task_settid(TaskState *ts)
@@ -2703,24 +2665,342 @@ void init_task_state(TaskState *ts)
     }
     ts->sigqueue_table[i].next = NULL;
 }
- 
+
+static void handle_arg_help(const char *arg, envlist_t *envlist)
+{
+    usage();
+}
+
+static void handle_arg_log(const char *arg, envlist_t *envlist)
+{
+    int mask;
+    const CPULogItem *item;
+
+    mask = cpu_str_to_log_mask(arg);
+    if (!mask) {
+        printf("Log items (comma separated):\n");
+        for(item = cpu_log_items; item->mask != 0; item++) {
+            printf("%-10s %s\n", item->name, item->help);
+        }
+        exit(1);
+    }
+    cpu_set_log(mask);
+}
+
+static void handle_arg_set_env(const char *arg, envlist_t *envlist)
+{
+    char *r, *p, *token;
+    r = p = strdup(arg);
+    while ((token = strsep(&p, ",")) != NULL) {
+        if (envlist_setenv(envlist, token) != 0)
+            usage();
+    }
+    free(r);
+}
+
+static void handle_arg_unset_env(const char *arg, envlist_t *envlist)
+{
+    char *r, *p, *token;
+    r = p = strdup(arg);
+    while ((token = strsep(&p, ",")) != NULL) {
+        if (envlist_unsetenv(envlist, token) != 0)
+            usage();
+    }
+    free(r);
+}
+
+static void handle_arg_argv0(const char *arg, envlist_t *envlist)
+{
+    argv0 = strdup(arg);
+}
+
+static void handle_arg_stack_size(const char *arg, envlist_t *envlist)
+{
+    char *p;
+    guest_stack_size = strtoul(arg, &p, 0);
+    if (guest_stack_size == 0)
+        usage();
+    if (*p == 'M')
+        guest_stack_size *= 1024 * 1024;
+    else if (*p == 'k' || *p == 'K')
+        guest_stack_size *= 1024;
+}
+
+static void handle_arg_ld_prefix(const char *arg, envlist_t *envlist)
+{
+    interp_prefix = strdup(arg);
+}
+
+static void handle_arg_pagesize(const char *arg, envlist_t *envlist)
+{
+    qemu_host_page_size = atoi(arg);
+    if (qemu_host_page_size == 0 ||
+        (qemu_host_page_size & (qemu_host_page_size - 1)) != 0) {
+        fprintf(stderr, "page size must be a power of two\n");
+        exit(1);
+    }
+}
+
+static void handle_arg_gdb(const char *arg, envlist_t *envlist)
+{
+    gdbstub_port = atoi(arg);
+}
+
+static void handle_arg_uname(const char *arg, envlist_t *envlist)
+{
+    qemu_uname_release = strdup(arg);
+}
+
+static void handle_arg_cpu(const char *arg, envlist_t *envlist)
+{
+    cpu_model = strdup(arg);
+    if (cpu_model == NULL || strcmp(cpu_model, "?") == 0) {
+/* XXX: implement xxx_cpu_list for targets that still miss it */
+#if defined(cpu_list_id)
+        cpu_list_id(stdout, &fprintf, "");
+#elif defined(cpu_list)
+        cpu_list(stdout, &fprintf); /* deprecated */
+#endif
+        exit(1);
+    }
+}
+
+#if defined(CONFIG_USE_GUEST_BASE)
+static void handle_arg_guest_base(const char *arg, envlist_t *envlist)
+{
+    guest_base = strtol(arg, NULL, 0);
+    have_guest_base = 1;
+}
+
+static void handle_arg_reserved_va(const char *arg, envlist_t *envlist)
+{
+    char *p;
+    int shift = 0;
+    reserved_va = strtoul(arg, &p, 0);
+    switch (*p) {
+    case 'k':
+    case 'K':
+        shift = 10;
+        break;
+    case 'M':
+        shift = 20;
+        break;
+    case 'G':
+        shift = 30;
+        break;
+    }
+    if (shift) {
+        unsigned long unshifted = reserved_va;
+        p++;
+        reserved_va <<= shift;
+        if (((reserved_va >> shift) != unshifted)
+#if HOST_LONG_BITS > TARGET_VIRT_ADDR_SPACE_BITS
+            || (reserved_va > (1ul << TARGET_VIRT_ADDR_SPACE_BITS))
+#endif
+            ) {
+            fprintf(stderr, "Reserved virtual address too big\n");
+            exit(1);
+        }
+    }
+    if (*p) {
+        fprintf(stderr, "Unrecognised -R size suffix '%s'\n", p);
+        exit(1);
+    }
+}
+#endif
+
+static void handle_arg_singlestep(const char *arg, envlist_t *envlist)
+{
+    singlestep = 1;
+}
+
+static void handle_arg_strace(const char *arg, envlist_t *envlist)
+{
+    do_strace = 1;
+}
+
+struct qemu_argument {
+    const char *argv;
+    const char *env;
+    bool has_arg;
+    void (*handle_opt)(const char *arg, envlist_t *envlist);
+    const char *example;
+    const char *help;
+};
+
+struct qemu_argument arg_table[] = {
+    {"h",          "",                 false, handle_arg_help,
+     "",           "print this help"},
+    {"g",          "QEMU_GDB",         true,  handle_arg_gdb,
+     "port",       "wait gdb connection to 'port'"},
+    {"L",          "QEMU_LD_PREFIX",   true,  handle_arg_ld_prefix,
+     "path",       "set the elf interpreter prefix to 'path'"},
+    {"s",          "QEMU_STACK_SIZE",  true,  handle_arg_stack_size,
+     "size",       "set the stack size to 'size' bytes"},
+    {"cpu",        "QEMU_CPU",         true,  handle_arg_cpu,
+     "model",      "select CPU (-cpu ? for list)"},
+    {"E",          "QEMU_SET_ENV",     true,  handle_arg_set_env,
+     "var=value",  "sets targets environment variable (see below)"},
+    {"U",          "QEMU_UNSET_ENV",   true,  handle_arg_unset_env,
+     "var",        "unsets targets environment variable (see below)"},
+    {"0",          "QEMU_ARGV0",       true,  handle_arg_argv0,
+     "argv0",      "forces target process argv[0] to be 'argv0'"},
+    {"r",          "QEMU_UNAME",       true,  handle_arg_uname,
+     "uname",      "set qemu uname release string to 'uname'"},
+#if defined(CONFIG_USE_GUEST_BASE)
+    {"B",          "QEMU_GUEST_BASE",  true,  handle_arg_guest_base,
+     "address",    "set guest_base address to 'address'"},
+    {"R",          "QEMU_RESERVED_VA", true,  handle_arg_reserved_va,
+     "size",       "reserve 'size' bytes for guest virtual address space"},
+#endif
+    {"d",          "QEMU_LOG",         true,  handle_arg_log,
+     "options",    "activate log"},
+    {"p",          "QEMU_PAGESIZE",    true,  handle_arg_pagesize,
+     "pagesize",   "set the host page size to 'pagesize'"},
+    {"singlestep", "QEMU_SINGLESTEP",  false, handle_arg_singlestep,
+     "",           "run in singlestep mode"},
+    {"strace",     "QEMU_STRACE",      false, handle_arg_strace,
+     "",           "log system calls"},
+    {NULL, NULL, false, NULL, NULL, NULL}
+};
+
+static void usage(void)
+{
+    struct qemu_argument *arginfo;
+    int maxarglen;
+    int maxenvlen;
+
+    printf("qemu-" TARGET_ARCH " version " QEMU_VERSION QEMU_PKGVERSION
+           ", Copyright (c) 2003-2008 Fabrice Bellard\n"
+           "usage: qemu-" TARGET_ARCH " [options] program [arguments...]\n"
+           "Linux CPU emulator (compiled for " TARGET_ARCH " emulation)\n"
+           "\n"
+           "Options and associated environment variables:\n"
+           "\n");
+
+    maxarglen = maxenvlen = 0;
+
+    for (arginfo = arg_table; arginfo->handle_opt != NULL; arginfo++) {
+        if (strlen(arginfo->env) > maxenvlen)
+            maxenvlen = strlen(arginfo->env);
+        if (strlen(arginfo->argv) > maxarglen)
+            maxarglen = strlen(arginfo->argv);
+    }
+
+    printf("%-*s%-*sDescription\n", maxarglen+3, "Argument",
+            maxenvlen+1, "Env-variable");
+
+    for (arginfo = arg_table; arginfo->handle_opt != NULL; arginfo++) {
+        if (arginfo->has_arg) {
+            printf("-%s %-*s %-*s %s\n", arginfo->argv,
+                    (int)(maxarglen-strlen(arginfo->argv)), arginfo->example,
+                    maxenvlen, arginfo->env, arginfo->help);
+        } else {
+            printf("-%-*s %-*s %s\n", maxarglen+1, arginfo->argv,
+                    maxenvlen, arginfo->env,
+                    arginfo->help);
+        }
+    }
+
+    printf("\n"
+           "Defaults:\n"
+           "QEMU_LD_PREFIX  = %s\n"
+           "QEMU_STACK_SIZE = %ld byte\n"
+           "QEMU_LOG        = %s\n",
+           interp_prefix,
+           guest_stack_size,
+           DEBUG_LOGFILE);
+
+    printf("\n"
+           "You can use -E and -U options or the QEMU_SET_ENV and\n"
+           "QEMU_UNSET_ENV environment variables to set and unset\n"
+           "environment variables for the target process.\n"
+           "It is possible to provide several variables by separating them\n"
+           "by commas in getsubopt(3) style. Additionally it is possible to\n"
+           "provide the -E and -U options multiple times.\n"
+           "The following lines are equivalent:\n"
+           "    -E var1=val2 -E var2=val2 -U LD_PRELOAD -U LD_DEBUG\n"
+           "    -E var1=val2,var2=val2 -U LD_PRELOAD,LD_DEBUG\n"
+           "    QEMU_SET_ENV=var1=val2,var2=val2 QEMU_UNSET_ENV=LD_PRELOAD,LD_DEBUG\n"
+           "Note that if you provide several changes to a single variable\n"
+           "the last change will stay in effect.\n");
+
+    exit(1);
+}
+
+static int parse_args(int argc, char **argv, envlist_t *envlist)
+{
+    const char *r;
+    int optind;
+    struct qemu_argument *arginfo;
+
+    for (arginfo = arg_table; arginfo->handle_opt != NULL; arginfo++) {
+        if (arginfo->env == NULL) {
+            continue;
+        }
+
+        if ((r = getenv(arginfo->env)) != NULL) {
+            arginfo->handle_opt(r, envlist);
+        }
+    }
+
+    optind = 1;
+    for(;;) {
+        if (optind >= argc)
+            break;
+        r = argv[optind];
+        if (r[0] != '-')
+            break;
+        optind++;
+        r++;
+        if (!strcmp(r, "-")) {
+            break;
+        }
+
+        for (arginfo = arg_table; arginfo->handle_opt != NULL; arginfo++) {
+            if (!strcmp(r, arginfo->argv)) {
+                if (optind >= argc) {
+                    usage();
+                }
+
+                arginfo->handle_opt(argv[optind], envlist);
+
+                if (arginfo->has_arg) {
+                    optind++;
+                }
+
+                break;
+            }
+        }
+
+        // no option matched the current argv
+        if (arginfo->handle_opt == NULL) {
+            usage();
+        }
+    }
+
+    if (optind >= argc) {
+        usage();
+    }
+
+    filename = argv[optind];
+    exec_path = argv[optind];
+
+    return optind;
+}
+
 int main(int argc, char **argv, char **envp)
 {
-    const char *filename;
-    const char *cpu_model;
     struct target_pt_regs regs1, *regs = &regs1;
     struct image_info info1, *info = &info1;
     struct linux_binprm bprm;
     TaskState ts1, *ts = &ts1;
     CPUState *env;
     int optind;
-    const char *r;
-    int gdbstub_port = 0;
     char **target_environ, **wrk;
     char **target_argv;
     int target_argc;
     envlist_t *envlist = NULL;
-    const char *argv0 = NULL;
     int i;
     int ret;
 
@@ -2758,143 +3038,7 @@ int main(int argc, char **argv, char **envp)
     cpudef_setup(); /* parse cpu definitions in target config file (TBD) */
 #endif
 
-    optind = 1;
-    for(;;) {
-        if (optind >= argc)
-            break;
-        r = argv[optind];
-        if (r[0] != '-')
-            break;
-        optind++;
-        r++;
-        if (!strcmp(r, "-")) {
-            break;
-        } else if (!strcmp(r, "d")) {
-            int mask;
-            const CPULogItem *item;
-
-	    if (optind >= argc)
-		break;
-
-	    r = argv[optind++];
-            mask = cpu_str_to_log_mask(r);
-            if (!mask) {
-                printf("Log items (comma separated):\n");
-                for(item = cpu_log_items; item->mask != 0; item++) {
-                    printf("%-10s %s\n", item->name, item->help);
-                }
-                exit(1);
-            }
-            cpu_set_log(mask);
-        } else if (!strcmp(r, "E")) {
-            r = argv[optind++];
-            if (envlist_setenv(envlist, r) != 0)
-                usage();
-        } else if (!strcmp(r, "ignore-environment")) {
-            envlist_free(envlist);
-            if ((envlist = envlist_create()) == NULL) {
-                (void) fprintf(stderr, "Unable to allocate envlist\n");
-                exit(1);
-            }
-        } else if (!strcmp(r, "U")) {
-            r = argv[optind++];
-            if (envlist_unsetenv(envlist, r) != 0)
-                usage();
-        } else if (!strcmp(r, "0")) {
-            r = argv[optind++];
-            argv0 = r;
-        } else if (!strcmp(r, "s")) {
-            if (optind >= argc)
-                break;
-            r = argv[optind++];
-            guest_stack_size = strtoul(r, (char **)&r, 0);
-            if (guest_stack_size == 0)
-                usage();
-            if (*r == 'M')
-                guest_stack_size *= 1024 * 1024;
-            else if (*r == 'k' || *r == 'K')
-                guest_stack_size *= 1024;
-        } else if (!strcmp(r, "L")) {
-            interp_prefix = argv[optind++];
-        } else if (!strcmp(r, "p")) {
-            if (optind >= argc)
-                break;
-            qemu_host_page_size = atoi(argv[optind++]);
-            if (qemu_host_page_size == 0 ||
-                (qemu_host_page_size & (qemu_host_page_size - 1)) != 0) {
-                fprintf(stderr, "page size must be a power of two\n");
-                exit(1);
-            }
-        } else if (!strcmp(r, "g")) {
-            if (optind >= argc)
-                break;
-            gdbstub_port = atoi(argv[optind++]);
-	} else if (!strcmp(r, "r")) {
-	    qemu_uname_release = argv[optind++];
-        } else if (!strcmp(r, "cpu")) {
-            cpu_model = argv[optind++];
-            if (cpu_model == NULL || strcmp(cpu_model, "?") == 0) {
-/* XXX: implement xxx_cpu_list for targets that still miss it */
-#if defined(cpu_list_id)
-                cpu_list_id(stdout, &fprintf, "");
-#elif defined(cpu_list)
-                cpu_list(stdout, &fprintf); /* deprecated */
-#endif
-                exit(1);
-            }
-#if defined(CONFIG_USE_GUEST_BASE)
-        } else if (!strcmp(r, "B")) {
-           guest_base = strtol(argv[optind++], NULL, 0);
-           have_guest_base = 1;
-        } else if (!strcmp(r, "R")) {
-            char *p;
-            int shift = 0;
-            reserved_va = strtoul(argv[optind++], &p, 0);
-            switch (*p) {
-            case 'k':
-            case 'K':
-                shift = 10;
-                break;
-            case 'M':
-                shift = 20;
-                break;
-            case 'G':
-                shift = 30;
-                break;
-            }
-            if (shift) {
-                unsigned long unshifted = reserved_va;
-                p++;
-                reserved_va <<= shift;
-                if (((reserved_va >> shift) != unshifted)
-#if HOST_LONG_BITS > TARGET_VIRT_ADDR_SPACE_BITS
-                    || (reserved_va > (1ul << TARGET_VIRT_ADDR_SPACE_BITS))
-#endif
-                    ) {
-                    fprintf(stderr, "Reserved virtual address too big\n");
-                    exit(1);
-                }
-            }
-            if (*p) {
-                fprintf(stderr, "Unrecognised -R size suffix '%s'\n", p);
-                exit(1);
-            }
-#endif
-        } else if (!strcmp(r, "drop-ld-preload")) {
-            (void) envlist_unsetenv(envlist, "LD_PRELOAD");
-        } else if (!strcmp(r, "singlestep")) {
-            singlestep = 1;
-        } else if (!strcmp(r, "strace")) {
-            do_strace = 1;
-        } else
-        {
-            usage();
-        }
-    }
-    if (optind >= argc)
-        usage();
-    filename = argv[optind];
-    exec_path = argv[optind];
+    optind = parse_args(argc, argv, envlist);
 
     /* Zero out regs */
     memset(regs, 0, sizeof(struct target_pt_regs));
-- 
1.7.5.4





Information forwarded to debian-bugs-dist@lists.debian.org, Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>:
Bug#632192; Package qemu. (Fri, 05 Aug 2011 10:06:28 GMT) Full text and rfc822 format available.

Acknowledgement sent to Peter Maydell <peter.maydell@linaro.org>:
Extra info received and forwarded to list. Copy sent to Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>. (Fri, 05 Aug 2011 10:06:32 GMT) Full text and rfc822 format available.

Message #112 received at 632192@bugs.debian.org (full text, mbox):

From: Peter Maydell <peter.maydell@linaro.org>
To: Johannes Schauer <j.schauer@email.de>
Cc: Riku Voipio <riku.voipio@iki.fi>, Vagrant Cascadian <vagrant@freegeek.org>, 632192@bugs.debian.org, qemu-devel@nongnu.org
Subject: Re: [PATCH] introduce environment variables for all qemu-user options
Date: Fri, 5 Aug 2011 11:04:59 +0100
On 31 July 2011 22:40, Johannes Schauer <j.schauer@email.de> wrote:
> Rework option parsing code for linux-user in a table-driven manner to allow
> environment variables for all commandline options.
>
> Also generate usage() output from option table.

Thanks for this, it looks good. A couple of minor points:
(1) what's the rationale for having most of the things the
arg-parsing-functions change be global variables but passing
envlist as an argument to each function? It's only used by
two of the functions so maybe that should just be a global too.
(2) scripts/checkpatch.pl complains about a number of formatting
nits; mostly these are in existing code you've just moved around,
but I think it's worth fixing them in passing anyway.

-- PMM




Information forwarded to debian-bugs-dist@lists.debian.org, Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>:
Bug#632192; Package qemu. (Sat, 06 Aug 2011 06:57:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Johannes Schauer <j.schauer@email.de>:
Extra info received and forwarded to list. Copy sent to Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>. (Sat, 06 Aug 2011 06:57:03 GMT) Full text and rfc822 format available.

Message #117 received at 632192@bugs.debian.org (full text, mbox):

From: Johannes Schauer <j.schauer@email.de>
To: Peter Maydell <peter.maydell@linaro.org>, qemu-devel@nongnu.org, 632192@bugs.debian.org, Riku Voipio <riku.voipio@iki.fi>, Vagrant Cascadian <vagrant@freegeek.org>
Cc: Johannes Schauer <j.schauer@email.de>
Subject: [PATCH] introduce environment variables for all qemu-user options
Date: Sat, 6 Aug 2011 08:54:12 +0200
Rework option parsing code for linux-user in a table-driven manner to allow
environment variables for all commandline options.

Also generate usage() output from option table.

Fix complains from checkpatch.pl, also have envlist global


Signed-off-by: Johannes Schauer <j.schauer@email.de>
---
 linux-user/main.c |  530 ++++++++++++++++++++++++++++++++++-------------------
 1 files changed, 342 insertions(+), 188 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index dbba8be..74c5198 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -40,6 +40,11 @@
 char *exec_path;
 
 int singlestep;
+const char *filename;
+const char *argv0;
+int gdbstub_port;
+envlist_t *envlist;
+const char *cpu_model;
 unsigned long mmap_min_addr;
 #if defined(CONFIG_USE_GUEST_BASE)
 unsigned long guest_base;
@@ -47,6 +52,8 @@ int have_guest_base;
 unsigned long reserved_va;
 #endif
 
+static void usage(void);
+
 static const char *interp_prefix = CONFIG_QEMU_INTERP_PREFIX;
 const char *qemu_uname_release = CONFIG_UNAME_RELEASE;
 
@@ -2624,50 +2631,6 @@ void cpu_loop (CPUState *env)
 }
 #endif /* TARGET_ALPHA */
 
-static void usage(void)
-{
-    printf("qemu-" TARGET_ARCH " version " QEMU_VERSION QEMU_PKGVERSION ", Copyright (c) 2003-2008 Fabrice Bellard\n"
-           "usage: qemu-" TARGET_ARCH " [options] program [arguments...]\n"
-           "Linux CPU emulator (compiled for %s emulation)\n"
-           "\n"
-           "Standard options:\n"
-           "-h                print this help\n"
-           "-g port           wait gdb connection to port\n"
-           "-L path           set the elf interpreter prefix (default=%s)\n"
-           "-s size           set the stack size in bytes (default=%ld)\n"
-           "-cpu model        select CPU (-cpu ? for list)\n"
-           "-drop-ld-preload  drop LD_PRELOAD for target process\n"
-           "-E var=value      sets/modifies targets environment variable(s)\n"
-           "-U var            unsets targets environment variable(s)\n"
-           "-0 argv0          forces target process argv[0] to be argv0\n"
-#if defined(CONFIG_USE_GUEST_BASE)
-           "-B address        set guest_base address to address\n"
-           "-R size           reserve size bytes for guest virtual address space\n"
-#endif
-           "\n"
-           "Debug options:\n"
-           "-d options   activate log (logfile=%s)\n"
-           "-p pagesize  set the host page size to 'pagesize'\n"
-           "-singlestep  always run in singlestep mode\n"
-           "-strace      log system calls\n"
-           "\n"
-           "Environment variables:\n"
-           "QEMU_STRACE       Print system calls and arguments similar to the\n"
-           "                  'strace' program.  Enable by setting to any value.\n"
-           "You can use -E and -U options to set/unset environment variables\n"
-           "for target process.  It is possible to provide several variables\n"
-           "by repeating the option.  For example:\n"
-           "    -E var1=val2 -E var2=val2 -U LD_PRELOAD -U LD_DEBUG\n"
-           "Note that if you provide several changes to single variable\n"
-           "last change will stay in effect.\n"
-           ,
-           TARGET_ARCH,
-           interp_prefix,
-           guest_stack_size,
-           DEBUG_LOGFILE);
-    exit(1);
-}
-
 THREAD CPUState *thread_env;
 
 void task_settid(TaskState *ts)
@@ -2703,24 +2666,351 @@ void init_task_state(TaskState *ts)
     }
     ts->sigqueue_table[i].next = NULL;
 }
- 
+
+static void handle_arg_help(const char *arg)
+{
+    usage();
+}
+
+static void handle_arg_log(const char *arg)
+{
+    int mask;
+    const CPULogItem *item;
+
+    mask = cpu_str_to_log_mask(arg);
+    if (!mask) {
+        printf("Log items (comma separated):\n");
+        for (item = cpu_log_items; item->mask != 0; item++) {
+            printf("%-10s %s\n", item->name, item->help);
+        }
+        exit(1);
+    }
+    cpu_set_log(mask);
+}
+
+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();
+        }
+    }
+    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();
+        }
+    }
+    free(r);
+}
+
+static void handle_arg_argv0(const char *arg)
+{
+    argv0 = strdup(arg);
+}
+
+static void handle_arg_stack_size(const char *arg)
+{
+    char *p;
+    guest_stack_size = strtoul(arg, &p, 0);
+    if (guest_stack_size == 0) {
+        usage();
+    }
+
+    if (*p == 'M') {
+        guest_stack_size *= 1024 * 1024;
+    } else if (*p == 'k' || *p == 'K') {
+        guest_stack_size *= 1024;
+    }
+}
+
+static void handle_arg_ld_prefix(const char *arg)
+{
+    interp_prefix = strdup(arg);
+}
+
+static void handle_arg_pagesize(const char *arg)
+{
+    qemu_host_page_size = atoi(arg);
+    if (qemu_host_page_size == 0 ||
+        (qemu_host_page_size & (qemu_host_page_size - 1)) != 0) {
+        fprintf(stderr, "page size must be a power of two\n");
+        exit(1);
+    }
+}
+
+static void handle_arg_gdb(const char *arg)
+{
+    gdbstub_port = atoi(arg);
+}
+
+static void handle_arg_uname(const char *arg)
+{
+    qemu_uname_release = strdup(arg);
+}
+
+static void handle_arg_cpu(const char *arg)
+{
+    cpu_model = strdup(arg);
+    if (cpu_model == NULL || strcmp(cpu_model, "?") == 0) {
+        /* XXX: implement xxx_cpu_list for targets that still miss it */
+#if defined(cpu_list_id)
+        cpu_list_id(stdout, &fprintf, "");
+#elif defined(cpu_list)
+        cpu_list(stdout, &fprintf); /* deprecated */
+#endif
+        exit(1);
+    }
+}
+
+#if defined(CONFIG_USE_GUEST_BASE)
+static void handle_arg_guest_base(const char *arg)
+{
+    guest_base = strtol(arg, NULL, 0);
+    have_guest_base = 1;
+}
+
+static void handle_arg_reserved_va(const char *arg)
+{
+    char *p;
+    int shift = 0;
+    reserved_va = strtoul(arg, &p, 0);
+    switch (*p) {
+    case 'k':
+    case 'K':
+        shift = 10;
+        break;
+    case 'M':
+        shift = 20;
+        break;
+    case 'G':
+        shift = 30;
+        break;
+    }
+    if (shift) {
+        unsigned long unshifted = reserved_va;
+        p++;
+        reserved_va <<= shift;
+        if (((reserved_va >> shift) != unshifted)
+#if HOST_LONG_BITS > TARGET_VIRT_ADDR_SPACE_BITS
+            || (reserved_va > (1ul << TARGET_VIRT_ADDR_SPACE_BITS))
+#endif
+            ) {
+            fprintf(stderr, "Reserved virtual address too big\n");
+            exit(1);
+        }
+    }
+    if (*p) {
+        fprintf(stderr, "Unrecognised -R size suffix '%s'\n", p);
+        exit(1);
+    }
+}
+#endif
+
+static void handle_arg_singlestep(const char *arg)
+{
+    singlestep = 1;
+}
+
+static void handle_arg_strace(const char *arg)
+{
+    do_strace = 1;
+}
+
+struct qemu_argument {
+    const char *argv;
+    const char *env;
+    bool has_arg;
+    void (*handle_opt)(const char *arg);
+    const char *example;
+    const char *help;
+};
+
+struct qemu_argument arg_table[] = {
+    {"h",          "",                 false, handle_arg_help,
+     "",           "print this help"},
+    {"g",          "QEMU_GDB",         true,  handle_arg_gdb,
+     "port",       "wait gdb connection to 'port'"},
+    {"L",          "QEMU_LD_PREFIX",   true,  handle_arg_ld_prefix,
+     "path",       "set the elf interpreter prefix to 'path'"},
+    {"s",          "QEMU_STACK_SIZE",  true,  handle_arg_stack_size,
+     "size",       "set the stack size to 'size' bytes"},
+    {"cpu",        "QEMU_CPU",         true,  handle_arg_cpu,
+     "model",      "select CPU (-cpu ? for list)"},
+    {"E",          "QEMU_SET_ENV",     true,  handle_arg_set_env,
+     "var=value",  "sets targets environment variable (see below)"},
+    {"U",          "QEMU_UNSET_ENV",   true,  handle_arg_unset_env,
+     "var",        "unsets targets environment variable (see below)"},
+    {"0",          "QEMU_ARGV0",       true,  handle_arg_argv0,
+     "argv0",      "forces target process argv[0] to be 'argv0'"},
+    {"r",          "QEMU_UNAME",       true,  handle_arg_uname,
+     "uname",      "set qemu uname release string to 'uname'"},
+#if defined(CONFIG_USE_GUEST_BASE)
+    {"B",          "QEMU_GUEST_BASE",  true,  handle_arg_guest_base,
+     "address",    "set guest_base address to 'address'"},
+    {"R",          "QEMU_RESERVED_VA", true,  handle_arg_reserved_va,
+     "size",       "reserve 'size' bytes for guest virtual address space"},
+#endif
+    {"d",          "QEMU_LOG",         true,  handle_arg_log,
+     "options",    "activate log"},
+    {"p",          "QEMU_PAGESIZE",    true,  handle_arg_pagesize,
+     "pagesize",   "set the host page size to 'pagesize'"},
+    {"singlestep", "QEMU_SINGLESTEP",  false, handle_arg_singlestep,
+     "",           "run in singlestep mode"},
+    {"strace",     "QEMU_STRACE",      false, handle_arg_strace,
+     "",           "log system calls"},
+    {NULL, NULL, false, NULL, NULL, NULL}
+};
+
+static void usage(void)
+{
+    struct qemu_argument *arginfo;
+    int maxarglen;
+    int maxenvlen;
+
+    printf("qemu-" TARGET_ARCH " version " QEMU_VERSION QEMU_PKGVERSION
+           ", Copyright (c) 2003-2008 Fabrice Bellard\n"
+           "usage: qemu-" TARGET_ARCH " [options] program [arguments...]\n"
+           "Linux CPU emulator (compiled for " TARGET_ARCH " emulation)\n"
+           "\n"
+           "Options and associated environment variables:\n"
+           "\n");
+
+    maxarglen = maxenvlen = 0;
+
+    for (arginfo = arg_table; arginfo->handle_opt != NULL; arginfo++) {
+        if (strlen(arginfo->env) > maxenvlen) {
+            maxenvlen = strlen(arginfo->env);
+        }
+        if (strlen(arginfo->argv) > maxarglen) {
+            maxarglen = strlen(arginfo->argv);
+        }
+    }
+
+    printf("%-*s%-*sDescription\n", maxarglen+3, "Argument",
+            maxenvlen+1, "Env-variable");
+
+    for (arginfo = arg_table; arginfo->handle_opt != NULL; arginfo++) {
+        if (arginfo->has_arg) {
+            printf("-%s %-*s %-*s %s\n", arginfo->argv,
+                    (int)(maxarglen-strlen(arginfo->argv)), arginfo->example,
+                    maxenvlen, arginfo->env, arginfo->help);
+        } else {
+            printf("-%-*s %-*s %s\n", maxarglen+1, arginfo->argv,
+                    maxenvlen, arginfo->env,
+                    arginfo->help);
+        }
+    }
+
+    printf("\n"
+           "Defaults:\n"
+           "QEMU_LD_PREFIX  = %s\n"
+           "QEMU_STACK_SIZE = %ld byte\n"
+           "QEMU_LOG        = %s\n",
+           interp_prefix,
+           guest_stack_size,
+           DEBUG_LOGFILE);
+
+    printf("\n"
+           "You can use -E and -U options or the QEMU_SET_ENV and\n"
+           "QEMU_UNSET_ENV environment variables to set and unset\n"
+           "environment variables for the target process.\n"
+           "It is possible to provide several variables by separating them\n"
+           "by commas in getsubopt(3) style. Additionally it is possible to\n"
+           "provide the -E and -U options multiple times.\n"
+           "The following lines are equivalent:\n"
+           "    -E var1=val2 -E var2=val2 -U LD_PRELOAD -U LD_DEBUG\n"
+           "    -E var1=val2,var2=val2 -U LD_PRELOAD,LD_DEBUG\n"
+           "    QEMU_SET_ENV=var1=val2,var2=val2 QEMU_UNSET_ENV=LD_PRELOAD,LD_DEBUG\n"
+           "Note that if you provide several changes to a single variable\n"
+           "the last change will stay in effect.\n");
+
+    exit(1);
+}
+
+static int parse_args(int argc, char **argv)
+{
+    const char *r;
+    int optind;
+    struct qemu_argument *arginfo;
+
+    for (arginfo = arg_table; arginfo->handle_opt != NULL; arginfo++) {
+        if (arginfo->env == NULL) {
+            continue;
+        }
+
+        r = getenv(arginfo->env);
+        if (r != NULL) {
+            arginfo->handle_opt(r);
+        }
+    }
+
+    optind = 1;
+    for (;;) {
+        if (optind >= argc) {
+            break;
+        }
+        r = argv[optind];
+        if (r[0] != '-') {
+            break;
+        }
+        optind++;
+        r++;
+        if (!strcmp(r, "-")) {
+            break;
+        }
+
+        for (arginfo = arg_table; arginfo->handle_opt != NULL; arginfo++) {
+            if (!strcmp(r, arginfo->argv)) {
+                if (optind >= argc) {
+                    usage();
+                }
+
+                arginfo->handle_opt(argv[optind]);
+
+                if (arginfo->has_arg) {
+                    optind++;
+                }
+
+                break;
+            }
+        }
+
+        /* no option matched the current argv */
+        if (arginfo->handle_opt == NULL) {
+            usage();
+        }
+    }
+
+    if (optind >= argc) {
+        usage();
+    }
+
+    filename = argv[optind];
+    exec_path = argv[optind];
+
+    return optind;
+}
+
 int main(int argc, char **argv, char **envp)
 {
-    const char *filename;
-    const char *cpu_model;
     struct target_pt_regs regs1, *regs = &regs1;
     struct image_info info1, *info = &info1;
     struct linux_binprm bprm;
     TaskState ts1, *ts = &ts1;
     CPUState *env;
     int optind;
-    const char *r;
-    int gdbstub_port = 0;
     char **target_environ, **wrk;
     char **target_argv;
     int target_argc;
-    envlist_t *envlist = NULL;
-    const char *argv0 = NULL;
     int i;
     int ret;
 
@@ -2758,143 +3048,7 @@ int main(int argc, char **argv, char **envp)
     cpudef_setup(); /* parse cpu definitions in target config file (TBD) */
 #endif
 
-    optind = 1;
-    for(;;) {
-        if (optind >= argc)
-            break;
-        r = argv[optind];
-        if (r[0] != '-')
-            break;
-        optind++;
-        r++;
-        if (!strcmp(r, "-")) {
-            break;
-        } else if (!strcmp(r, "d")) {
-            int mask;
-            const CPULogItem *item;
-
-	    if (optind >= argc)
-		break;
-
-	    r = argv[optind++];
-            mask = cpu_str_to_log_mask(r);
-            if (!mask) {
-                printf("Log items (comma separated):\n");
-                for(item = cpu_log_items; item->mask != 0; item++) {
-                    printf("%-10s %s\n", item->name, item->help);
-                }
-                exit(1);
-            }
-            cpu_set_log(mask);
-        } else if (!strcmp(r, "E")) {
-            r = argv[optind++];
-            if (envlist_setenv(envlist, r) != 0)
-                usage();
-        } else if (!strcmp(r, "ignore-environment")) {
-            envlist_free(envlist);
-            if ((envlist = envlist_create()) == NULL) {
-                (void) fprintf(stderr, "Unable to allocate envlist\n");
-                exit(1);
-            }
-        } else if (!strcmp(r, "U")) {
-            r = argv[optind++];
-            if (envlist_unsetenv(envlist, r) != 0)
-                usage();
-        } else if (!strcmp(r, "0")) {
-            r = argv[optind++];
-            argv0 = r;
-        } else if (!strcmp(r, "s")) {
-            if (optind >= argc)
-                break;
-            r = argv[optind++];
-            guest_stack_size = strtoul(r, (char **)&r, 0);
-            if (guest_stack_size == 0)
-                usage();
-            if (*r == 'M')
-                guest_stack_size *= 1024 * 1024;
-            else if (*r == 'k' || *r == 'K')
-                guest_stack_size *= 1024;
-        } else if (!strcmp(r, "L")) {
-            interp_prefix = argv[optind++];
-        } else if (!strcmp(r, "p")) {
-            if (optind >= argc)
-                break;
-            qemu_host_page_size = atoi(argv[optind++]);
-            if (qemu_host_page_size == 0 ||
-                (qemu_host_page_size & (qemu_host_page_size - 1)) != 0) {
-                fprintf(stderr, "page size must be a power of two\n");
-                exit(1);
-            }
-        } else if (!strcmp(r, "g")) {
-            if (optind >= argc)
-                break;
-            gdbstub_port = atoi(argv[optind++]);
-	} else if (!strcmp(r, "r")) {
-	    qemu_uname_release = argv[optind++];
-        } else if (!strcmp(r, "cpu")) {
-            cpu_model = argv[optind++];
-            if (cpu_model == NULL || strcmp(cpu_model, "?") == 0) {
-/* XXX: implement xxx_cpu_list for targets that still miss it */
-#if defined(cpu_list_id)
-                cpu_list_id(stdout, &fprintf, "");
-#elif defined(cpu_list)
-                cpu_list(stdout, &fprintf); /* deprecated */
-#endif
-                exit(1);
-            }
-#if defined(CONFIG_USE_GUEST_BASE)
-        } else if (!strcmp(r, "B")) {
-           guest_base = strtol(argv[optind++], NULL, 0);
-           have_guest_base = 1;
-        } else if (!strcmp(r, "R")) {
-            char *p;
-            int shift = 0;
-            reserved_va = strtoul(argv[optind++], &p, 0);
-            switch (*p) {
-            case 'k':
-            case 'K':
-                shift = 10;
-                break;
-            case 'M':
-                shift = 20;
-                break;
-            case 'G':
-                shift = 30;
-                break;
-            }
-            if (shift) {
-                unsigned long unshifted = reserved_va;
-                p++;
-                reserved_va <<= shift;
-                if (((reserved_va >> shift) != unshifted)
-#if HOST_LONG_BITS > TARGET_VIRT_ADDR_SPACE_BITS
-                    || (reserved_va > (1ul << TARGET_VIRT_ADDR_SPACE_BITS))
-#endif
-                    ) {
-                    fprintf(stderr, "Reserved virtual address too big\n");
-                    exit(1);
-                }
-            }
-            if (*p) {
-                fprintf(stderr, "Unrecognised -R size suffix '%s'\n", p);
-                exit(1);
-            }
-#endif
-        } else if (!strcmp(r, "drop-ld-preload")) {
-            (void) envlist_unsetenv(envlist, "LD_PRELOAD");
-        } else if (!strcmp(r, "singlestep")) {
-            singlestep = 1;
-        } else if (!strcmp(r, "strace")) {
-            do_strace = 1;
-        } else
-        {
-            usage();
-        }
-    }
-    if (optind >= argc)
-        usage();
-    filename = argv[optind];
-    exec_path = argv[optind];
+    optind = parse_args(argc, argv);
 
     /* Zero out regs */
     memset(regs, 0, sizeof(struct target_pt_regs));
-- 
1.7.5.4





Information forwarded to debian-bugs-dist@lists.debian.org, Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>:
Bug#632192; Package qemu. (Sat, 20 Aug 2011 17:33:07 GMT) Full text and rfc822 format available.

Acknowledgement sent to Yann Dirson <ydirson@free.fr>:
Extra info received and forwarded to list. Copy sent to Debian QEMU Team <pkg-qemu-devel@lists.alioth.debian.org>. (Sat, 20 Aug 2011 17:33:07 GMT) Full text and rfc822 format available.

Message #122 received at 632192@bugs.debian.org (full text, mbox):

From: Yann Dirson <ydirson@free.fr>
To: Johannes Schauer <j.schauer@email.de>
Cc: 632192@bugs.debian.org, Peter Maydell <peter.maydell@linaro.org>, qemu-devel@nongnu.org, Riku Voipio <riku.voipio@iki.fi>, Vagrant Cascadian <vagrant@freegeek.org>
Subject: Re: [PATCH] introduce environment variables for all qemu-user options
Date: Sat, 20 Aug 2011 19:29:47 +0200
This patch will be useful, but there is a security problem in its
current form.  The qemu-user-static package installs binfmt-misc
entries with "flags: OC", which makes the binary honor setuid bits.

Regardless of whether it is a good idea or not, the envvars ought to
be ignored in such a case.  Some clever checks using getresuid(), or
just geteuid() and getuid() when getresuid() is not available, surely
have to done.  There is probably some existing code for this in other
programs...

Best regards,
-- 
Yann




Reply sent to Vagrant Cascadian <vagrant@freegeek.org>:
You have taken responsibility. (Fri, 16 Dec 2011 20:27:04 GMT) Full text and rfc822 format available.

Notification sent to Johannes Schauer <j.schauer@email.de>:
Bug acknowledged by developer. (Fri, 16 Dec 2011 20:27:04 GMT) Full text and rfc822 format available.

Message #127 received at 632192-done@bugs.debian.org (full text, mbox):

From: Vagrant Cascadian <vagrant@freegeek.org>
To: 632192-done@bugs.debian.org
Subject: Bug#632192: qemu: get elf interpreter prefix from environment variable
Date: Fri, 16 Dec 2011 12:24:57 -0800
Version: 1.0~rc4+dfsg-1

fixed in new upstream versions:

commit fc9c54124d134dbd76338a92a91804dab2df8166
Author: Johannes Schauer <j.schauer@email.de>
Date:   Sat Aug 6 08:54:12 2011 +0200

    introduce environment variables for all qemu-user options

    (Edits by Riku Voipio to apply to current HEAD)

    Rework option parsing code for linux-user in a table-driven manner to allow
    environment variables for all commandline options.

    Also generate usage() output from option table.

    Fix complains from checkpatch.pl, also have envlist global

    Signed-off-by: Johannes Schauer <j.schauer@email.de>
    Signed-off-by: Riku Voipio <riku.voipio@linaro.org>

Thanks for making it happen!

live well,
  vagrant




Bug archived. Request was from Debbugs Internal Request <owner@bugs.debian.org> to internal_control@bugs.debian.org. (Fri, 17 Feb 2012 07:42:53 GMT) Full text and rfc822 format available.

Send a report that this bug log contains spam.


Debian bug tracking system administrator <owner@bugs.debian.org>. Last modified: Fri Apr 18 19:36:09 2014; Machine Name: buxtehude.debian.org

Debian Bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.