[Qemu-devel] [PATCH] Allow setting qemu process name

[Qemu-devel] [PATCH] Allow setting qemu process name

[Andi Kleen]

[this is a port of a old KVM userland patch I had; Avi back then suggested to 
submit it to qemu]

Set the Linux process name to the name argument specified with "-name". I find
this useful to see which guests are taking CPU time in top.

This doesn't affect ps, which checks argv[0], but rewriting the 
environment uses much more code, so I only used this simple way.

Signed-off-by: Andi Kleen <ak@linux.intel.com>

diff --git a/vl.c b/vl.c
index 7b7489c..584c48e 100644
--- a/vl.c
+++ b/vl.c
@@ -68,6 +68,7 @@
 #include <pty.h>
 #include <malloc.h>
 #include <linux/rtc.h>
+#include <sys/prctl.h>
 
 /* For the benefit of older linux systems which don't supply it,
    we use a local copy of hpet.h. */
@@ -526,6 +527,19 @@ void hw_error(const char *fmt, ...)
     va_end(ap);
     abort();
 }
+
+static void set_proc_name(const char *prefix, const char *s)
+{
+#ifdef __linux__
+   char name[16];
+   if (!s)
+       return;
+   /* Could rewrite argv[0] too, but that's a bit more complicated.
+      This simple way is enough for `top'. */
+    snprintf(name, sizeof name, "%s-%.10s", prefix, s);
+    prctl(PR_SET_NAME, name);
+#endif    	
+}
  
 /***************/
 /* ballooning */
@@ -6008,11 +6022,15 @@ int main(int argc, char **argv, char **envp)
     if (kvm_enabled()) {
         int ret;
 
+        set_proc_name("kvm", qemu_name);
+
         ret = kvm_init(smp_cpus);
         if (ret < 0) {
             fprintf(stderr, "failed to initialize KVM\n");
             exit(1);
         }
+    } else {
+        set_proc_name("qemu", qemu_name);
     }
 
     if (monitor_device) {
-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [Qemu-devel] [PATCH] Allow setting qemu process name

[Anthony Liguori]

Andi Kleen wrote:
> [this is a port of a old KVM userland patch I had; Avi back then suggested to 
> submit it to qemu]
>
> Set the Linux process name to the name argument specified with "-name". I find
> this useful to see which guests are taking CPU time in top.
>   

This is not a bad idea but it has to be optional and non-default.  Maybe 
a new command line option?

>  /***************/
>  /* ballooning */
> @@ -6008,11 +6022,15 @@ int main(int argc, char **argv, char **envp)
>      if (kvm_enabled()) {
>          int ret;
>  
> +        set_proc_name("kvm", qemu_name);
> +
>          ret = kvm_init(smp_cpus);
>          if (ret < 0) {
>              fprintf(stderr, "failed to initialize KVM\n");
>              exit(1);
>          }
> +    } else {
> +        set_proc_name("qemu", qemu_name);
>      }

I'd rather see qemu be the prefix unconditionally.

Regards,

Anthony Liguori

>  
>      if (monitor_device) {
>   

Re: [Qemu-devel] [PATCH] Allow setting qemu process name

[Andi Kleen]

On Wed, Jul 01, 2009 at 08:29:22AM -0500, Anthony Liguori wrote:
> Andi Kleen wrote:
> >[this is a port of a old KVM userland patch I had; Avi back then suggested 
> >to submit it to qemu]
> >
> >Set the Linux process name to the name argument specified with "-name". I 
> >find
> >this useful to see which guests are taking CPU time in top.
> >  
> 
> This is not a bad idea but it has to be optional and non-default.  Maybe 
> a new command line option?

It's only default with -name. I don't think anyone would look through
/proc matching for process names, that would be too broken anyways.
If they look through ps ax they would still work.

Or what are you worried about that it could break?

> I'd rather see qemu be the prefix unconditionally.

Ok.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [Qemu-devel] [PATCH] Allow setting qemu process name

[Anthony Liguori]

Andi Kleen wrote:
> On Wed, Jul 01, 2009 at 08:29:22AM -0500, Anthony Liguori wrote:
>   
> It's only default with -name. I don't think anyone would look through
> /proc matching for process names, that would be too broken anyways.
> If they look through ps ax they would still work.
>   

If someone has a script today that uses top -n 1 | grep 
qemu-system-x86_64 that script will break.

Since it's also linux-specific behavior, having it enabled by default 
means a subtle difference between platforms.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH] Allow setting qemu process name

[Andi Kleen]

On Wed, Jul 01, 2009 at 08:57:47AM -0500, Anthony Liguori wrote:
> Andi Kleen wrote:
> >On Wed, Jul 01, 2009 at 08:29:22AM -0500, Anthony Liguori wrote:
> >  
> >It's only default with -name. I don't think anyone would look through
> >/proc matching for process names, that would be too broken anyways.
> >If they look through ps ax they would still work.
> >  
> 
> If someone has a script today that uses top -n 1 | grep 
> qemu-system-x86_64 that script will break.

That would already break if qemu-system-x86_64 is by chance not in 
the ~20 or so processes that take the most CPU time. That is what I meant
with already broken. I consider it very likely that the scripts
all use ps at least, which is not affected.

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [Qemu-devel] [PATCH] Allow setting qemu process name

[Avi Kivity]

On 07/01/2009 05:06 PM, Andi Kleen wrote:
> On Wed, Jul 01, 2009 at 08:57:47AM -0500, Anthony Liguori wrote:
>    
>> Andi Kleen wrote:
>>      
>>> On Wed, Jul 01, 2009 at 08:29:22AM -0500, Anthony Liguori wrote:
>>>
>>> It's only default with -name. I don't think anyone would look through
>>> /proc matching for process names, that would be too broken anyways.
>>> If they look through ps ax they would still work.
>>>
>>>        
>> If someone has a script today that uses top -n 1 | grep
>> qemu-system-x86_64 that script will break.
>>      
>
> That would already break if qemu-system-x86_64 is by chance not in
> the ~20 or so processes that take the most CPU time. That is what I meant
> with already broken. I consider it very likely that the scripts
> all use ps at least, which is not affected.
>    

I often use 'pkill qemu', but no idea which process name that looks at.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH] Allow setting qemu process name

