[PATCH 0/4] qemu-img/io/nbd: Support help options for --object

[PATCH 0/4] qemu-img/io/nbd: Support help options for --object

[Kevin Wolf]

This series fixes that the tools don't consider help options for
--object and just treat them as regular object properties. For example:

    $ ./qemu-io --object secret,help
    qemu-io: Parameter 'id' is missing

With these patches, we get the expected behaviour like in the system
emulator.

Kevin Wolf (4):
  vl: Split off user_creatable_print_help()
  qemu-io: Support help options for --object
  qemu-img: Support help options for --object
  qemu-nbd: Support help options for --object

 include/qom/object_interfaces.h | 12 +++++++
 qemu-img.c                      | 34 +++++++++++-------
 qemu-io.c                       |  9 ++++-
 qemu-nbd.c                      |  9 ++++-
 qom/object_interfaces.c         | 61 +++++++++++++++++++++++++++++++++
 vl.c                            | 52 +---------------------------
 6 files changed, 111 insertions(+), 66 deletions(-)

-- 
2.20.1

[PATCH 1/4] vl: Split off user_creatable_print_help()

[Kevin Wolf]

Printing help for --object is something that we don't only want in the
system emulator, but also in tools that support --object. Move it into a
separate function in qom/object_interfaces.c to make the code accessible
for tools.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 include/qom/object_interfaces.h | 12 +++++++
 qom/object_interfaces.c         | 61 +++++++++++++++++++++++++++++++++
 vl.c                            | 52 +---------------------------
 3 files changed, 74 insertions(+), 51 deletions(-)

diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
index 682ba1d9b0..3e4e1d928b 100644
--- a/include/qom/object_interfaces.h
+++ b/include/qom/object_interfaces.h
@@ -132,6 +132,18 @@ typedef bool (*user_creatable_add_opts_predicate)(const char *type);
 int user_creatable_add_opts_foreach(void *opaque,
                                     QemuOpts *opts, Error **errp);
 
