Patchwork [v2] Man page: Add -global description

login
register
mail settings
Submitter Miroslav Rezanina
Date March 14, 2012, 8:53 a.m.
Message ID <20120314085310.GA4042@lws.brq.redhat.com>
Download mbox | patch
Permalink /patch/146582/
State New
Headers show

Comments

Miroslav Rezanina - March 14, 2012, 8:53 a.m.
There's only TODO information in qemu man page for -global option. This is a basic description of this option with simple example.

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>

Patch:
--
Markus Armbruster - March 14, 2012, 6:09 p.m.
Miroslav Rezanina <mrezanin@redhat.com> writes:

> There's only TODO information in qemu man page for -global option. This is a basic description of this option with simple example.
>
> Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
>
> Patch:
> --
> diff --git a/qemu-options.hx b/qemu-options.hx
> index daefce3..876f929 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -292,9 +292,13 @@ DEF("global", HAS_ARG, QEMU_OPTION_global,
>      "                set a global default for a driver property\n",
>      QEMU_ARCH_ALL)
>  STEXI
> -@item -global
> +@item -global @var{driver}.@var{property}=@var{value}
>  @findex -global
> -TODO
> +Set default value of @var{driver}'s @var{property} to @var{value}, e.g.:
> +
> +@example
> +qemu -global ide-drive.physical_block_size=4096 -drive file=file,if=ide,index=0
> +@end example
>  ETEXI
>  
>  DEF("mtdblock", HAS_ARG, QEMU_OPTION_mtdblock,

Much better than nothing.  Stefan, would you like to take this through
your trivial queue?
Peter Maydell - March 14, 2012, 6:21 p.m.
On 14 March 2012 08:53, Miroslav Rezanina <mrezanin@redhat.com> wrote:
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -292,9 +292,13 @@ DEF("global", HAS_ARG, QEMU_OPTION_global,
>     "                set a global default for a driver property\n",
>     QEMU_ARCH_ALL)
>  STEXI
> -@item -global
> +@item -global @var{driver}.@var{property}=@var{value}

We seem to use @var{prop}, not @var{property}, elsewhere in the docs.

>  @findex -global
> -TODO
> +Set default value of @var{driver}'s @var{property} to @var{value}, e.g.:
> +
> +@example
> +qemu -global ide-drive.physical_block_size=4096 -drive file=file,if=ide,index=0
> +@end example
>  ETEXI

This is missing any motivation for why you would want to actually
use this option. How about:

"In particular, you can use this to set driver properties for devices which
are created automatically by the machine model. (To create a device which is
not created automatically and set properties on it, use -device.)"

?

That's still not great, but I think it helps a little.

(ideally if -device/-global are the new standard interface we should have a
section explaining the general concepts and syntax and then documentation of
how to do specific things like networking via -device, and relegate all the
'legacy' options to a section clearly marked as 'legacy' with pointers back
to the new ways of doing the same thing. That would be a much bigger job,
though.)

-- PMM
Anthony Liguori - March 14, 2012, 7:23 p.m.
On 03/14/2012 01:21 PM, Peter Maydell wrote:
> On 14 March 2012 08:53, Miroslav Rezanina<mrezanin@redhat.com>  wrote:
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -292,9 +292,13 @@ DEF("global", HAS_ARG, QEMU_OPTION_global,
>>      "                set a global default for a driver property\n",
>>      QEMU_ARCH_ALL)
>>   STEXI
>> -@item -global
>> +@item -global @var{driver}.@var{property}=@var{value}
>
> We seem to use @var{prop}, not @var{property}, elsewhere in the docs.
>
>>   @findex -global
>> -TODO
>> +Set default value of @var{driver}'s @var{property} to @var{value}, e.g.:
>> +
>> +@example
>> +qemu -global ide-drive.physical_block_size=4096 -drive file=file,if=ide,index=0
>> +@end example
>>   ETEXI
>
> This is missing any motivation for why you would want to actually
> use this option. How about:
>
> "In particular, you can use this to set driver properties for devices which
> are created automatically by the machine model. (To create a device which is
> not created automatically and set properties on it, use -device.)"
>
> ?
>
> That's still not great, but I think it helps a little.
>
> (ideally if -device/-global are the new standard interface we should have a
> section explaining the general concepts and syntax and then documentation of
> how to do specific things like networking via -device, and relegate all the
> 'legacy' options to a section clearly marked as 'legacy' with pointers back
> to the new ways of doing the same thing. That would be a much bigger job,
> though.)

Just while we're here, I'll be posting the following shortly:

commit 82aff428155d469ab705294486cc26cb34947999
Author: Anthony Liguori <aliguori@us.ibm.com>
Date:   Fri Dec 23 11:30:45 2011 -0600

qdev: don't allow globals to be set by bus name

This is technically a compatibility breaker.  However:

1) libvirt does not rely on this (it always uses the driver name)

2) This behavior isn't actually documented anywhere (the docs just say driver).

3) I suspect there are less than three people on earth that even know this is
    possible (minus the people reading this message).

So I think we can safely break it :-)

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