[Jamie Lokier]

Andi Kleen wrote:
> On Wed, Jul 01, 2009 at 08:57:47AM -0500, Anthony Liguori wrote:
> > Andi Kleen wrote:
> > >On Wed, Jul 01, 2009 at 08:29:22AM -0500, Anthony Liguori wrote:
> > >  
> > >It's only default with -name. I don't think anyone would look through
> > >/proc matching for process names, that would be too broken anyways.
> > >If they look through ps ax they would still work.
> > >  
> > 
> > If someone has a script today that uses top -n 1 | grep 
> > qemu-system-x86_64 that script will break.
> 
> That would already break if qemu-system-x86_64 is by chance not in 
> the ~20 or so processes that take the most CPU time. That is what I meant
> with already broken. I consider it very likely that the scripts
> all use ps at least, which is not affected.

These's also ps's "c" option:

    "Show the true command name. This is derived from the name of the
    executable file, rather than from the argv value. Command
    arguments and any modifications to them (see setproctitle(3)) are
    thus not shown."

Looks liks Linux has 3 different command names to choose from:

    - argv[0]
    - PR_SET_NAME
    - Name derived from /proc/self/exe

-- Jamie

Re: [Qemu-devel] [PATCH] Allow setting qemu process name

[Avi Kivity]

On 07/01/2009 04:29 PM, Anthony Liguori wrote:
> Andi Kleen wrote:
>> [this is a port of a old KVM userland patch I had; Avi back then 
>> suggested to submit it to qemu]
>>
>> Set the Linux process name to the name argument specified with 
>> "-name". I find
>> this useful to see which guests are taking CPU time in top.
>
> This is not a bad idea but it has to be optional and non-default.  
> Maybe a new command line option?