+/**
+ * user_creatable_print_help:
+ * @type: the QOM type to be added
+ * @opts: options to create
+ *
+ * Prints help if requested in @opts.
+ *
+ * Returns: true if @opts contained a help option and help was printed, false
+ * if no help option was found.
+ */
+bool user_creatable_print_help(const char *type, QemuOpts *opts);
+
 /**
  * user_creatable_del:
  * @id: the unique ID for the object
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index cb5809934a..46cd6eab5c 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -1,8 +1,11 @@
 #include "qemu/osdep.h"
+
+#include "qemu/cutils.h"
 #include "qapi/error.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qerror.h"
 #include "qom/object_interfaces.h"
+#include "qemu/help_option.h"
 #include "qemu/module.h"
 #include "qemu/option.h"
 #include "qapi/opts-visitor.h"
@@ -155,6 +158,64 @@ int user_creatable_add_opts_foreach(void *opaque, QemuOpts *opts, Error **errp)
     return 0;
 }
 
+bool user_creatable_print_help(const char *type, QemuOpts *opts)
+{
+    ObjectClass *klass;
+
+    if (is_help_option(type)) {
+        GSList *l, *list;
+
+        printf("List of user creatable objects:\n");
+        list = object_class_get_list_sorted(TYPE_USER_CREATABLE, false);
+        for (l = list; l != NULL; l = l->next) {
+            ObjectClass *oc = OBJECT_CLASS(l->data);
+            printf("  %s\n", object_class_get_name(oc));
+        }
+        g_slist_free(list);
+        return true;
+    }
+
+    klass = object_class_by_name(type);
+    if (klass && qemu_opt_has_help_opt(opts)) {
+        ObjectPropertyIterator iter;
+        ObjectProperty *prop;
+        GPtrArray *array = g_ptr_array_new();
+        int i;
+
+        object_class_property_iter_init(&iter, klass);
+        while ((prop = object_property_iter_next(&iter))) {
+            GString *str;
+
+            if (!prop->set) {
+                continue;
+            }
+
+            str = g_string_new(NULL);
+            g_string_append_printf(str, "  %s=<%s>", prop->name, prop->type);
+            if (prop->description) {
+                if (str->len < 24) {
+                    g_string_append_printf(str, "%*s", 24 - (int)str->len, "");
+                }
+                g_string_append_printf(str, " - %s", prop->description);
+            }
+            g_ptr_array_add(array, g_string_free(str, false));
+        }
+        g_ptr_array_sort(array, (GCompareFunc)qemu_pstrcmp0);
+        if (array->len > 0) {
+            printf("%s options:\n", type);
+        } else {
+            printf("There are no options for %s.\n", type);
+        }
+        for (i = 0; i < array->len; i++) {
+            printf("%s\n", (char *)array->pdata[i]);
+        }
+        g_ptr_array_set_free_func(array, g_free);
+        g_ptr_array_free(array, true);
+        return true;
+    }
+
+    return false;
+}
 
 void user_creatable_del(const char *id, Error **errp)
 {
diff --git a/vl.c b/vl.c
index 002bf4919e..b86d4e502d 100644
--- a/vl.c
+++ b/vl.c
@@ -2649,57 +2649,7 @@ static int machine_set_property(void *opaque,
  */
 static bool object_create_initial(const char *type, QemuOpts *opts)
 {
-    ObjectClass *klass;
-
-    if (is_help_option(type)) {
-        GSList *l, *list;
-
-        printf("List of user creatable objects:\n");
-        list = object_class_get_list_sorted(TYPE_USER_CREATABLE, false);
-        for (l = list; l != NULL; l = l->next) {
-            ObjectClass *oc = OBJECT_CLASS(l->data);
-            printf("  %s\n", object_class_get_name(oc));
-        }
-        g_slist_free(list);
-        exit(0);
-    }
-
-    klass = object_class_by_name(type);
-    if (klass && qemu_opt_has_help_opt(opts)) {
-        ObjectPropertyIterator iter;
-        ObjectProperty *prop;
-        GPtrArray *array = g_ptr_array_new();
-        int i;
-
-        object_class_property_iter_init(&iter, klass);
-        while ((prop = object_property_iter_next(&iter))) {
-            GString *str;
-
-            if (!prop->set) {
-                continue;
-            }
-
-            str = g_string_new(NULL);
-            g_string_append_printf(str, "  %s=<%s>", prop->name, prop->type);
-            if (prop->description) {
-                if (str->len < 24) {
-                    g_string_append_printf(str, "%*s", 24 - (int)str->len, "");
-                }
-                g_string_append_printf(str, " - %s", prop->description);
-            }
-            g_ptr_array_add(array, g_string_free(str, false));
-        }
-        g_ptr_array_sort(array, (GCompareFunc)qemu_pstrcmp0);
-        if (array->len > 0) {
-            printf("%s options:\n", type);
-        } else {
-            printf("There are no options for %s.\n", type);
-        }
-        for (i = 0; i < array->len; i++) {
-            printf("%s\n", (char *)array->pdata[i]);
-        }
-        g_ptr_array_set_free_func(array, g_free);
-        g_ptr_array_free(array, true);
+    if (user_creatable_print_help(type, opts)) {
         exit(0);
     }
 
-- 
2.20.1

Re: [PATCH 1/4] vl: Split off user_creatable_print_help()

[Eric Blake]

On 10/11/19 3:55 PM, Kevin Wolf wrote:
> Printing help for --object is something that we don't only want in the

s/don't/not/

> system emulator, but also in tools that support --object. Move it into a
> separate function in qom/object_interfaces.c to make the code accessible
> for tools.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   include/qom/object_interfaces.h | 12 +++++++
>   qom/object_interfaces.c         | 61 +++++++++++++++++++++++++++++++++
>   vl.c                            | 52 +---------------------------
>   3 files changed, 74 insertions(+), 51 deletions(-)
> 
> diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
> index 682ba1d9b0..3e4e1d928b 100644
> --- a/include/qom/object_interfaces.h

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 1/4] vl: Split off user_creatable_print_help()

[Kevin Wolf]

Am 11.10.2019 um 23:35 hat Eric Blake geschrieben:
> On 10/11/19 3:55 PM, Kevin Wolf wrote:
> > Printing help for --object is something that we don't only want in the
> 
> s/don't/not/

Can someone send a fix for the English grammar? It's obviously broken
and doesn't know what it wants. Actually, maybe do-support was a bad
idea and we should just revert it and restore consistent use of proper
verb-second word order?

(Hm, actually, since this seems to negate "only" rather than the verb,
does "...that we want not only in..." work without patching the
grammar?)

(Thanks for the correction.)

Kevin

> > system emulator, but also in tools that support --object. Move it into a
> > separate function in qom/object_interfaces.c to make the code accessible
> > for tools.
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Re: [PATCH 1/4] vl: Split off user_creatable_print_help()

[Eric Blake]

On 10/14/19 4:55 AM, Kevin Wolf wrote:
> Am 11.10.2019 um 23:35 hat Eric Blake geschrieben:
>> On 10/11/19 3:55 PM, Kevin Wolf wrote:
>>> Printing help for --object is something that we don't only want in the
>>
>> s/don't/not/
> 
> Can someone send a fix for the English grammar? It's obviously broken
> and doesn't know what it wants. Actually, maybe do-support was a bad
> idea and we should just revert it and restore consistent use of proper
> verb-second word order?

Lol

> 
> (Hm, actually, since this seems to negate "only" rather than the verb,
> does "...that we want not only in..." work without patching the
> grammar?)

Yes, that formulation also works.

> 
> (Thanks for the correction.)
> 
> Kevin
> 
>>> system emulator, but also in tools that support --object. Move it into a
>>> separate function in qom/object_interfaces.c to make the code accessible
>>> for tools.
>>>
>>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

[PATCH 2/4] qemu-io: Support help options for --object

[Kevin Wolf]

Instead of parsing help options as normal object properties and
returning an error, provide the same help functionality as the system
emulator in qemu-io, too.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-io.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/qemu-io.c b/qemu-io.c
index f64eca6940..91e3276592 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -475,6 +475,13 @@ static QemuOptsList qemu_object_opts = {
     },
 };
 
