[Qemu-devel] [PATCH v2 0/3] Fix confused output for alias properties

[Qemu-devel] [PATCH v2 0/3] Fix confused output for alias properties

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

At present, people have no way to know they should
have a specific format for alias properties.

 Example:

before output:

virtio-blk-pci.physical_block_size=uint16
virtio-blk-pci.logical_block_size=uint16
virtio-blk-pci.drive=str

after output applied this patch series:

virtio-blk-pci.physical_block_size=blocksize
virtio-blk-pci.logical_block_size=blocksize
virtio-blk-pci.drive=drive

v2 -> v1:
 - free prop when error is happened PATCH 1/3.
 - move AliasProperty strut to Object.h, object_property_add_alias()
  API allows objects to alias a property on the same object or
  another object. The source and target names can be different,
  we should address this scenario.
 - add 'Reviewed-by' (PATCH 1/3) and 'Acked-by' (PATCH 3/3) tags,
  hope Paolo and Michael don't mind about those changes. Thanks. :)

Gonglei (3):
  qom: add error handler for object alias property
  qom: add AliasProperty struct poniter for alias property in
    ObjectProperty
  qmp: print real legacy_name for alias property

 include/qom/object.h |  8 ++++++
 qmp.c                | 79 +++++++++++++++++++++++++++++++++++++---------------
 qom/object.c         | 16 +++++++----
 3 files changed, 74 insertions(+), 29 deletions(-)

-- 
1.7.12.4

[Qemu-devel] [PATCH v2 1/3] qom: add error handler for object alias property

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

object_property_add_alias() is called at some
places at present. And its parameter errp may not NULL,
such as
 object_property_add_alias(obj, "iothread", OBJECT(&dev->vdev),"iothread",
                              &error_abort);
This patch add error handler for security.

Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qom/object.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/qom/object.c b/qom/object.c
index da0919a..a8c3065 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1634,6 +1634,7 @@ void object_property_add_alias(Object *obj, const char *name,
     ObjectProperty *op;
     ObjectProperty *target_prop;
     gchar *prop_type;
+    Error *local_err = NULL;
 
     target_prop = object_property_find(target_obj, target_name, errp);
     if (!target_prop) {
@@ -1655,9 +1656,15 @@ void object_property_add_alias(Object *obj, const char *name,
                              property_get_alias,
                              property_set_alias,
                              property_release_alias,
-                             prop, errp);
+                             prop, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        g_free(prop);
+        goto out;
+    }
     op->resolve = property_resolve_alias;
 
+out:
     g_free(prop_type);
 }
 
-- 
1.7.12.4

[Qemu-devel] [PATCH v2 2/3] qom: add AliasProperty struct poniter for alias property in ObjectProperty

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

In this way, we can use target object and get its qdev
property legacy_name etc.

Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 include/qom/object.h | 8 ++++++++
 qom/object.c         | 7 ++-----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index 8a05a81..e3bb52e 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -334,6 +334,11 @@ typedef void (ObjectPropertyRelease)(Object *obj,
                                      const char *name,
                                      void *opaque);
 
+typedef struct {
+    Object *target_obj;
+    const char *target_name;
+} AliasProperty;
+
 typedef struct ObjectProperty
 {
     gchar *name;
@@ -344,6 +349,9 @@ typedef struct ObjectProperty
     ObjectPropertyRelease *release;
     void *opaque;
 
+    bool is_alias;
+    AliasProperty *alias_property;
+
     QTAILQ_ENTRY(ObjectProperty) node;
 } ObjectProperty;
 
diff --git a/qom/object.c b/qom/object.c
index a8c3065..9a0dd2e 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1590,11 +1590,6 @@ void object_property_add_uint64_ptr(Object *obj, const char *name,
                         NULL, NULL, (void *)v, errp);
 }
 
