Patchwork [5/5] qdev: Add new '-device help' option, shows all devices and properties

login
register
mail settings
Submitter Amit Shah
Date May 31, 2010, 12:41 p.m.
Message ID <383cd43ef87e1a699648301657902d3fe20f2ec5.1275309375.git.amit.shah@redhat.com>
Download mbox | patch
Permalink /patch/54089/
State New
Headers show

Comments

Amit Shah - May 31, 2010, 12:41 p.m.
The new '-device help' option shows all the devices that are registered
with qdev and prints out all the properties each device has, along with
the description for each property.

This is useful in creating automatic documentation for all the options
that we have and support.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/qdev.c |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)
Markus Armbruster - June 7, 2010, 2:43 p.m.
I like PATCH 1-3/5, but this one needs discussion.

Amit Shah <amit.shah@redhat.com> writes:

> The new '-device help' option shows all the devices that are registered
> with qdev and prints out all the properties each device has, along with
> the description for each property.
>
> This is useful in creating automatic documentation for all the options
> that we have and support.
>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
>  hw/qdev.c |   15 +++++++++++----
>  1 files changed, 11 insertions(+), 4 deletions(-)

Documentation update missing: qemu-options.hx

> diff --git a/hw/qdev.c b/hw/qdev.c
> index 89ba986..4be2f66 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -151,7 +151,7 @@ static int set_property(const char *name, const char *value, void *opaque)
>      return 0;
>  }
>  
> -static int show_device_props(const char *driver)
> +static int show_device_props(const char *driver, const char *prefix)
>  {
>      DeviceInfo *info;
>      Property *prop;
> @@ -161,6 +161,10 @@ static int show_device_props(const char *driver)
>          return 0;
>      }
>  
> +    if (!prefix) {
> +        prefix = "";
> +    }
> +

Just make the caller pass "" instead of NULL.

>      for (prop = info->props; prop && prop->name; prop++) {
>          /*
>           * TODO Properties without a parser are just for dirty hacks.
> @@ -171,7 +175,7 @@ static int show_device_props(const char *driver)
>          if (!prop->info->parse) {
>              continue;           /* no way to set it, don't show */
>          }
> -        error_printf("%s.%s=%s, %s\n", info->name, prop->name,
> +        error_printf("%s%s.%s=%s, %s\n", prefix, info->name, prop->name,
>                       prop->info->name, prop->desc ?: "");
>      }
>      return 1;
> @@ -183,12 +187,15 @@ int qdev_device_help(QemuOpts *opts)
>      DeviceInfo *info;
>  
>      driver = qemu_opt_get(opts, "driver");
> -    if (driver && !strcmp(driver, "?")) {
> +    if (driver && (!strcmp(driver, "?") || !strcmp(driver, "help"))) {
>          for (info = device_info_list; info != NULL; info = info->next) {
>              if (info->no_user) {
>                  continue;       /* not available, don't show */
>              }
>              qdev_print_devinfo(info);
> +            if (!strcmp(driver, "help")) {
> +                show_device_props(info->name, "\t");
> +            }
>          }
>          return 1;
>      }

There is "-device \?" and "-device help", but the user interface
provides no clue about the difference between them.

"-device help" loses when we ever pick up a device model with name
"help".  Not that "?" was a particularly smart choice...

Do we really need two kinds of help output?  Where the second is
basically the same as "-device FOO,\?" for all FOO?  Is that convenience
worth the extra UI complexity?

Doing it in the shell isn't exactly hard:

$ for i in `qemu -device \? 2>&1 | awk -F \" '{ print $2 }'`; do qemu -device $i,\?; done

> @@ -197,7 +204,7 @@ int qdev_device_help(QemuOpts *opts)
>          return 0;
>      }
>  
> -    return show_device_props(driver);
> +    return show_device_props(driver, NULL);
>  }
>  
>  DeviceState *qdev_device_add(QemuOpts *opts)
Amit Shah - June 8, 2010, 5:13 a.m.
On (Mon) Jun 07 2010 [16:43:05], Markus Armbruster wrote:
> 
> There is "-device \?" and "-device help", but the user interface
> provides no clue about the difference between them.
> 
> "-device help" loses when we ever pick up a device model with name
> "help".  Not that "?" was a particularly smart choice...
> 
> Do we really need two kinds of help output?  Where the second is
> basically the same as "-device FOO,\?" for all FOO?  Is that convenience
> worth the extra UI complexity?