+static bool qemu_io_object_print_help(const char *type, QemuOpts *opts)
+{
+    if (user_creatable_print_help(type, opts)) {
+        exit(0);
+    }
+    return true;
+}
 
 static QemuOptsList file_opts = {
     .name = "file",
@@ -622,7 +629,7 @@ int main(int argc, char **argv)
 
     qemu_opts_foreach(&qemu_object_opts,
                       user_creatable_add_opts_foreach,
-                      NULL, &error_fatal);
+                      qemu_io_object_print_help, &error_fatal);
 
     if (!trace_init_backends()) {
         exit(1);
-- 
2.20.1

Re: [PATCH 2/4] qemu-io: Support help options for --object

[Eric Blake]

On 10/11/19 3:55 PM, Kevin Wolf wrote:
> Instead of parsing help options as normal object properties and
> returning an error, provide the same help functionality as the system
> emulator in qemu-io, too.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   qemu-io.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

[PATCH 3/4] qemu-img: Support help options for --object

[Kevin Wolf]

Instead of parsing help options as normal object properties and
returning an error, provide the same help functionality as the system
emulator in qemu-img, too.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-img.c | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 384c6f38bc..8b03ef8171 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -214,6 +214,14 @@ static QemuOptsList qemu_object_opts = {
     },
 };
 
