[libvirt] process= support for 'qemu-kvm -name' [Bug 576950]

[Daniel P. Berrange]

On Mon, Oct 18, 2010 at 01:45:12PM -0400, John Morrissey wrote:
> [plowing through a stack of small patches that i've been neglecting...
>  sorry for the delay.]
> 
> On Tue, Mar 30, 2010 at 03:05:06PM +0100, Daniel P. Berrange wrote:
> > On Sun, Mar 28, 2010 at 10:19:51PM -0400, John Morrissey wrote:
> > > I wrote (attached here, and to the bug) a quick patch that sets the
> > > process name to the same value as the window title.
> [snip]
> > > @@ -3550,7 +3553,15 @@ int qemudBuildCommandLine(virConnectPtr conn,
> > >  
> > >      if (qemuCmdFlags & QEMUD_CMD_FLAG_NAME) {
> > >          ADD_ARG_LIT("-name");
> > > -        ADD_ARG_LIT(def->name);
> > > +        if (qemuCmdFlags & QEMUD_CMD_FLAG_NAME_PROCESS) {
> > > +            char *name;
> > > +            if (virAsprintf(&name, "%s,process=%s",
> > > +                            def->name, def->name) < 0)
> > > +                goto no_memory;
> > > +            ADD_ARG_LIT(name);
> > 
> > I think it will be quite misleading to do this. eg a VM named 'foo'
> > 
> >   # qemu-system-x86_64 -vnc :2 -hda /var/lib/libvirt/images/plain.img -name foo,process=foo
> > 
> > Now the process listing shows
> > 
> >   # ps -w
> >     PID TTY          TIME CMD
> >   12009 pts/1    00:00:01 bash
> >   12646 pts/1    00:00:00 ksmtuned
> >   14494 pts/1    00:00:02 foo
> >   14508 pts/1    00:00:00 sleep
> >   14511 pts/1    00:00:00 ps
> > 
> > which leaves no indication that 'foo' is a QEMU process at all which is 
> > rather bad IMHO. At the very least I think we should keep the binary base
> > name here, and have the VM name as a postfix, eg so it shows
> 
> Unfortunately, qemu uses prctl() to set the process title, which has a limit
> of 16 characters. How about "qemu:$VM_NAME" for the process title
> (attached), so we waste as little as possible?

Urgh, 16 characters is tiny :-(  I think we had better add a configuration
parameter to /etc/qemu/qemu.conf to allow admins to turn this feature on/off,
with the default being off. At least until QEMU can support a sensibly
sized process name...

> 
> I'll look at doing something to increase the length limitation in qemu.
> 
> john
> -- 
> John Morrissey          _o            /\         ----  __o
> jwm at horde.net        _-< \_          /  \       ----  <  \,
> www.horde.net/    __(_)/_(_)________/    \_______(_) /_(_)__

> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 83c0f83..d3eba0c 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -1165,8 +1165,11 @@ static unsigned long long qemudComputeCmdFlags(const char *help,
>          flags |= QEMUD_CMD_FLAG_ENABLE_KVM;
>      if (strstr(help, "-no-reboot"))
>          flags |= QEMUD_CMD_FLAG_NO_REBOOT;
> -    if (strstr(help, "-name"))
> +    if (strstr(help, "-name")) {
>          flags |= QEMUD_CMD_FLAG_NAME;
> +        if (strstr(help, ",process="))
> +            flags |= QEMUD_CMD_FLAG_NAME_PROCESS;
> +    }
>      if (strstr(help, "-uuid"))
>          flags |= QEMUD_CMD_FLAG_UUID;
>      if (strstr(help, "-xen-domid"))
> @@ -4025,7 +4028,15 @@ int qemudBuildCommandLine(virConnectPtr conn,
>  
>      if (qemuCmdFlags & QEMUD_CMD_FLAG_NAME) {
>          ADD_ARG_LIT("-name");
> -        ADD_ARG_LIT(def->name);
> +        if (qemuCmdFlags & QEMUD_CMD_FLAG_NAME_PROCESS) {
> +            char *name;
> +            if (virAsprintf(&name, "%s,process=\"qemu:%s\"",
> +                            def->name, def->name) < 0)
> +                goto no_memory;
> +            ADD_ARG_LIT(name);
> +        } else {
> +            ADD_ARG_LIT(def->name);
> +        }
>      }
>      if (qemuCmdFlags & QEMUD_CMD_FLAG_UUID) {
>          ADD_ARG_LIT("-uuid");
> @@ -6462,9 +6473,16 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
>              if (strstr(val, "menu=on"))
>                  def->os.bootmenu = 1;
>          } else if (STREQ(arg, "-name")) {
> +            char *process;
>              WANT_VALUE();
> -            if (!(def->name = strdup(val)))
> -                goto no_memory;
> +            process = strstr(val, ",process=");
> +            if (process == NULL) {
> +                if (!(def->name = strdup(val)))
> +                    goto no_memory;
> +            } else {
> +                if (!(def->name = strndup(val, process - val)))
> +                    goto no_memory;
> +            }
>          } else if (STREQ(arg, "-M")) {
>              WANT_VALUE();
>              if (!(def->os.machine = strdup(val)))
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index d2e6857..1e4483a 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -95,6 +95,7 @@ enum qemud_cmd_flags {
>      QEMUD_CMD_FLAG_ENABLE_KQEMU  = (1LL << 39), /* -enable-kqemu flag */
>      QEMUD_CMD_FLAG_FSDEV         = (1LL << 40), /* -fstype filesystem passthrough */
>      QEMUD_CMD_FLAG_NESTING       = (1LL << 41), /* -enable-nesting (SVM/VMX) */
> +    QEMUD_CMD_FLAG_NAME_PROCESS  = (1LL << 42), /* Is -name process= available */
>  };
>  
>  /* Main driver state */

The patch looks fine, as long as you can add the qemu.conf config
param to turn it on/off

Regards,
Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|