OK, so my big plan is to have something like this:

---

./qemu -device help

...
name "virtserialport", bus virtio-serial-bus
	nr=uint32, The 'number' for the port for predictable port numbers.
	chardev=chr, The chardev to associate this port with.
	name=string, Name for the port that's exposed to the guest for port discovery.
...

---

./qemu -device virtserialport,?

virtserialport.nr=uint32, The 'number' for the port for predictable port numbers.

   Use this to spawn ports if you plan to migrate the guest and perform
   hot-plug and unplug operations.

virtserialport.chardev=chr, The chardev to associate this port with.

   External programs can communicate with the guest side of the channel
   using chardev

virtserialport.name=string, Name for the port that's exposed to the guest for port discovery.

   The name is exposed in Linux guests via sysfs and udev rules can be
   written to create symlinks to the ports. Apps in the guest can then
   easily find these ports and start communication.

---

So basically -device help gives a short, one-liner help message and the
-device ? option gives a slightly detailed message, if one is available.

How to do that: I'm thinking of two ways:

1)

    DEFINE_PROP_UINT32("nr", VirtConsole, port.id, VIRTIO_CONSOLE_BAD_ID,
                       "The 'number' for the port for predictable port numbers.",
                       "Use this to spawn ports if you plan to migrate the guest and perform \n"
                       "hot-plug and unplug operations." ),

ie, two different strings for the short and long descriptions

2)

    DEFINE_PROP_UINT32("nr", VirtConsole, port.id, VIRTIO_CONSOLE_BAD_ID,
                       "The 'number' for the port for predictable port numbers.\n"
                       "Use this to spawn ports if you plan to migrate the guest and perform \n"
                       "hot-plug and unplug operations." ),

ie, the same string, but the short and long descriptions are separated
by a '\n'.

Doing it the adds parsing to the printing routine for no extra benefit,
so I'm inclined to go with 1. Thoughts?

		Amit

Patch

diff --git a/hw/qdev.c b/hw/qdev.c
index 89ba986..4be2f66 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -151,7 +151,7 @@  static int set_property(const char *name, const char *value, void *opaque)
     return 0;
 }
 
-static int show_device_props(const char *driver)
+static int show_device_props(const char *driver, const char *prefix)
 {
     DeviceInfo *info;
     Property *prop;
@@ -161,6 +161,10 @@  static int show_device_props(const char *driver)
         return 0;
     }
 
+    if (!prefix) {
+        prefix = "";
+    }
+
     for (prop = info->props; prop && prop->name; prop++) {
         /*
          * TODO Properties without a parser are just for dirty hacks.
@@ -171,7 +175,7 @@  static int show_device_props(const char *driver)
         if (!prop->info->parse) {
             continue;           /* no way to set it, don't show */
         }
-        error_printf("%s.%s=%s, %s\n", info->name, prop->name,
+        error_printf("%s%s.%s=%s, %s\n", prefix, info->name, prop->name,
                      prop->info->name, prop->desc ?: "");
     }
     return 1;
@@ -183,12 +187,15 @@  int qdev_device_help(QemuOpts *opts)
     DeviceInfo *info;
 
     driver = qemu_opt_get(opts, "driver");
-    if (driver && !strcmp(driver, "?")) {
+    if (driver && (!strcmp(driver, "?") || !strcmp(driver, "help"))) {
         for (info = device_info_list; info != NULL; info = info->next) {
             if (info->no_user) {
                 continue;       /* not available, don't show */
             }
             qdev_print_devinfo(info);
+            if (!strcmp(driver, "help")) {
+                show_device_props(info->name, "\t");
+            }
         }
         return 1;
     }
@@ -197,7 +204,7 @@  int qdev_device_help(QemuOpts *opts)
         return 0;
     }
 
-    return show_device_props(driver);
+    return show_device_props(driver, NULL);
 }
 
 DeviceState *qdev_device_add(QemuOpts *opts)