+static bool qemu_img_object_print_help(const char *type, QemuOpts *opts)
+{
+    if (user_creatable_print_help(type, opts)) {
+        exit(0);
+    }
+    return true;
+}
+
 static QemuOptsList qemu_source_opts = {
     .name = "source",
     .implied_opt_name = "file",
@@ -516,7 +524,7 @@ static int img_create(int argc, char **argv)
 
     if (qemu_opts_foreach(&qemu_object_opts,
                           user_creatable_add_opts_foreach,
-                          NULL, &error_fatal)) {
+                          qemu_img_object_print_help, &error_fatal)) {
         goto fail;
     }
 
@@ -766,7 +774,7 @@ static int img_check(int argc, char **argv)
 
     if (qemu_opts_foreach(&qemu_object_opts,
                           user_creatable_add_opts_foreach,
-                          NULL, &error_fatal)) {
+                          qemu_img_object_print_help, &error_fatal)) {
         return 1;
     }
 
@@ -979,7 +987,7 @@ static int img_commit(int argc, char **argv)
 
     if (qemu_opts_foreach(&qemu_object_opts,
                           user_creatable_add_opts_foreach,
-                          NULL, &error_fatal)) {
+                          qemu_img_object_print_help, &error_fatal)) {
         return 1;
     }
 
@@ -1362,7 +1370,7 @@ static int img_compare(int argc, char **argv)
 
     if (qemu_opts_foreach(&qemu_object_opts,
                           user_creatable_add_opts_foreach,
-                          NULL, &error_fatal)) {
+                          qemu_img_object_print_help, &error_fatal)) {
         ret = 2;
         goto out4;
     }
@@ -2210,7 +2218,7 @@ static int img_convert(int argc, char **argv)
 
     if (qemu_opts_foreach(&qemu_object_opts,
                           user_creatable_add_opts_foreach,
-                          NULL, &error_fatal)) {
+                          qemu_img_object_print_help, &error_fatal)) {
         goto fail_getopt;
     }
 
@@ -2776,7 +2784,7 @@ static int img_info(int argc, char **argv)
 
     if (qemu_opts_foreach(&qemu_object_opts,
                           user_creatable_add_opts_foreach,
-                          NULL, &error_fatal)) {
+                          qemu_img_object_print_help, &error_fatal)) {
         return 1;
     }
 
@@ -3002,7 +3010,7 @@ static int img_map(int argc, char **argv)
 
     if (qemu_opts_foreach(&qemu_object_opts,
                           user_creatable_add_opts_foreach,
-                          NULL, &error_fatal)) {
+                          qemu_img_object_print_help, &error_fatal)) {
         return 1;
     }
 
@@ -3154,7 +3162,7 @@ static int img_snapshot(int argc, char **argv)
 
     if (qemu_opts_foreach(&qemu_object_opts,
                           user_creatable_add_opts_foreach,
-                          NULL, &error_fatal)) {
+                          qemu_img_object_print_help, &error_fatal)) {
         return 1;
     }
 
@@ -3321,7 +3329,7 @@ static int img_rebase(int argc, char **argv)
 
     if (qemu_opts_foreach(&qemu_object_opts,
                           user_creatable_add_opts_foreach,
-                          NULL, &error_fatal)) {
+                          qemu_img_object_print_help, &error_fatal)) {
         return 1;
     }
 
@@ -3742,7 +3750,7 @@ static int img_resize(int argc, char **argv)
 
     if (qemu_opts_foreach(&qemu_object_opts,
                           user_creatable_add_opts_foreach,
-                          NULL, &error_fatal)) {
+                          qemu_img_object_print_help, &error_fatal)) {
         return 1;
     }
 
@@ -3986,7 +3994,7 @@ static int img_amend(int argc, char **argv)
 
     if (qemu_opts_foreach(&qemu_object_opts,
                           user_creatable_add_opts_foreach,
-                          NULL, &error_fatal)) {
+                          qemu_img_object_print_help, &error_fatal)) {
         ret = -1;
         goto out_no_progress;
     }