-typedef struct {
-    Object *target_obj;
-    const char *target_name;
-} AliasProperty;
-
 static void property_get_alias(Object *obj, struct Visitor *v, void *opaque,
                                const char *name, Error **errp)
 {
@@ -1663,6 +1658,8 @@ void object_property_add_alias(Object *obj, const char *name,
         goto out;
     }
     op->resolve = property_resolve_alias;
+    op->is_alias = true;
+    op->alias_property = prop;
 
 out:
     g_free(prop_type);
-- 
1.7.12.4

[Qemu-devel] [PATCH v2 3/3] qmp: print real legacy_name for alias property

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

In this way, we can get the real legacy_name for
alias properties, and avoide confusing people because of
property type. People have no way to know they should
have a specific format.

Note: we should also consider the scenario that
source and target names are different because
object_property_add_alias() API allows objects to alias a
property on the same object or another object. The source
and target names can be different.

 Example:

before output:

virtio-blk-pci.physical_block_size=uint16
virtio-blk-pci.logical_block_size=uint16
virtio-blk-pci.drive=str

after output applied this patch:

virtio-blk-pci.physical_block_size=blocksize
virtio-blk-pci.logical_block_size=blocksize
virtio-blk-pci.drive=drive

Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---
 qmp.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 56 insertions(+), 23 deletions(-)

diff --git a/qmp.c b/qmp.c
index c6767c4..a52ad0e 100644
--- a/qmp.c
+++ b/qmp.c
@@ -433,6 +433,44 @@ ObjectTypeInfoList *qmp_qom_list_types(bool has_implements,
     return ret;
 }
 
+static bool find_device_property_info(ObjectClass *klass,
+                                      const char *name,
+                                      DevicePropertyInfo **info,
+                                      bool is_alias,
+                                      AliasProperty *alias_property)
+{
+    Property *prop;
+    bool found = false;
+    const char *compared_name = is_alias ? alias_property->target_name : name;
+
+    for (prop = DEVICE_CLASS(klass)->props; prop && prop->name; prop++) {
+        if (strcmp(compared_name, prop->name) != 0) {
+            continue;
+        }
+
+        /*
+         * 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) {
+            found = true;
+            goto out;           /* no way to set it, don't show */
+        }
+
+        *info = g_malloc0(sizeof(**info));
+        (*info)->name = g_strdup(name); /* use original name  */
+        (*info)->type = g_strdup(prop->info->legacy_name ?: prop->info->name);
+
+        found = true;
+        goto out;
+    }
+
+out:
+    return found;
+}
+
 /* Return a DevicePropertyInfo for a qdev property.
  *
  * If a qdev property with the given name does not exist, use the given default
@@ -440,32 +478,26 @@ ObjectTypeInfoList *qmp_qom_list_types(bool has_implements,
  *
  * The caller must free the return value.
  */
-static DevicePropertyInfo *make_device_property_info(ObjectClass *klass,
-                                                     const char *name,
-                                                     const char *default_type)
+static
+DevicePropertyInfo *make_device_property_info(ObjectClass *klass,
+                                              const char *name,
+                                              const char *default_type,
+                                              bool is_alias,
+                                              AliasProperty *alias_property)
 {
     DevicePropertyInfo *info;
-    Property *prop;
-
-    do {
-        for (prop = DEVICE_CLASS(klass)->props; prop && prop->name; prop++) {
-            if (strcmp(name, prop->name) != 0) {
-                continue;
-            }
 
-            /*
-             * 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) {
-                return NULL;           /* no way to set it, don't show */
-            }
+    if (is_alias) {
+        ObjectClass *class = object_get_class(alias_property->target_obj);
+        if (find_device_property_info(class, name, &info,
+                                      is_alias, alias_property)) {
+            return info;
+        }
+    }
 
-            info = g_malloc0(sizeof(*info));
-            info->name = g_strdup(prop->name);
-            info->type = g_strdup(prop->info->legacy_name ?: prop->info->name);
+    do {
+        if (find_device_property_info(klass, name, &info,
+                                      is_alias, alias_property)) {
             return info;
         }
         klass = object_class_get_parent(klass);
@@ -521,7 +553,8 @@ DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
             continue;
         }
 
-        info = make_device_property_info(klass, prop->name, prop->type);
+        info = make_device_property_info(klass, prop->name, prop->type,
+                                         prop->is_alias, prop->alias_property);
         if (!info) {
             continue;
         }
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH v2 2/3] qom: add AliasProperty struct poniter for alias property in ObjectProperty

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 667 bytes --]

On 09/15/2014 11:51 PM, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
> 

s/poniter/pointer/ in the subject

> In this way, we can use target object and get its qdev
> property legacy_name etc.
> 
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  include/qom/object.h | 8 ++++++++
>  qom/object.c         | 7 ++-----
>  2 files changed, 10 insertions(+), 5 deletions(-)
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

Re: [Qemu-devel] [PATCH v2 2/3] qom: add AliasProperty struct poniter for alias property in ObjectProperty

[Gonglei (Arei)]

> From: Eric Blake [mailto:eblake@redhat.com]
> Sent: Tuesday, September 16, 2014 8:23 PM
> Subject: Re: [PATCH v2 2/3] qom: add AliasProperty struct poniter for alias
> property in ObjectProperty
> 
> On 09/15/2014 11:51 PM, arei.gonglei@huawei.com wrote:
> > From: Gonglei <arei.gonglei@huawei.com>
> >
> 
> s/poniter/pointer/ in the subject
> 
OK. Thanks. :)

Best regards,
-Gonglei

> > In this way, we can use target object and get its qdev
> > property legacy_name etc.
> >
> > Cc: Stefan Hajnoczi <stefanha@redhat.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > ---
> >  include/qom/object.h | 8 ++++++++
> >  qom/object.c         | 7 ++-----
> >  2 files changed, 10 insertions(+), 5 deletions(-)
> >
> 
> --
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org