All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH for 2.1 00/14] My pending qom and memory QOMification patches
@ 2014-06-25 12:25 Paolo Bonzini
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 01/14] qom: add object_property_add_alias() Paolo Bonzini
                   ` (13 more replies)
  0 siblings, 14 replies; 15+ messages in thread
From: Paolo Bonzini @ 2014-06-25 12:25 UTC (permalink / raw)
  To: qemu-devel

I'd like an ack for the first two patches, next monday I'll go
ahead and send a pull request if I haven't received it yet.

Marcelo Tosatti (1):
  mc146818rtc: add "rtc-time" link to "/machine/rtc"

Paolo Bonzini (6):
  qom: add a generic mechanism to resolve paths
  qom: allow creating an alias of a child<> property
  libqtest: escape strings in QMP commands, fix leak
  memory: MemoryRegion: use /machine as default owner
  memory: MemoryRegion: replace owner field with QOM parent
  memory: do not give a name to the internal exec.c regions

Peter Crosthwaite (6):
  qom: object: remove parent pointer when unparenting
  qom: object: Ignore refs/unrefs of NULL
  memory: MemoryRegion: QOMify
  memory: MemoryRegion: Add container and addr props
  memory: MemoryRegion: Add may-overlap and priority props
  memory: MemoryRegion: Add size property

Stefan Hajnoczi (1):
  qom: add object_property_add_alias()

 exec.c                 |  12 +--
 hw/timer/mc146818rtc.c |   9 ++
 include/exec/memory.h  |   9 +-
 include/qom/object.h   |  54 ++++++++++--
 memory.c               | 231 +++++++++++++++++++++++++++++++++++++++++++------
 qom/object.c           | 148 ++++++++++++++++++++++++-------
 tests/fdc-test.c       |   2 +-
 tests/libqtest.c       |  47 +++++++---
 tests/qom-test.c       |   6 +-
 tests/tmp105-test.c    |   4 +-
 10 files changed, 437 insertions(+), 85 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH for 2.1 01/14] qom: add object_property_add_alias()
  2014-06-25 12:25 [Qemu-devel] [PATCH for 2.1 00/14] My pending qom and memory QOMification patches Paolo Bonzini
@ 2014-06-25 12:25 ` Paolo Bonzini
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 02/14] qom: add a generic mechanism to resolve paths Paolo Bonzini
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2014-06-25 12:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Hajnoczi

From: Stefan Hajnoczi <stefanha@redhat.com>

Sometimes an object needs to present a property which is actually on
another object, or it needs to provide an alias name for an existing
property.

Examples:
  a.foo -> b.foo
  a.old_name -> a.new_name

The new 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.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qom/object.h | 20 ++++++++++++++++++++
 qom/object.c         | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+)

diff --git a/include/qom/object.h b/include/qom/object.h
index b882ccc..44c513f 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1231,6 +1231,26 @@ void object_property_add_uint64_ptr(Object *obj, const char *name,
                                     const uint64_t *v, Error **Errp);
 
 /**
+ * object_property_add_alias:
+ * @obj: the object to add a property to
+ * @name: the name of the property
+ * @target_obj: the object to forward property access to
+ * @target_name: the name of the property on the forwarded object
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * Add an alias for a property on an object.  This function will add a property
+ * of the same type as the forwarded property.
+ *
+ * The caller must ensure that <code>@target_obj</code> stays alive as long as
+ * this property exists.  In the case of a child object or an alias on the same
+ * object this will be the case.  For aliases to other objects the caller is
+ * responsible for taking a reference.
+ */
+void object_property_add_alias(Object *obj, const char *name,
+                               Object *target_obj, const char *target_name,
+                               Error **errp);
+
+/**
  * object_child_foreach:
  * @obj: the object whose children will be navigated
  * @fn: the iterator function to be called
diff --git a/qom/object.c b/qom/object.c
index 3876618..a760514 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1550,6 +1550,57 @@ 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)
+{
+    AliasProperty *prop = opaque;
+
+    object_property_get(prop->target_obj, v, prop->target_name, errp);
+}
+
+static void property_set_alias(Object *obj, struct Visitor *v, void *opaque,
+                               const char *name, Error **errp)
+{
+    AliasProperty *prop = opaque;
+
+    object_property_set(prop->target_obj, v, prop->target_name, errp);
+}
+
+static void property_release_alias(Object *obj, const char *name, void *opaque)
+{
+    AliasProperty *prop = opaque;
+
+    g_free(prop);
+}
+
+void object_property_add_alias(Object *obj, const char *name,
+                               Object *target_obj, const char *target_name,
+                               Error **errp)
+{
+    AliasProperty *prop;
+    ObjectProperty *target_prop;
+
+    target_prop = object_property_find(target_obj, target_name, errp);
+    if (!target_prop) {
+        return;
+    }
+
+    prop = g_malloc(sizeof(*prop));
+    prop->target_obj = target_obj;
+    prop->target_name = target_name;
+
+    object_property_add(obj, name, target_prop->type,
+                        property_get_alias,
+                        property_set_alias,
+                        property_release_alias,
+                        prop, errp);
+}
+
 static void object_instance_init(Object *obj)
 {
     object_property_add_str(obj, "type", qdev_get_type, NULL, NULL);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH for 2.1 02/14] qom: add a generic mechanism to resolve paths
  2014-06-25 12:25 [Qemu-devel] [PATCH for 2.1 00/14] My pending qom and memory QOMification patches Paolo Bonzini
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 01/14] qom: add object_property_add_alias() Paolo Bonzini
@ 2014-06-25 12:25 ` Paolo Bonzini
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 03/14] qom: allow creating an alias of a child<> property Paolo Bonzini
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2014-06-25 12:25 UTC (permalink / raw)
  To: qemu-devel

It may be desirable to have custom link<> properties that do more
than just store an object.  Even the addition of a "check"
function is not enough if setting the link has side effects
or if a non-standard reference counting is preferrable.

Avoid the assumption that the opaque field of a link<> is a
LinkProperty struct, by adding a generic "resolve" callback
to ObjectProperty.  This fixes aliases of link properties.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qom/object.h | 34 ++++++++++++++++++----
 qom/object.c         | 82 +++++++++++++++++++++++++++++++++-------------------
 2 files changed, 82 insertions(+), 34 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index 44c513f..8a05a81 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -304,6 +304,25 @@ typedef void (ObjectPropertyAccessor)(Object *obj,
                                       Error **errp);
 
 /**
+ * ObjectPropertyResolve:
+ * @obj: the object that owns the property
+ * @opaque: the opaque registered with the property
+ * @part: the name of the property
+ *
+ * Resolves the #Object corresponding to property @part.
+ *
+ * The returned object can also be used as a starting point
+ * to resolve a relative path starting with "@part".
+ *
+ * Returns: If @path is the path that led to @obj, the function
+ * returns the #Object corresponding to "@path/@part".
+ * If "@path/@part" is not a valid object path, it returns #NULL.
+ */
+typedef Object *(ObjectPropertyResolve)(Object *obj,
+                                        void *opaque,
+                                        const char *part);
+
+/**
  * ObjectPropertyRelease:
  * @obj: the object that owns the property
  * @name: the name of the property
@@ -321,6 +340,7 @@ typedef struct ObjectProperty
     gchar *type;
     ObjectPropertyAccessor *get;
     ObjectPropertyAccessor *set;
+    ObjectPropertyResolve *resolve;
     ObjectPropertyRelease *release;
     void *opaque;
 
@@ -787,12 +807,16 @@ void object_unref(Object *obj);
  *   destruction.  This may be NULL.
  * @opaque: an opaque pointer to pass to the callbacks for the property
  * @errp: returns an error if this function fails
+ *
+ * Returns: The #ObjectProperty; this can be used to set the @resolve
+ * callback for child and link properties.
  */