>
> -- PMM
>
Markus Armbruster - March 15, 2012, 7:50 a.m.
Anthony Liguori <anthony@codemonkey.ws> writes:

> Just while we're here, I'll be posting the following shortly:
>
> commit 82aff428155d469ab705294486cc26cb34947999
> Author: Anthony Liguori <aliguori@us.ibm.com>
> Date:   Fri Dec 23 11:30:45 2011 -0600
>
> qdev: don't allow globals to be set by bus name
>
> This is technically a compatibility breaker.  However:
>
> 1) libvirt does not rely on this (it always uses the driver name)
>
> 2) This behavior isn't actually documented anywhere (the docs just say driver).
>
> 3) I suspect there are less than three people on earth that even know this is
>    possible (minus the people reading this message).
>
> So I think we can safely break it :-)

Go right ahead.  We shouldn't tie ourselves up in knots over
undocumented behavior, unless there's clear evidence of use.
Miroslav Rezanina - March 15, 2012, 8:35 a.m.
----- Original Message -----
> From: "Peter Maydell" <peter.maydell@linaro.org>
> To: "Miroslav Rezanina" <mrezanin@redhat.com>
> Cc: qemu-devel@nongnu.org
> Sent: Wednesday, March 14, 2012 7:21:30 PM
> Subject: Re: [Qemu-devel] [PATCH v2] Man page: Add -global description
> 
> On 14 March 2012 08:53, Miroslav Rezanina <mrezanin@redhat.com>
> wrote:
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -292,9 +292,13 @@ DEF("global", HAS_ARG, QEMU_OPTION_global,
> >     "                set a global default for a driver property\n",
> >     QEMU_ARCH_ALL)
> >  STEXI
> > -@item -global
> > +@item -global @var{driver}.@var{property}=@var{value}
> 
> We seem to use @var{prop}, not @var{property}, elsewhere in the docs.
>
You're right, I reused naming from DEF part...both has to be updated.

> >  @findex -global
> > -TODO
> > +Set default value of @var{driver}'s @var{property} to @var{value},
> > e.g.:
> > +
> > +@example
> > +qemu -global ide-drive.physical_block_size=4096 -drive
> > file=file,if=ide,index=0
> > +@end example
> >  ETEXI
> 
> This is missing any motivation for why you would want to actually
> use this option. How about:
> 
> "In particular, you can use this to set driver properties for devices
> which
> are created automatically by the machine model. (To create a device
> which is
> not created automatically and set properties on it, use -device.)"
> 
> ?
> 
> That's still not great, but I think it helps a little.
> 

Yeah, more use case should be stated but I do not think man page is good
place for extensive user motivation. 

> (ideally if -device/-global are the new standard interface we should
> have a
> section explaining the general concepts and syntax and then
> documentation of
> how to do specific things like networking via -device, and relegate
> all the
> 'legacy' options to a section clearly marked as 'legacy' with
> pointers back
> to the new ways of doing the same thing. That would be a much bigger
> job,
> though.)
> 

Agree, man page is really behind the code. However, I think documenting new
interfaces should be done first to agree on them and their usage. After new
way is known extracting old interface as legacy stuff would be easier.

> -- PMM
> 


Anyway, I will resend this patch with updates you recommended.

Mirek
Gerd Hoffmann - March 16, 2012, 9:24 a.m.
Hi,

> commit 82aff428155d469ab705294486cc26cb34947999
> Author: Anthony Liguori <aliguori@us.ibm.com>
> Date:   Fri Dec 23 11:30:45 2011 -0600
> 
> qdev: don't allow globals to be set by bus name

> So I think we can safely break it :-)

There are compat properties using that (turn off new pci features on old
releases for all pci devices).

cheers,
  Gerd
Paolo Bonzini - March 19, 2012, 5:13 p.m.
Il 16/03/2012 10:24, Gerd Hoffmann ha scritto:
>> > commit 82aff428155d469ab705294486cc26cb34947999
>> > Author: Anthony Liguori <aliguori@us.ibm.com>
>> > Date:   Fri Dec 23 11:30:45 2011 -0600
>> > 
>> > qdev: don't allow globals to be set by bus name
>> > So I think we can safely break it :-)
> There are compat properties using that (turn off new pci features on old
> releases for all pci devices).

It could be changed to a compat property on the abstract PCI device
class (with changes to look up global properties on the whole hierarchy
of course).

Paolo

Patch

diff --git a/qemu-options.hx b/qemu-options.hx
index daefce3..876f929 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -292,9 +292,13 @@  DEF("global", HAS_ARG, QEMU_OPTION_global,
     "                set a global default for a driver property\n",
     QEMU_ARCH_ALL)
 STEXI
-@item -global
+@item -global @var{driver}.@var{property}=@var{value}
 @findex -global
-TODO
+Set default value of @var{driver}'s @var{property} to @var{value}, e.g.:
+
+@example
+qemu -global ide-drive.physical_block_size=4096 -drive file=file,if=ide,index=0
+@end example
 ETEXI
 
 DEF("mtdblock", HAS_ARG, QEMU_OPTION_mtdblock,