-name [title=]blah[,process=foobar]

> +    } else {
> +        set_proc_name("qemu", qemu_name);
>      }
>
> I'd rather see qemu be the prefix unconditionally.

Why have a prefix at all?  The user can add it if they're so inclined.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH] Allow setting qemu process name

[Luiz Capitulino]

On Wed, 01 Jul 2009 18:07:50 +0300
Avi Kivity <avi@redhat.com> wrote:

> On 07/01/2009 04:29 PM, Anthony Liguori wrote:
> > Andi Kleen wrote:
> >> [this is a port of a old KVM userland patch I had; Avi back then 
> >> suggested to submit it to qemu]
> >>
> >> Set the Linux process name to the name argument specified with 
> >> "-name". I find
> >> this useful to see which guests are taking CPU time in top.
> >
> > This is not a bad idea but it has to be optional and non-default.  
> > Maybe a new command line option?
> 
> -name [title=]blah[,process=foobar]

 Just a question: do we have a plan on stopping/avoiding adding new
command-line options?

Re: [Qemu-devel] [PATCH] Allow setting qemu process name

[Avi Kivity]

On 07/01/2009 07:19 PM, Luiz Capitulino wrote:
>>> Andi Kleen wrote:
>>>        
>>>> [this is a port of a old KVM userland patch I had; Avi back then
>>>> suggested to submit it to qemu]
>>>>
>>>> Set the Linux process name to the name argument specified with
>>>> "-name". I find
>>>> this useful to see which guests are taking CPU time in top.
>>>>          
>>> This is not a bad idea but it has to be optional and non-default.
>>> Maybe a new command line option?
>>>        
>> -name [title=]blah[,process=foobar]
>>      
>
>   Just a question: do we have a plan on stopping/avoiding adding new
> command-line options?
>    

Not at all.  But it makes sense to group related features into a single 
command line option IMO.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH] Allow setting qemu process name

[Luiz Capitulino]

On Wed, 01 Jul 2009 19:27:50 +0300
Avi Kivity <avi@redhat.com> wrote:

> On 07/01/2009 07:19 PM, Luiz Capitulino wrote:
> >>> Andi Kleen wrote:
> >>>        
> >>>> [this is a port of a old KVM userland patch I had; Avi back then
> >>>> suggested to submit it to qemu]
> >>>>
> >>>> Set the Linux process name to the name argument specified with
> >>>> "-name". I find
> >>>> this useful to see which guests are taking CPU time in top.
> >>>>          
> >>> This is not a bad idea but it has to be optional and non-default.
> >>> Maybe a new command line option?
> >>>        
> >> -name [title=]blah[,process=foobar]
> >>      
> >
> >   Just a question: do we have a plan on stopping/avoiding adding new
> > command-line options?
> >    
> 
> Not at all.  But it makes sense to group related features into a single 
> command line option IMO.

 Sure, it's seems better to me as well. I'm just curious on what we
are going to do for new features that requires configurable options.

 The command-line we have is already quite big...

Re: [Qemu-devel] [PATCH] Allow setting qemu process name

[Avi Kivity]

On 07/01/2009 07:34 PM, Luiz Capitulino wrote:
>   The command-line we have is already quite big...
>    

We can add a response file, for example '@my-vm-options.cfg' or 
'-options my-vm-options.cfg' (the first syntax is common on DOS and 
descendants).  It's nice as a baseline that you can override:

    qemu @my-vm -m 8G

starts a guest but specifies a different memory size.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH] Allow setting qemu process name

[Jamie Lokier]

Avi Kivity wrote:
> On 07/01/2009 07:34 PM, Luiz Capitulino wrote:
> >  The command-line we have is already quite big...
> >   
> 
> We can add a response file, for example '@my-vm-options.cfg' or 
> '-options my-vm-options.cfg' (the first syntax is common on DOS and 
> descendants).  It's nice as a baseline that you can override:
> 
>    qemu @my-vm -m 8G
> 
> starts a guest but specifies a different memory size.