-void object_property_add(Object *obj, const char *name, const char *type,
-                         ObjectPropertyAccessor *get,
-                         ObjectPropertyAccessor *set,
-                         ObjectPropertyRelease *release,
-                         void *opaque, Error **errp);
+ObjectProperty *object_property_add(Object *obj, const char *name,
+                                    const char *type,
+                                    ObjectPropertyAccessor *get,
+                                    ObjectPropertyAccessor *set,
+                                    ObjectPropertyRelease *release,
+                                    void *opaque, Error **errp);
 
 void object_property_del(Object *obj, const char *name, Error **errp);
 
diff --git a/qom/object.c b/qom/object.c
index a760514..1c97022 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -356,11 +356,6 @@ static inline bool object_property_is_child(ObjectProperty *prop)
     return strstart(prop->type, "child<", NULL);
 }
 
-static inline bool object_property_is_link(ObjectProperty *prop)
-{
-    return strstart(prop->type, "link<", NULL);
-}
-
 static void object_property_del_all(Object *obj)
 {
     while (!QTAILQ_EMPTY(&obj->properties)) {
@@ -728,11 +723,12 @@ void object_unref(Object *obj)
     }
 }
 
-void object_property_add(Object *obj, const char *name, const char *type,
-                         ObjectPropertyAccessor *get,
-                         ObjectPropertyAccessor *set,
-                         ObjectPropertyRelease *release,
-                         void *opaque, Error **errp)
+ObjectProperty *
+object_property_add(Object *obj, const char *name, const char *type,
+                    ObjectPropertyAccessor *get,
+                    ObjectPropertyAccessor *set,
+                    ObjectPropertyRelease *release,
+                    void *opaque, Error **errp)
 {
     ObjectProperty *prop;
 
@@ -741,7 +737,7 @@ void object_property_add(Object *obj, const char *name, const char *type,
             error_setg(errp, "attempt to add duplicate property '%s'"
                        " to object (type '%s')", name,
                        object_get_typename(obj));
-            return;
+            return NULL;
         }
     }
 