@@ -4630,7 +4638,7 @@ static int img_dd(int argc, char **argv)
 
     if (qemu_opts_foreach(&qemu_object_opts,
                           user_creatable_add_opts_foreach,
-                          NULL, &error_fatal)) {
+                          qemu_img_object_print_help, &error_fatal)) {
         ret = -1;
         goto out;
     }
@@ -4907,7 +4915,7 @@ static int img_measure(int argc, char **argv)
 
     if (qemu_opts_foreach(&qemu_object_opts,
                           user_creatable_add_opts_foreach,
-                          NULL, &error_fatal)) {
+                          qemu_img_object_print_help, &error_fatal)) {
         goto out;
     }
 
-- 
2.20.1

Re: [PATCH 3/4] qemu-img: Support help options for --object

[Eric Blake]

On 10/11/19 3:55 PM, Kevin Wolf wrote:
> Instead of parsing help options as normal object properties and
> returning an error, provide the same help functionality as the system
> emulator in qemu-img, too.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   qemu-img.c | 34 +++++++++++++++++++++-------------
>   1 file changed, 21 insertions(+), 13 deletions(-)
> 
Missing man page documentation of this new feature.  Perhaps iotest 
coverage is also warranted?  (qemu-io is only for testing purposes, so I 
don't care about that in patch 2 as much as I do here)

But the patch itself is a strict improvement, so

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

[PATCH 4/4] qemu-nbd: Support help options for --object

[Kevin Wolf]

Instead of parsing help options as normal object properties and
returning an error, provide the same help functionality as the system
emulator in qemu-nbd, too.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-nbd.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index 9032b6de2a..caacf0ed73 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -507,6 +507,13 @@ static QemuOptsList qemu_object_opts = {
     },
 };
 
+static bool qemu_nbd_object_print_help(const char *type, QemuOpts *opts)
+{
+    if (user_creatable_print_help(type, opts)) {
+        exit(0);
+    }
+    return true;
+}
 
 
 static QCryptoTLSCreds *nbd_get_tls_creds(const char *id, bool list,
@@ -902,7 +909,7 @@ int main(int argc, char **argv)
 
     qemu_opts_foreach(&qemu_object_opts,
                       user_creatable_add_opts_foreach,
-                      NULL, &error_fatal);
+                      qemu_nbd_object_print_help, &error_fatal);
 
     if (!trace_init_backends()) {
         exit(1);
-- 
2.20.1

Re: [PATCH 4/4] qemu-nbd: Support help options for --object

[Eric Blake]

On 10/11/19 3:55 PM, Kevin Wolf wrote:
> Instead of parsing help options as normal object properties and
> returning an error, provide the same help functionality as the system
> emulator in qemu-nbd, too.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   qemu-nbd.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)

Missing a change in qemu-nbd.texi for man page coverage.  But the patch 
is a strict improvement, so even if we have to add a followup patch for 
documentation, I'm okay with:

Reviewed-by: Eric Blake <eblake@redhat.com>

This patch touches NBD, but I'm assuming it's easier to take the series 
through your tree.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 4/4] qemu-nbd: Support help options for --object

[Kevin Wolf]

Am 11.10.2019 um 23:39 hat Eric Blake geschrieben:
> On 10/11/19 3:55 PM, Kevin Wolf wrote:
> > Instead of parsing help options as normal object properties and
> > returning an error, provide the same help functionality as the system
> > emulator in qemu-nbd, too.
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> 
> Missing a change in qemu-nbd.texi for man page coverage.

Hm... Both qemu-img and qemu-nbd manpages refer to qemu(1) for the
details. I wouldn't mind copying the text for '-object help' from there
anyway, but unfortunately it doesn't even exist. :-)

So this looks like a separate patch that fixes it for qemu, too.

> But the patch is a strict improvement, so even if we have to add a
> followup patch for documentation, I'm okay with:
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> This patch touches NBD, but I'm assuming it's easier to take the series
> through your tree.

Yes, thanks.

Kevin