From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52610) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X4tfV-0003M4-CT for qemu-devel@nongnu.org; Wed, 09 Jul 2014 11:20:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X4tfP-00045d-0a for qemu-devel@nongnu.org; Wed, 09 Jul 2014 11:20:25 -0400 Received: from mx1.redhat.com ([209.132.183.28]:65519) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X4tfO-00045W-Ow for qemu-devel@nongnu.org; Wed, 09 Jul 2014 11:20:18 -0400 Message-ID: <53BD5DAB.7070701@redhat.com> Date: Wed, 09 Jul 2014 11:20:11 -0400 From: Cole Robinson MIME-Version: 1.0 References: <1404907292-20442-1-git-send-email-stefanha@redhat.com> <1404907292-20442-3-git-send-email-stefanha@redhat.com> In-Reply-To: <1404907292-20442-3-git-send-email-stefanha@redhat.com> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH for-2.1 2/2] qdev-monitor: include QOM properties in -device FOO, help output List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi , qemu-devel@nongnu.org Cc: Andreas Faerber , Markus Armbruster On 07/09/2014 08:01 AM, Stefan Hajnoczi wrote: > Update -device FOO,help to include QOM properties in addition to qdev > properties. Devices are gradually adding more QOM properties that are > not reflected as qdev properties. > > It is important to report all device properties since management tools > like libvirt use this information (and device-list-properties QMP) to > detect the presence of QEMU features. > > This patch reuses the device-list-properties QMP machinery to avoid code > duplication. > > Reported-by: Cole Robinson > Signed-off-by: Stefan Hajnoczi > --- > qdev-monitor.c | 40 +++++++++++++++++----------------------- > 1 file changed, 17 insertions(+), 23 deletions(-) > > diff --git a/qdev-monitor.c b/qdev-monitor.c > index f87f3d8..5fe5e75 100644 > --- a/qdev-monitor.c > +++ b/qdev-monitor.c > @@ -182,9 +182,10 @@ static const char *find_typename_by_alias(const char *alias) > > int qdev_device_help(QemuOpts *opts) > { > + Error *local_err = NULL; > const char *driver; > - Property *prop; > - ObjectClass *klass; > + DevicePropertyInfoList *prop_list; > + DevicePropertyInfoList *prop; > > driver = qemu_opt_get(opts, "driver"); > if (driver && is_help_option(driver)) { > @@ -196,35 +197,28 @@ int qdev_device_help(QemuOpts *opts) > return 0; > } > > - klass = object_class_by_name(driver); > - if (!klass) { > + if (!object_class_by_name(driver)) { > const char *typename = find_typename_by_alias(driver); > > if (typename) { > driver = typename; > - klass = object_class_by_name(driver); > } > } > > - if (!object_class_dynamic_cast(klass, TYPE_DEVICE)) { > - return 0; > + prop_list = qmp_device_list_properties(driver, &local_err); > + if (!prop_list) { > + error_printf("%s\n", error_get_pretty(local_err)); > + error_free(local_err); > + return 1; > } > - do { > - for (prop = DEVICE_CLASS(klass)->props; prop && prop->name; prop++) { > - /* > - * TODO Properties without a parser are just for dirty hacks. > - * qdev_prop_ptr is the only such PropertyInfo. It's marked > - * for removal. This conditional should be removed along with > - * it. > - */ > - if (!prop->info->set) { > - continue; /* no way to set it, don't show */ > - } > - error_printf("%s.%s=%s\n", driver, prop->name, > - prop->info->legacy_name ?: prop->info->name); > - } > - klass = object_class_get_parent(klass); > - } while (klass != object_class_by_name(TYPE_DEVICE)); > + > + for (prop = prop_list; prop; prop = prop->next) { > + error_printf("%s.%s=%s\n", driver, > + prop->value->name, > + prop->value->type); > + } > + > + qapi_free_DevicePropertyInfoList(prop_list); > return 1; > } > > Ah, I was a bit confused here. The actual issue I was hitting is fixed by the commit that Paolo pointed to: commit f4eb32b590bf58c1c67570775eb78beb09964fad Author: Stefan Hajnoczi Date: Tue May 20 14:29:01 2014 +0200 qmp: show QOM properties in device-list-properties All libvirt versions since 1.0.0 use device-list-properties over the command line introspection. I tried testing with libvirt 0.10.2.8 but there are other issues that trip it up, like changed -help output, so not sure if we even care about that case. I did verify that -device virtio-blk,? now lists bootindex again, so: Tested-by: Cole Robinson Thanks, Cole