@@ -756,6 +752,7 @@ void object_property_add(Object *obj, const char *name, const char *type,
     prop->opaque = opaque;
 
     QTAILQ_INSERT_TAIL(&obj->properties, prop, node);
+    return prop;
 }
 
 ObjectProperty *object_property_find(Object *obj, const char *name,
@@ -1028,6 +1025,11 @@ static void object_get_child_property(Object *obj, Visitor *v, void *opaque,
     g_free(path);
 }
 
+static Object *object_resolve_child_property(Object *parent, void *opaque, const gchar *part)
+{
+    return opaque;
+}
+
 static void object_finalize_child_property(Object *obj, const char *name,
                                            void *opaque)
 {
@@ -1041,15 +1043,18 @@ void object_property_add_child(Object *obj, const char *name,
 {
     Error *local_err = NULL;
     gchar *type;
+    ObjectProperty *op;
 
     type = g_strdup_printf("child<%s>", object_get_typename(OBJECT(child)));
 
-    object_property_add(obj, name, type, object_get_child_property, NULL,
-                        object_finalize_child_property, child, &local_err);
+    op = object_property_add(obj, name, type, object_get_child_property, NULL,
+                             object_finalize_child_property, child, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         goto out;
     }
+
+    op->resolve = object_resolve_child_property;
     object_ref(child);
     g_assert(child->parent == NULL);
     child->parent = obj;
@@ -1163,6 +1168,13 @@ static void object_set_link_property(Object *obj, Visitor *v, void *opaque,
     }
 }
 
+static Object *object_resolve_link_property(Object *parent, void *opaque, const gchar *part)
+{
+    LinkProperty *lprop = opaque;
+
+    return *lprop->child;
+}
+
 static void object_release_link_property(Object *obj, const char *name,
                                          void *opaque)
 {
@@ -1184,6 +1196,7 @@ void object_property_add_link(Object *obj, const char *name,
     Error *local_err = NULL;
     LinkProperty *prop = g_malloc(sizeof(*prop));
     gchar *full_type;
+    ObjectProperty *op;
 
     prop->child = child;
     prop->check = check;
@@ -1191,17 +1204,21 @@ void object_property_add_link(Object *obj, const char *name,
 
     full_type = g_strdup_printf("link<%s>", type);
 
-    object_property_add(obj, name, full_type,
-                        object_get_link_property,
-                        check ? object_set_link_property : NULL,
-                        object_release_link_property,
-                        prop,
-                        &local_err);
+    op = object_property_add(obj, name, full_type,
+                             object_get_link_property,
+                             check ? object_set_link_property : NULL,
+                             object_release_link_property,
+                             prop,
+                             &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         g_free(prop);
+        goto out;
     }
 
+    op->resolve = object_resolve_link_property;
+
+out:
     g_free(full_type);
 }
 
@@ -1260,11 +1277,8 @@ Object *object_resolve_path_component(Object *parent, const gchar *part)
         return NULL;
     }
 
-    if (object_property_is_link(prop)) {
-        LinkProperty *lprop = prop->opaque;
-        return *lprop->child;
-    } else if (object_property_is_child(prop)) {
-        return prop->opaque;
+    if (prop->resolve) {
+        return prop->resolve(parent, prop->opaque, part);
     } else {
         return NULL;
     }
@@ -1571,6 +1585,14 @@ static void property_set_alias(Object *obj, struct Visitor *v, void *opaque,
     object_property_set(prop->target_obj, v, prop->target_name, errp);
 }
 
+static Object *property_resolve_alias(Object *obj, void *opaque,
+                                      const gchar *part)
+{
+    AliasProperty *prop = opaque;
+
+    return object_resolve_path_component(prop->target_obj, prop->target_name);
+}
+
 static void property_release_alias(Object *obj, const char *name, void *opaque)
 {
     AliasProperty *prop = opaque;
@@ -1583,6 +1605,7 @@ void object_property_add_alias(Object *obj, const char *name,
                                Error **errp)
 {
     AliasProperty *prop;
+    ObjectProperty *op;
     ObjectProperty *target_prop;
 
     target_prop = object_property_find(target_obj, target_name, errp);
@@ -1594,11 +1617,12 @@ void object_property_add_alias(Object *obj, const char *name,
     prop->target_obj = target_obj;
     prop->target_name = target_name;
 
-    object_property_add(obj, name, target_prop->type,
-                        property_get_alias,
-                        property_set_alias,
-                        property_release_alias,
-                        prop, errp);
+    op = object_property_add(obj, name, target_prop->type,
+                             property_get_alias,
+                             property_set_alias,
+                             property_release_alias,
+                             alias, errp);
+    op->resolve = property_resolve_alias;
 }
 
 static void object_instance_init(Object *obj)
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH for 2.1 03/14] qom: allow creating an alias of a child<> property
  2014-06-25 12:25 [Qemu-devel] [PATCH for 2.1 00/14] My pending qom and memory QOMification patches Paolo Bonzini
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 01/14] qom: add object_property_add_alias() Paolo Bonzini
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 02/14] qom: add a generic mechanism to resolve paths Paolo Bonzini
@ 2014-06-25 12:25 ` Paolo Bonzini
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 04/14] mc146818rtc: add "rtc-time" link to "/machine/rtc" Paolo Bonzini
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2014-06-25 12:25 UTC (permalink / raw)
  To: qemu-devel

Child properties must be unique.  Fix this problem by
turning their aliases into links.

The resolve function that forwards to the target property
does not have any knowledge of the target property's type,
so it works fine.

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qom/object.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/qom/object.c b/qom/object.c
index 1c97022..4d1c999 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1607,22 +1607,32 @@ void object_property_add_alias(Object *obj, const char *name,
     AliasProperty *prop;
     ObjectProperty *op;
     ObjectProperty *target_prop;
+    gchar *prop_type;
 
     target_prop = object_property_find(target_obj, target_name, errp);
     if (!target_prop) {
         return;
     }
 
+    if (object_property_is_child(target_prop)) {
+        prop_type = g_strdup_printf("link%s",
+                                    target_prop->type + strlen("child"));
+    } else {
+        prop_type = g_strdup(target_prop->type);
+    }
+
     prop = g_malloc(sizeof(*prop));
     prop->target_obj = target_obj;
     prop->target_name = target_name;
 
-    op = object_property_add(obj, name, target_prop->type,
+    op = object_property_add(obj, name, prop_type,
                              property_get_alias,
                              property_set_alias,
                              property_release_alias,
                              alias, errp);
     op->resolve = property_resolve_alias;
+
+    g_free(prop_type);
 }
 
 static void object_instance_init(Object *obj)
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH for 2.1 04/14] mc146818rtc: add "rtc-time" link to "/machine/rtc"
  2014-06-25 12:25 [Qemu-devel] [PATCH for 2.1 00/14] My pending qom and memory QOMification patches Paolo Bonzini
                   ` (2 preceding siblings ...)
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 03/14] qom: allow creating an alias of a child<> property Paolo Bonzini
@ 2014-06-25 12:25 ` Paolo Bonzini
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 05/14] qom: object: remove parent pointer when unparenting Paolo Bonzini
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2014-06-25 12:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Marcelo Tosatti

From: Marcelo Tosatti <mtosatti@redhat.com>

Add a link to rtc under /machine providing a stable
location for management apps to query the value of the
time.  The link should be added by any object that sends
RTC_TIME_CHANGE events.

{"execute":"qom-get","arguments":{"path":"/machine","property":"rtc-date"} }

Suggested by Paolo Bonzini and Andreas Faerber.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/timer/mc146818rtc.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
index 05002bf..81eb517 100644
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -893,6 +893,9 @@ static void rtc_realizefn(DeviceState *dev, Error **errp)
 
     object_property_add(OBJECT(s), "date", "struct tm",
                         rtc_get_date, NULL, NULL, s, NULL);
+
+    object_property_add_alias(qdev_get_machine(), "rtc-time",
+                              OBJECT(s), "date", NULL);
 }
 
 ISADevice *rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq)
@@ -932,11 +935,17 @@ static void rtc_class_initfn(ObjectClass *klass, void *data)
     dc->cannot_instantiate_with_device_add_yet = true;
 }
 
+static void rtc_finalize(Object *obj)
+{
+    object_property_del(qdev_get_machine(), "rtc", NULL);
+}
+
 static const TypeInfo mc146818rtc_info = {
     .name          = TYPE_MC146818_RTC,
     .parent        = TYPE_ISA_DEVICE,
     .instance_size = sizeof(RTCState),
     .class_init    = rtc_class_initfn,
+    .instance_finalize = rtc_finalize,
 };
 
 static void mc146818rtc_register_types(void)
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH for 2.1 05/14] qom: object: remove parent pointer when unparenting
  2014-06-25 12:25 [Qemu-devel] [PATCH for 2.1 00/14] My pending qom and memory QOMification patches Paolo Bonzini
                   ` (3 preceding siblings ...)
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 04/14] mc146818rtc: add "rtc-time" link to "/machine/rtc" Paolo Bonzini
@ 2014-06-25 12:25 ` Paolo Bonzini
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 06/14] qom: object: Ignore refs/unrefs of NULL Paolo Bonzini
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2014-06-25 12:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Crosthwaite

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Certain parts of the QOM framework test this pointer to determine if
an object is parented. Nuke it when the object is unparented to allow
for reuse of an object after unparenting.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qom/object.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/qom/object.c b/qom/object.c
index 4d1c999..a3b8cf3 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -397,6 +397,7 @@ void object_unparent(Object *obj)
     }
     if (obj->parent) {
         object_property_del_child(obj->parent, obj, NULL);
+        obj->parent = NULL;
     }
     object_unref(obj);
 }
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH for 2.1 06/14] qom: object: Ignore refs/unrefs of NULL
  2014-06-25 12:25 [Qemu-devel] [PATCH for 2.1 00/14] My pending qom and memory QOMification patches Paolo Bonzini
                   ` (4 preceding siblings ...)
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 05/14] qom: object: remove parent pointer when unparenting Paolo Bonzini
@ 2014-06-25 12:25 ` Paolo Bonzini
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 07/14] libqtest: escape strings in QMP commands, fix leak Paolo Bonzini
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2014-06-25 12:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Crosthwaite

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Just do nothing if passed NULL for a ref or unref. This avoids
call sites that manage a combination of NULL or non-NULL pointers
having to add iffery around every ref and unref.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qom/object.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index a3b8cf3..31bf8b9 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -711,11 +711,17 @@ GSList *object_class_get_list(const char *implements_type,
 
 void object_ref(Object *obj)
 {
+    if (!obj) {
+        return;
+    }
      atomic_inc(&obj->ref);
 }
 
 void object_unref(Object *obj)
 {
+    if (!obj) {
+        return;
+    }
     g_assert(obj->ref > 0);
 
     /* parent always holds a reference to its children */
@@ -1160,13 +1166,9 @@ static void object_set_link_property(Object *obj, Visitor *v, void *opaque,
         return;
     }
 
-    if (new_target) {
-        object_ref(new_target);
-    }
+    object_ref(new_target);
     *child = new_target;
-    if (old_target != NULL) {
-        object_unref(old_target);
-    }
+    object_unref(old_target);
 }
 
 static Object *object_resolve_link_property(Object *parent, void *opaque, const gchar *part)
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH for 2.1 07/14] libqtest: escape strings in QMP commands, fix leak
  2014-06-25 12:25 [Qemu-devel] [PATCH for 2.1 00/14] My pending qom and memory QOMification patches Paolo Bonzini
                   ` (5 preceding siblings ...)
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 06/14] qom: object: Ignore refs/unrefs of NULL Paolo Bonzini
@ 2014-06-25 12:25 ` Paolo Bonzini
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 08/14] memory: MemoryRegion: use /machine as default owner Paolo Bonzini
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2014-06-25 12:25 UTC (permalink / raw)
  To: qemu-devel

libqtest is using g_strdup_printf to format QMP commands, but
this does not work if the argument strings need to be escaped.
Instead, use the fancy %-formatting functionality of QObject.
The only change required in tests is that strings have to be
formatted as %s, not '%s' or \"%s\".  Luckily this usage of
parameterized QMP commands is not that frequent.

The leak is in socket_sendf.  Since we are extracting the send
loop to a new function, fix it now.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Amos Kong <akong@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/fdc-test.c    |  2 +-
 tests/libqtest.c    | 47 +++++++++++++++++++++++++++++++++++++----------
 tests/qom-test.c    |  6 +++---
 tests/tmp105-test.c |  4 ++--
 4 files changed, 43 insertions(+), 16 deletions(-)

diff --git a/tests/fdc-test.c b/tests/fdc-test.c
index 37096dc..c8e1e7b 100644
--- a/tests/fdc-test.c
+++ b/tests/fdc-test.c
@@ -291,7 +291,7 @@ static void test_media_insert(void)
     /* Insert media in drive. DSKCHK should not be reset until a step pulse
      * is sent. */
     qmp_discard_response("{'execute':'change', 'arguments':{"
-                         " 'device':'floppy0', 'target': '%s' }}",
+                         " 'device':'floppy0', 'target': %s }}",
                          test_image);
     qmp_discard_response(""); /* ignore event
                                  (FIXME open -> open transition?!) */
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 71468ac..98e8f4b 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -30,8 +30,9 @@
 
 #include "qemu/compiler.h"
 #include "qemu/osdep.h"
-#include "qapi/qmp/json-streamer.h"
 #include "qapi/qmp/json-parser.h"
+#include "qapi/qmp/json-streamer.h"
+#include "qapi/qmp/qjson.h"
 
 #define MAX_IRQ 256
 #define SOCKET_TIMEOUT 5
@@ -220,19 +221,15 @@ void qtest_quit(QTestState *s)
     g_free(s);
 }
 
-static void socket_sendf(int fd, const char *fmt, va_list ap)
+static void socket_send(int fd, const char *buf, size_t size)
 {
-    gchar *str;
-    size_t size, offset;
-
-    str = g_strdup_vprintf(fmt, ap);
-    size = strlen(str);
+    size_t offset;
 
     offset = 0;
     while (offset < size) {
         ssize_t len;
 
-        len = write(fd, str + offset, size - offset);
+        len = write(fd, buf + offset, size - offset);
         if (len == -1 && errno == EINTR) {
             continue;
         }
@@ -244,6 +241,15 @@ static void socket_sendf(int fd, const char *fmt, va_list ap)
     }
 }
 
+static void socket_sendf(int fd, const char *fmt, va_list ap)
+{
+    gchar *str = g_strdup_vprintf(fmt, ap);
+    size_t size = strlen(str);
+
+    socket_send(fd, str, size);
+    g_free(str);
+}
+
 static void GCC_FMT_ATTR(2, 3) qtest_sendf(QTestState *s, const char *fmt, ...)
 {
     va_list ap;
@@ -378,8 +384,29 @@ QDict *qtest_qmp_receive(QTestState *s)
 
 QDict *qtest_qmpv(QTestState *s, const char *fmt, va_list ap)
 {
-    /* Send QMP request */
-    socket_sendf(s->qmp_fd, fmt, ap);
+    va_list ap_copy;
+    QObject *qobj;
+
+    /* Going through qobject ensures we escape strings properly.
+     * This seemingly unnecessary copy is required in case va_list
+     * is an array type.
+     */
+    va_copy(ap_copy, ap);
+    qobj = qobject_from_jsonv(fmt, &ap_copy);
+    va_end(ap_copy);
+
+    /* No need to send anything for an empty QObject.  */
+    if (qobj) {
+        QString *qstr = qobject_to_json(qobj);
+        const char *str = qstring_get_str(qstr);
+        size_t size = qstring_get_length(qstr);
+
+        /* Send QMP request */
+        socket_send(s->qmp_fd, str, size);
+
+        QDECREF(qstr);
+        qobject_decref(qobj);
+    }
 
     /* Receive reply */
     return qtest_qmp_receive(s);
diff --git a/tests/qom-test.c b/tests/qom-test.c
index d8d1d8d..4246382 100644
--- a/tests/qom-test.c
+++ b/tests/qom-test.c
@@ -53,7 +53,7 @@ static void test_properties(const char *path, bool recurse)
 
     g_test_message("Obtaining properties of %s", path);
     response = qmp("{ 'execute': 'qom-list',"
-                   "  'arguments': { 'path': '%s' } }", path);
+                   "  'arguments': { 'path': %s } }", path);
     g_assert(response);
 
     if (!recurse) {
@@ -76,8 +76,8 @@ static void test_properties(const char *path, bool recurse)
             const char *prop = qdict_get_str(tuple, "name");
             g_test_message("Testing property %s.%s", path, prop);
             response = qmp("{ 'execute': 'qom-get',"
-                           "  'arguments': { 'path': '%s',"
-                           "                 'property': '%s' } }",
+                           "  'arguments': { 'path': %s,"
+                           "                 'property': %s } }",
                            path, prop);
             /* qom-get may fail but should not, e.g., segfault. */
             g_assert(response);
diff --git a/tests/tmp105-test.c b/tests/tmp105-test.c
index 15ddaf3..99db538 100644
--- a/tests/tmp105-test.c
+++ b/tests/tmp105-test.c
@@ -69,7 +69,7 @@ static int qmp_tmp105_get_temperature(const char *id)
     QDict *response;
     int ret;
 
-    response = qmp("{ 'execute': 'qom-get', 'arguments': { 'path': '%s', "
+    response = qmp("{ 'execute': 'qom-get', 'arguments': { 'path': %s, "
                    "'property': 'temperature' } }", id);
     g_assert(qdict_haskey(response, "return"));
     ret = qdict_get_int(response, "return");
@@ -81,7 +81,7 @@ static void qmp_tmp105_set_temperature(const char *id, int value)
 {
     QDict *response;
 
-    response = qmp("{ 'execute': 'qom-set', 'arguments': { 'path': '%s', "
+    response = qmp("{ 'execute': 'qom-set', 'arguments': { 'path': %s, "
                    "'property': 'temperature', 'value': %d } }", id, value);
     g_assert(qdict_haskey(response, "return"));
     QDECREF(response);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH for 2.1 08/14] memory: MemoryRegion: use /machine as default owner
  2014-06-25 12:25 [Qemu-devel] [PATCH for 2.1 00/14] My pending qom and memory QOMification patches Paolo Bonzini
                   ` (6 preceding siblings ...)
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 07/14] libqtest: escape strings in QMP commands, fix leak Paolo Bonzini
@ 2014-06-25 12:25 ` Paolo Bonzini
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 09/14] memory: MemoryRegion: QOMify Paolo Bonzini
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2014-06-25 12:25 UTC (permalink / raw)
  To: qemu-devel

This will be added (after QOMification) as the QOM parent.

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 memory.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/memory.c b/memory.c
index b91a60a..7eaa1e9 100644
--- a/memory.c
+++ b/memory.c
@@ -849,7 +849,7 @@ void memory_region_init(MemoryRegion *mr,
 {
     mr->ops = &unassigned_mem_ops;
     mr->opaque = NULL;
-    mr->owner = owner;
+    mr->owner = owner ? owner : qdev_get_machine();
     mr->iommu_ops = NULL;
     mr->container = NULL;
     mr->size = int128_make64(size);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH for 2.1 09/14] memory: MemoryRegion: QOMify
  2014-06-25 12:25 [Qemu-devel] [PATCH for 2.1 00/14] My pending qom and memory QOMification patches Paolo Bonzini
                   ` (7 preceding siblings ...)
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 08/14] memory: MemoryRegion: use /machine as default owner Paolo Bonzini
@ 2014-06-25 12:25 ` Paolo Bonzini
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 10/14] memory: MemoryRegion: replace owner field with QOM parent Paolo Bonzini
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2014-06-25 12:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Crosthwaite

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

QOMify memory regions as an Object. The former init() and destroy()
routines become instance_init() and instance_finalize() resp.

memory_region_init() is re-implemented to be:
object_initialize() + set fields

memory_region_destroy() is re-implemented to call unparent().

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
[Add newly-created MR as child, unparent on destruction. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c                |   4 +-
 include/exec/memory.h |   6 +++
 memory.c              | 121 +++++++++++++++++++++++++++++++++++++++++---------
 3 files changed, 109 insertions(+), 22 deletions(-)

diff --git a/exec.c b/exec.c
index c849405..6e2c4f0 100644
--- a/exec.c
+++ b/exec.c
@@ -883,7 +883,7 @@ static void phys_section_destroy(MemoryRegion *mr)
 
     if (mr->subpage) {
         subpage_t *subpage = container_of(mr, subpage_t, iomem);
-        memory_region_destroy(&subpage->iomem);
+        object_unref(OBJECT(&subpage->iomem));
         g_free(subpage);
     }
 }
@@ -1761,7 +1761,7 @@ static subpage_t *subpage_init(AddressSpace *as, hwaddr base)
     mmio->as = as;
     mmio->base = base;
     memory_region_init_io(&mmio->iomem, NULL, &subpage_ops, mmio,
-                          "subpage", TARGET_PAGE_SIZE);
+                          NULL, TARGET_PAGE_SIZE);
     mmio->iomem.subpage = true;
 #if defined(DEBUG_SUBPAGE)
     printf("%s: %p base " TARGET_FMT_plx " len %08x\n", __func__,
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 3d778d7..85b56e2 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -32,10 +32,15 @@
 #include "qemu/int128.h"
 #include "qemu/notify.h"
 #include "qapi/error.h"
+#include "qom/object.h"
 
 #define MAX_PHYS_ADDR_SPACE_BITS 62
 #define MAX_PHYS_ADDR            (((hwaddr)1 << MAX_PHYS_ADDR_SPACE_BITS) - 1)
 
+#define TYPE_MEMORY_REGION "qemu:memory-region"
+#define MEMORY_REGION(obj) \
+        OBJECT_CHECK(MemoryRegion, (obj), TYPE_MEMORY_REGION)
+
 typedef struct MemoryRegionOps MemoryRegionOps;
 typedef struct MemoryRegionMmio MemoryRegionMmio;
 
@@ -131,6 +136,7 @@ typedef struct CoalescedMemoryRange CoalescedMemoryRange;
 typedef struct MemoryRegionIoeventfd MemoryRegionIoeventfd;
 
 struct MemoryRegion {
+    Object parent_obj;
     /* All fields are private - violators will be prosecuted */
     const MemoryRegionOps *ops;
     const MemoryRegionIOMMUOps *iommu_ops;
diff --git a/memory.c b/memory.c
index 7eaa1e9..9397fec 100644
--- a/memory.c
+++ b/memory.c
@@ -842,40 +842,94 @@ static void memory_region_destructor_rom_device(MemoryRegion *mr)
     qemu_ram_free(mr->ram_addr & TARGET_PAGE_MASK);
 }
 
+static bool memory_region_need_escape(char c)
+{
+    return c == '/' || c == '[' || c == '\\' || c == ']';
+}
+
+static char *memory_region_escape_name(const char *name)
+{
+    const char *p;
+    char *escaped, *q;
+    uint8_t c;
+    size_t bytes = 0;
+
+    for (p = name; *p; p++) {
+        bytes += memory_region_need_escape(*p) ? 4 : 1;
+    }
+    if (bytes == p - name) {
+       return g_memdup(name, bytes + 1);
+    }
+
+    escaped = g_malloc(bytes + 1);
+    for (p = name, q = escaped; *p; p++) {
+        c = *p;
+        if (unlikely(memory_region_need_escape(c))) {
+            *q++ = '\\';
+            *q++ = 'x';
+            *q++ = "0123456789abcdef"[c >> 4];
+            c = "0123456789abcdef"[c & 15];
+        }
+        *q++ = c;
+    }
+    *q = 0;
+    return escaped;
+}
+
+static void object_property_add_child_array(Object *owner,
+                                            const char *name,
+                                            Object *child)
+{
+    int i;
+    char *base_name = memory_region_escape_name(name);
+
+    for (i = 0; ; i++) {
+        char *full_name = g_strdup_printf("%s[%d]", base_name, i);
+        Error *local_err = NULL;
+
+        object_property_add_child(owner, full_name, child, &local_err);
+        g_free(full_name);
+        if (!local_err) {
+            break;
+        }
+
+        error_free(local_err);
+    }
+
+    g_free(base_name);
+}
+        
+
 void memory_region_init(MemoryRegion *mr,
                         Object *owner,
                         const char *name,
                         uint64_t size)
 {
-    mr->ops = &unassigned_mem_ops;
-    mr->opaque = NULL;
+    object_initialize(mr, sizeof(*mr), TYPE_MEMORY_REGION);
+
     mr->owner = owner ? owner : qdev_get_machine();
-    mr->iommu_ops = NULL;
-    mr->container = NULL;
     mr->size = int128_make64(size);
     if (size == UINT64_MAX) {
         mr->size = int128_2_64();
     }
-    mr->addr = 0;
-    mr->subpage = false;
+    mr->name = g_strdup(name);
+
+    if (name) {
+        object_property_add_child_array(mr->owner, name, OBJECT(mr));
+        object_unref(OBJECT(mr));
+    }
+}
+
+static void memory_region_initfn(Object *obj)
+{
+    MemoryRegion *mr = MEMORY_REGION(obj);
+
+    mr->ops = &unassigned_mem_ops;
     mr->enabled = true;
-    mr->terminates = false;
-    mr->ram = false;
     mr->romd_mode = true;
-    mr->readonly = false;
-    mr->rom_device = false;
     mr->destructor = memory_region_destructor_none;
-    mr->priority = 0;
-    mr->may_overlap = false;
-    mr->alias = NULL;
     QTAILQ_INIT(&mr->subregions);
-    memset(&mr->subregions_link, 0, sizeof mr->subregions_link);
     QTAILQ_INIT(&mr->coalesced);
-    mr->name = g_strdup(name);
-    mr->dirty_log_mask = 0;
-    mr->ioeventfd_nb = 0;
-    mr->ioeventfds = NULL;
-    mr->flush_coalesced_mmio = false;
 }
 
 static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
@@ -1113,8 +1167,10 @@ void memory_region_init_reservation(MemoryRegion *mr,
     memory_region_init_io(mr, owner, &unassigned_mem_ops, mr, name, size);
 }
 
-void memory_region_destroy(MemoryRegion *mr)
+static void memory_region_finalize(Object *obj)
 {
+    MemoryRegion *mr = MEMORY_REGION(obj);
+
     assert(QTAILQ_EMPTY(&mr->subregions));
     assert(memory_region_transaction_depth == 0);
     mr->destructor(mr);
@@ -1123,6 +1179,12 @@ void memory_region_destroy(MemoryRegion *mr)
     g_free(mr->ioeventfds);
 }
 
+void memory_region_destroy(MemoryRegion *mr)
+{
+    object_unparent(OBJECT(mr));
+}
+
+
 Object *memory_region_owner(MemoryRegion *mr)
 {
     return mr->owner;
@@ -1132,6 +1194,8 @@ void memory_region_ref(MemoryRegion *mr)
 {
     if (mr && mr->owner) {
         object_ref(mr->owner);
+    } else {
+        object_ref(OBJECT(mr));
     }
 }
 
@@ -1139,6 +1203,8 @@ void memory_region_unref(MemoryRegion *mr)
 {
     if (mr && mr->owner) {
         object_unref(mr->owner);
+    } else {
+        object_unref(OBJECT(mr));
     }
 }
 
@@ -1946,3 +2012,18 @@ void mtree_info(fprintf_function mon_printf, void *f)
         g_free(ml);
     }
 }
+
+static const TypeInfo memory_region_info = {
+    .parent             = TYPE_OBJECT,
+    .name               = TYPE_MEMORY_REGION,
+    .instance_size      = sizeof(MemoryRegion),
+    .instance_init      = memory_region_initfn,
+    .instance_finalize  = memory_region_finalize,
+};
+
+static void memory_register_types(void)
+{
+    type_register_static(&memory_region_info);
+}
+
+type_init(memory_register_types)
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH for 2.1 10/14] memory: MemoryRegion: replace owner field with QOM parent
  2014-06-25 12:25 [Qemu-devel] [PATCH for 2.1 00/14] My pending qom and memory QOMification patches Paolo Bonzini
                   ` (8 preceding siblings ...)
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 09/14] memory: MemoryRegion: QOMify Paolo Bonzini
@ 2014-06-25 12:25 ` Paolo Bonzini
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 11/14] memory: MemoryRegion: Add container and addr props Paolo Bonzini
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2014-06-25 12:25 UTC (permalink / raw)
  To: qemu-devel

The two are now the same.

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/exec/memory.h |  1 -
 memory.c              | 35 +++++++++++++++++++++++++----------
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 85b56e2..0c7e825 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -141,7 +141,6 @@ struct MemoryRegion {
     const MemoryRegionOps *ops;
     const MemoryRegionIOMMUOps *iommu_ops;
     void *opaque;
-    struct Object *owner;
     MemoryRegion *container;
     Int128 size;
     hwaddr addr;
diff --git a/memory.c b/memory.c
index 9397fec..8970081 100644
--- a/memory.c
+++ b/memory.c
@@ -905,9 +905,11 @@ void memory_region_init(MemoryRegion *mr,
                         const char *name,
                         uint64_t size)
 {
-    object_initialize(mr, sizeof(*mr), TYPE_MEMORY_REGION);
+    if (!owner) {
+        owner = qdev_get_machine();
+    }
 
-    mr->owner = owner ? owner : qdev_get_machine();
+    object_initialize(mr, sizeof(*mr), TYPE_MEMORY_REGION);
     mr->size = int128_make64(size);
     if (size == UINT64_MAX) {
         mr->size = int128_2_64();
@@ -915,7 +917,7 @@ void memory_region_init(MemoryRegion *mr,
     mr->name = g_strdup(name);
 
     if (name) {
-        object_property_add_child_array(mr->owner, name, OBJECT(mr));
+        object_property_add_child_array(owner, name, OBJECT(mr));
         object_unref(OBJECT(mr));
     }
 }
@@ -1187,24 +1189,37 @@ void memory_region_destroy(MemoryRegion *mr)
 
 Object *memory_region_owner(MemoryRegion *mr)
 {
-    return mr->owner;
+    Object *obj = OBJECT(mr);
+    return obj->parent;
 }
 
 void memory_region_ref(MemoryRegion *mr)
 {
-    if (mr && mr->owner) {
-        object_ref(mr->owner);
+    /* MMIO callbacks most likely will access data that belongs
+     * to the owner, hence the need to ref/unref the owner whenever
+     * the memory region is in use.
+     *
+     * The memory region is a child of its owner.  As long as the
+     * owner doesn't call unparent itself on the memory region,
+     * ref-ing the owner will also keep the memory region alive.
+     * Memory regions without an owner are supposed to never go away,
+     * but we still ref/unref them for debugging purposes.
+     */
+    Object *obj = OBJECT(mr);
+    if (obj && obj->parent) {
+        object_ref(obj->parent);
     } else {
-        object_ref(OBJECT(mr));
+        object_ref(obj);
     }
 }
 
 void memory_region_unref(MemoryRegion *mr)
 {
-    if (mr && mr->owner) {
-        object_unref(mr->owner);
+    Object *obj = OBJECT(mr);
+    if (obj && obj->parent) {
+        object_unref(obj->parent);
     } else {
-        object_unref(OBJECT(mr));
+        object_unref(obj);
     }
 }
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH for 2.1 11/14] memory: MemoryRegion: Add container and addr props
  2014-06-25 12:25 [Qemu-devel] [PATCH for 2.1 00/14] My pending qom and memory QOMification patches Paolo Bonzini
                   ` (9 preceding siblings ...)
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 10/14] memory: MemoryRegion: replace owner field with QOM parent Paolo Bonzini
@ 2014-06-25 12:25 ` Paolo Bonzini
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 12/14] memory: MemoryRegion: Add may-overlap and priority props Paolo Bonzini
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2014-06-25 12:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Crosthwaite

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Expose the already existing .parent and .addr fields as QOM properties.
.parent (i.e. the field describing the memory region that contains this
one in Memory hierachy) is renamed "container". This is to avoid
confusion with the QOM parent.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
[Remove setters.  Do not unref parent on releasing the property. Clean
 up error propagation. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 memory.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/memory.c b/memory.c
index 8970081..04dc933 100644
--- a/memory.c
+++ b/memory.c
@@ -16,6 +16,7 @@
 #include "exec/memory.h"
 #include "exec/address-spaces.h"
 #include "exec/ioport.h"
+#include "qapi/visitor.h"
 #include "qemu/bitops.h"
 #include "qom/object.h"
 #include "trace.h"
@@ -922,9 +923,42 @@ void memory_region_init(MemoryRegion *mr,
     }
 }
 
+static void memory_region_get_addr(Object *obj, Visitor *v, void *opaque,
+                                   const char *name, Error **errp)
+{
+    MemoryRegion *mr = MEMORY_REGION(obj);
+    uint64_t value = mr->addr;
+
+    visit_type_uint64(v, &value, name, errp);
+}
+
+static void memory_region_get_container(Object *obj, Visitor *v, void *opaque,
+                                        const char *name, Error **errp)
+{
+    MemoryRegion *mr = MEMORY_REGION(obj);
+    gchar *path = (gchar *)"";
+
+    if (mr->container) {
+        path = object_get_canonical_path(OBJECT(mr->container));
+    }
+    visit_type_str(v, &path, name, errp);
+    if (mr->container) {
+        g_free(path);
+    }
+}
+
+static Object *memory_region_resolve_container(Object *obj, void *opaque,
+                                               const char *part)
+{
+    MemoryRegion *mr = MEMORY_REGION(obj);
+
+    return OBJECT(mr->container);
+}
+
 static void memory_region_initfn(Object *obj)
 {
     MemoryRegion *mr = MEMORY_REGION(obj);
+    ObjectProperty *op;
 
     mr->ops = &unassigned_mem_ops;
     mr->enabled = true;
@@ -932,6 +966,18 @@ static void memory_region_initfn(Object *obj)
     mr->destructor = memory_region_destructor_none;
     QTAILQ_INIT(&mr->subregions);
     QTAILQ_INIT(&mr->coalesced);
+
+    op = object_property_add(OBJECT(mr), "container",
+                             "link<" TYPE_MEMORY_REGION ">",
+                             memory_region_get_container,
+                             NULL, /* memory_region_set_container */
+                             NULL, NULL, &error_abort);
+    op->resolve = memory_region_resolve_container;
+
+    object_property_add(OBJECT(mr), "addr", "uint64",
+                        memory_region_get_addr,
+                        NULL, /* memory_region_set_addr */
+                        NULL, NULL, &error_abort);
 }
 
 static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH for 2.1 12/14] memory: MemoryRegion: Add may-overlap and priority props
  2014-06-25 12:25 [Qemu-devel] [PATCH for 2.1 00/14] My pending qom and memory QOMification patches Paolo Bonzini
                   ` (10 preceding siblings ...)
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 11/14] memory: MemoryRegion: Add container and addr props Paolo Bonzini
@ 2014-06-25 12:25 ` Paolo Bonzini
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 13/14] memory: MemoryRegion: Add size property Paolo Bonzini
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 14/14] memory: do not give a name to the internal exec.c regions Paolo Bonzini
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2014-06-25 12:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Crosthwaite

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

QOM propertyify the .may-overlap and .priority fields. The setters
will re-add the memory as a subregion if needed (i.e. the values change
when the memory region is already contained).

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
[Remove setters. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/exec/memory.h |  2 +-
 memory.c              | 24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 0c7e825..e2c8e3e 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -157,7 +157,7 @@ struct MemoryRegion {
     bool flush_coalesced_mmio;
     MemoryRegion *alias;
     hwaddr alias_offset;
-    int priority;
+    int32_t priority;
     bool may_overlap;
     QTAILQ_HEAD(subregions, MemoryRegion) subregions;
     QTAILQ_ENTRY(MemoryRegion) subregions_link;
diff --git a/memory.c b/memory.c
index 04dc933..834959b 100644
--- a/memory.c
+++ b/memory.c
@@ -955,6 +955,22 @@ static Object *memory_region_resolve_container(Object *obj, void *opaque,
     return OBJECT(mr->container);
 }
 
+static void memory_region_get_priority(Object *obj, Visitor *v, void *opaque,
+                                       const char *name, Error **errp)
+{
+    MemoryRegion *mr = MEMORY_REGION(obj);
+    int32_t value = mr->priority;
+
+    visit_type_int32(v, &value, name, errp);
+}
+
+static bool memory_region_get_may_overlap(Object *obj, Error **errp)
+{
+    MemoryRegion *mr = MEMORY_REGION(obj);
+
+    return mr->may_overlap;
+}
+
 static void memory_region_initfn(Object *obj)
 {
     MemoryRegion *mr = MEMORY_REGION(obj);
@@ -978,6 +994,14 @@ static void memory_region_initfn(Object *obj)
                         memory_region_get_addr,
                         NULL, /* memory_region_set_addr */
                         NULL, NULL, &error_abort);
+    object_property_add(OBJECT(mr), "priority", "uint32",
+                        memory_region_get_priority,
+                        NULL, /* memory_region_set_priority */
+                        NULL, NULL, &error_abort);
+    object_property_add_bool(OBJECT(mr), "may-overlap",
+                             memory_region_get_may_overlap,
+                             NULL, /* memory_region_set_may_overlap */
+                             &error_abort);
 }
 
 static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH for 2.1 13/14] memory: MemoryRegion: Add size property
  2014-06-25 12:25 [Qemu-devel] [PATCH for 2.1 00/14] My pending qom and memory QOMification patches Paolo Bonzini
                   ` (11 preceding siblings ...)
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 12/14] memory: MemoryRegion: Add may-overlap and priority props Paolo Bonzini
@ 2014-06-25 12:25 ` Paolo Bonzini
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 14/14] memory: do not give a name to the internal exec.c regions Paolo Bonzini
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2014-06-25 12:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Crosthwaite

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

To allow devices to dynamically resize the device. The motivation is
to allow devices with variable size to init their memory_region
without size early and then correctly populate size at realize() time.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 memory.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/memory.c b/memory.c
index 834959b..64d7176 100644
--- a/memory.c
+++ b/memory.c
@@ -971,6 +971,15 @@ static bool memory_region_get_may_overlap(Object *obj, Error **errp)
     return mr->may_overlap;
 }
 
+static void memory_region_get_size(Object *obj, Visitor *v, void *opaque,
+                                   const char *name, Error **errp)
+{
+    MemoryRegion *mr = MEMORY_REGION(obj);
+    uint64_t value = memory_region_size(mr);
+
+    visit_type_uint64(v, &value, name, errp);
+}
+
 static void memory_region_initfn(Object *obj)
 {
     MemoryRegion *mr = MEMORY_REGION(obj);
@@ -1002,6 +1011,10 @@ static void memory_region_initfn(Object *obj)
                              memory_region_get_may_overlap,
                              NULL, /* memory_region_set_may_overlap */
                              &error_abort);
+    object_property_add(OBJECT(mr), "size", "uint64",
+                        memory_region_get_size,
+                        NULL, /* memory_region_set_size, */
+                        NULL, NULL, &error_abort);
 }
 
 static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PATCH for 2.1 14/14] memory: do not give a name to the internal exec.c regions
  2014-06-25 12:25 [Qemu-devel] [PATCH for 2.1 00/14] My pending qom and memory QOMification patches Paolo Bonzini
                   ` (12 preceding siblings ...)
  2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 13/14] memory: MemoryRegion: Add size property Paolo Bonzini