We already have it:

    qemu `cat my-vm` -m 8G

MS-DOS applications have response files only because the shell doesn't.
We don't need it in unix.

-- Jamie

Re: [Qemu-devel] [PATCH] Allow setting qemu process name

[Stefan Weil]

Avi Kivity schrieb:
> On 07/01/2009 04:29 PM, Anthony Liguori wrote:
>> Andi Kleen wrote:
>>> [this is a port of a old KVM userland patch I had; Avi back then
>>> suggested to submit it to qemu]
>>>
>>> Set the Linux process name to the name argument specified with
>>> "-name". I find
>>> this useful to see which guests are taking CPU time in top.
>>
>> This is not a bad idea but it has to be optional and non-default. 
>> Maybe a new command line option?
>
> -name [title=]blah[,process=foobar]
>
>> +    } else {
>> +        set_proc_name("qemu", qemu_name);
>>      }
>>
>> I'd rather see qemu be the prefix unconditionally.
>
> Why have a prefix at all?  The user can add it if they're so inclined.
>

I'd prefer a solution without prefix, too.

The SDL caption is another place where the prefix
("QEMU (qemu_name)") is not needed.

Stefan

Re: [Qemu-devel] [PATCH] Allow setting qemu process name

[Todd T. Fries]

Penned by Andi Kleen on 20090701 11:32.52, we have:
| 
| [this is a port of a old KVM userland patch I had; Avi back then suggested to 
| submit it to qemu]
| 
| Set the Linux process name to the name argument specified with "-name". I find
| this useful to see which guests are taking CPU time in top.
| 
| This doesn't affect ps, which checks argv[0], but rewriting the 
| environment uses much more code, so I only used this simple way.
| 
| Signed-off-by: Andi Kleen <ak@linux.intel.com>
| 
| diff --git a/vl.c b/vl.c
| index 7b7489c..584c48e 100644
| --- a/vl.c
| +++ b/vl.c
| @@ -68,6 +68,7 @@
|  #include <pty.h>
|  #include <malloc.h>
|  #include <linux/rtc.h>
| +#include <sys/prctl.h>
|  
|  /* For the benefit of older linux systems which don't supply it,
|     we use a local copy of hpet.h. */
| @@ -526,6 +527,19 @@ void hw_error(const char *fmt, ...)
|      va_end(ap);
|      abort();
|  }
| +
| +static void set_proc_name(const char *prefix, const char *s)
| +{
| +#ifdef __linux__
| +   char name[16];
| +   if (!s)
| +       return;
| +   /* Could rewrite argv[0] too, but that's a bit more complicated.
| +      This simple way is enough for `top'. */
| +    snprintf(name, sizeof name, "%s-%.10s", prefix, s);
| +    prctl(PR_SET_NAME, name);
#elif defined(__OpenBSD__)
    if (!s)
        setproctitle("%s-%.10s", prefix, s);
| +#endif    	
| +}
|   
|  /***************/
|  /* ballooning */
| @@ -6008,11 +6022,15 @@ int main(int argc, char **argv, char **envp)
|      if (kvm_enabled()) {
|          int ret;
|  
| +        set_proc_name("kvm", qemu_name);
| +
|          ret = kvm_init(smp_cpus);
|          if (ret < 0) {
|              fprintf(stderr, "failed to initialize KVM\n");
|              exit(1);
|          }
| +    } else {
| +        set_proc_name("qemu", qemu_name);
|      }
|  
|      if (monitor_device) {
| -- 
| ak@linux.intel.com -- Speaking for myself only.
| 

-- 
Todd Fries .. todd@fries.net

 _____________________________________________
|                                             \  1.636.410.0632 (voice)
| Free Daemon Consulting, LLC                 \  1.405.227.9094 (voice)
| http://FreeDaemonConsulting.com             \  1.866.792.3418 (FAX)
| "..in support of free software solutions."  \  sip:freedaemon@ekiga.net
|                                             \  sip:4052279094@ekiga.net
 \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
                                                 
              37E7 D3EB 74D0 8D66 A68D  B866 0326 204E 3F42 004A
                        http://todd.fries.net/pgp.txt

Re: [Qemu-devel] [PATCH] Allow setting qemu process name

[Andi Kleen]

> | +static void set_proc_name(const char *prefix, const char *s)
> | +{
> | +#ifdef __linux__
> | +   char name[16];
> | +   if (!s)
> | +       return;
> | +   /* Could rewrite argv[0] too, but that's a bit more complicated.
> | +      This simple way is enough for `top'. */
> | +    snprintf(name, sizeof name, "%s-%.10s", prefix, s);
> | +    prctl(PR_SET_NAME, name);
> #elif defined(__OpenBSD__)
>     if (!s)
>         setproctitle("%s-%.10s", prefix, s);

Are you sure? A google codesearch on openbsd.org doesn't find
any references to PR_SET_NAME in the system.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [Qemu-devel] [PATCH] Allow setting qemu process name

[Todd T. Fries]

Penned by Andi Kleen on 20090723 14:01.42, we have:
| > | +static void set_proc_name(const char *prefix, const char *s)
| > | +{
| > | +#ifdef __linux__
| > | +   char name[16];
| > | +   if (!s)
| > | +       return;
| > | +   /* Could rewrite argv[0] too, but that's a bit more complicated.
| > | +      This simple way is enough for `top'. */
| > | +    snprintf(name, sizeof name, "%s-%.10s", prefix, s);
| > | +    prctl(PR_SET_NAME, name);
| > #elif defined(__OpenBSD__)
| >     if (!s)
| >         setproctitle("%s-%.10s", prefix, s);
| 
| Are you sure? A google codesearch on openbsd.org doesn't find
| any references to PR_SET_NAME in the system.
| 
| -Andi
| 
| -- 
| ak@linux.intel.com -- Speaking for myself only.

I didn't use PR_SET_NAME, read the code..

http://www.openbsd.org/cgi-bin/man.cgi?query=setproctitle
-- 
Todd Fries .. todd@fries.net

 _____________________________________________
|                                             \  1.636.410.0632 (voice)
| Free Daemon Consulting, LLC                 \  1.405.227.9094 (voice)
| http://FreeDaemonConsulting.com             \  1.866.792.3418 (FAX)
| "..in support of free software solutions."  \  sip:freedaemon@ekiga.net
|                                             \  sip:4052279094@ekiga.net
 \\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
                                                 
              37E7 D3EB 74D0 8D66 A68D  B866 0326 204E 3F42 004A
                        http://todd.fries.net/pgp.txt

Re: [Qemu-devel] [PATCH] Allow setting qemu process name

[Andi Kleen]

> I didn't use PR_SET_NAME, read the code..
> 
> http://www.openbsd.org/cgi-bin/man.cgi?query=setproctitle

Ok. Unfortunately it looks like the qemu maintainers were not interested
in my patch anyways.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [Qemu-devel] [PATCH] Allow setting qemu process name

[Richard W.M. Jones]

On Thu, Jul 23, 2009 at 06:47:16AM -0500, Todd T. Fries wrote:
> | +#ifdef __linux__
> | +   char name[16];
> | +   if (!s)
> | +       return;
> | +   /* Could rewrite argv[0] too, but that's a bit more complicated.
> | +      This simple way is enough for `top'. */
> | +    snprintf(name, sizeof name, "%s-%.10s", prefix, s);
> | +    prctl(PR_SET_NAME, name);
> #elif defined(__OpenBSD__)
>     if (!s)
>         setproctitle("%s-%.10s", prefix, s);
> | +#endif    	

In case anyone is interested, setting process titles for all Un*x-like
platforms is _really_ complex.  This is how PostgreSQL does it:

http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/misc/ps_status.c?rev=1.33.2.1;content-type=text%2Fplain

Rich.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v