@ 2014-06-25 12:25 ` Paolo Bonzini
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2014-06-25 12:25 UTC (permalink / raw)
  To: qemu-devel

There is no need to have them visible under /machine.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/exec.c b/exec.c
index 6e2c4f0..ef57519 100644
--- a/exec.c
+++ b/exec.c
@@ -1794,13 +1794,13 @@ MemoryRegion *iotlb_to_region(AddressSpace *as, hwaddr index)
 
 static void io_mem_init(void)
 {
-    memory_region_init_io(&io_mem_rom, NULL, &unassigned_mem_ops, NULL, "rom", UINT64_MAX);
+    memory_region_init_io(&io_mem_rom, NULL, &unassigned_mem_ops, NULL, NULL, UINT64_MAX);
     memory_region_init_io(&io_mem_unassigned, NULL, &unassigned_mem_ops, NULL,
-                          "unassigned", UINT64_MAX);
+                          NULL, UINT64_MAX);
     memory_region_init_io(&io_mem_notdirty, NULL, &notdirty_mem_ops, NULL,
-                          "notdirty", UINT64_MAX);
+                          NULL, UINT64_MAX);
     memory_region_init_io(&io_mem_watch, NULL, &watch_mem_ops, NULL,
-                          "watch", UINT64_MAX);
+                          NULL, UINT64_MAX);
 }
 
 static void mem_begin(MemoryListener *listener)
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2014-06-25 12:26 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-25 12:25 [Qemu-devel] [PATCH for 2.1 00/14] My pending qom and memory QOMification patches Paolo Bonzini
2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 01/14] qom: add object_property_add_alias() Paolo Bonzini
2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 02/14] qom: add a generic mechanism to resolve paths Paolo Bonzini
2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 03/14] qom: allow creating an alias of a child<> property Paolo Bonzini
2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 04/14] mc146818rtc: add "rtc-time" link to "/machine/rtc" Paolo Bonzini
2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 05/14] qom: object: remove parent pointer when unparenting Paolo Bonzini
2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 06/14] qom: object: Ignore refs/unrefs of NULL Paolo Bonzini
2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 07/14] libqtest: escape strings in QMP commands, fix leak Paolo Bonzini
2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 08/14] memory: MemoryRegion: use /machine as default owner Paolo Bonzini
2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 09/14] memory: MemoryRegion: QOMify Paolo Bonzini
2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 10/14] memory: MemoryRegion: replace owner field with QOM parent Paolo Bonzini
2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 11/14] memory: MemoryRegion: Add container and addr props Paolo Bonzini
2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 12/14] memory: MemoryRegion: Add may-overlap and priority props Paolo Bonzini
2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 13/14] memory: MemoryRegion: Add size property Paolo Bonzini
2014-06-25 12:25 ` [Qemu-devel] [PATCH for 2.1 14/14] memory: do not give a name to the internal exec.c regions Paolo Bonzini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.