qom: misc fixes & enhancements to support TLS work

Add object_get_objects_root() method which is a convience for obtaining the Object * located at /objects in the object composition tree. Convert existing code over to use the new API where appropriate.

Signed-off-by Daniel P. Berrange <[email protected]>

---
 include/qom/object.h | 12 ++++++++++++
 iothread.c           |  4 +---
 numa.c               |  2 +-
 qmp.c                |  6 +++---
 qom/object.c         |  5 +++++
 5 files changed, 22 insertions(+), 7 deletions(-)

qom/object.c +31 -0

+Object *object_new_with_propv(const char *typename,
+                              Object *parent,
+                              const char *id,
+                              Error **errp,
+                              va_list vargs)
+{
+    Object *obj;
+    ObjectClass *klass;
+    Error *local_err = NULL;
+
+    klass = object_class_by_name(typename);
+    if (!klass) {
+        error_setg(errp, "invalid object type: %s", typename);
+        return NULL;
+    }
+
+    if (object_class_is_abstract(klass)) {
+        error_setg(errp, "object type '%s' is abstract", typename);
+        return NULL;
+    }
+    obj = object_new(typename);
+
+    if (object_set_propv(obj, &local_err, vargs) < 0) {
+        goto error;
+    }
+
+    object_property_add_child(parent, id, obj, &local_err);
+    if (local_err) {
+        goto error;
+    }
+

Am 13.05.2015 um 18:14 schrieb Daniel P. Berrange:

Quoted 37 lines

qom/object.c +37 -0

+Object *object_new_with_propv(const char *typename,
+                              Object *parent,
+                              const char *id,
+                              Error **errp,
+                              va_list vargs)
+{
+    Object *obj;
+    ObjectClass *klass;
+    Error *local_err = NULL;
+
+    klass = object_class_by_name(typename);
+    if (!klass) {
+        error_setg(errp, "invalid object type: %s", typename);
+        return NULL;
+    }
+
+    if (object_class_is_abstract(klass)) {
+        error_setg(errp, "object type '%s' is abstract", typename);
+        return NULL;
+    }
+    obj = object_new(typename);
+
+    if (object_set_propv(obj, &local_err, vargs) < 0) {
+        goto error;
+    }
+
+    object_property_add_child(parent, id, obj, &local_err);
+    if (local_err) {
+        goto error;
+    }
+
+    if (object_dynamic_cast(obj, TYPE_USER_CREATABLE)) {
+        user_creatable_complete(obj, &local_err);
+        if (local_err) {
+            object_unparent(obj);
+            goto error;
+        }

qom/object.c Object *object_new(const char *typename) · line 501

Quoted 4 lines

qom/object.c +4 -0

+    }
+
+    object_unref(OBJECT(obj));
+    return obj;

This looks fishy. Either we return obj and need to retain a ref for that (caller's responsibility to unref) or we unref it and return void. Or am I misreading the code?

Quoted 38 lines

Am 13.05.2015 um 18:14 schrieb Daniel P. Berrange:

Quoted 37 lines

qom/object.c +37 -0

+Object *object_new_with_propv(const char *typename,
+                              Object *parent,
+                              const char *id,
+                              Error **errp,
+                              va_list vargs)
+{
+    Object *obj;
+    ObjectClass *klass;
+    Error *local_err = NULL;
+
+    klass = object_class_by_name(typename);
+    if (!klass) {
+        error_setg(errp, "invalid object type: %s", typename);
+        return NULL;
+    }
+
+    if (object_class_is_abstract(klass)) {
+        error_setg(errp, "object type '%s' is abstract", typename);
+        return NULL;
+    }
+    obj = object_new(typename);
+
+    if (object_set_propv(obj, &local_err, vargs) < 0) {
+        goto error;
+    }
+
+    object_property_add_child(parent, id, obj, &local_err);
+    if (local_err) {
+        goto error;
+    }
+
+    if (object_dynamic_cast(obj, TYPE_USER_CREATABLE)) {
+        user_creatable_complete(obj, &local_err);
+        if (local_err) {
+            object_unparent(obj);
+            goto error;
+        }

Quoted 8 lines

qom/object.c Object *object_new(const char *typename) · line 501

Quoted 4 lines

qom/object.c +4 -0

+    }
+
+    object_unref(OBJECT(obj));
+    return obj;

This looks fishy. Either we return obj and need to retain a ref for that (caller's responsibility to unref) or we unref it and return void. Or am I misreading the code?

Paolo told me on previous posting that object_property_add_child() holds a reference on 'obj' for as long as it is registered in the object hierarchy composition. So it sufficient to rely on that long term reference, and let the caller dispose of the object by calling object_unparent(obj) when finally done.

Paolo told me on previous posting that object_property_add_child() holds a reference on 'obj' for as long as it is registered in the object hierarchy composition. So it sufficient to rely on that long term reference, and let the caller dispose of the object by calling object_unparent(obj) when finally done.

Quoted 5 lines · Daniel P. Berrange

Paolo told me on previous posting that object_property_add_child() holds a reference on 'obj' for as long as it is registered in the object hierarchy composition. So it sufficient to rely on that long term reference, and let the caller dispose of the object by calling object_unparent(obj) when finally done.

For an example of the same pattern:

DeviceState *qdev_try_create(BusState *bus, const char *type)

{
    DeviceState *dev;

    if (object_class_by_name(type) == NULL) {
        return NULL;
    }
    dev = DEVICE(object_new(type));
    if (!dev) {
        return NULL;
    }

    if (!bus) {
        bus = sysbus_get_default();
    }

    qdev_set_parent_bus(dev, bus);
    object_unref(OBJECT(dev));
    return dev;
}

Effectively this is idea as GObject's "floating reference". qdev_set_parent_bus (in qdev_try_create) and object_property_add_child (in Daniel's patches) "sink" the floating reference by doing object_unref. If we had floating references, the object would be returned to the caller unref'ed anyway.

Of course, the reference can go away via QMP. But that will only happen after the caller would have called object_unref itself.

Quoted 30 lines · Paolo Bonzini

Quoted 5 lines · Daniel P. Berrange

Paolo told me on previous posting that object_property_add_child() holds a reference on 'obj' for as long as it is registered in the object hierarchy composition. So it sufficient to rely on that long term reference, and let the caller dispose of the object by calling object_unparent(obj) when finally done.

For an example of the same pattern:

DeviceState *qdev_try_create(BusState *bus, const char *type)

{
    DeviceState *dev;

    if (object_class_by_name(type) == NULL) {
        return NULL;
    }
    dev = DEVICE(object_new(type));
    if (!dev) {
        return NULL;
    }

    if (!bus) {
        bus = sysbus_get_default();
    }

    qdev_set_parent_bus(dev, bus);
    object_unref(OBJECT(dev));
    return dev;
}

Quoted 5 lines

Effectively this is idea as GObject's "floating reference". qdev_set_parent_bus (in qdev_try_create) and object_property_add_child (in Daniel's patches) "sink" the floating reference by doing object_unref. If we had floating references, the object would be returned to the caller unref'ed anyway.

I was agreeing with Andreas at first (because it would make the reference ownership rules simpler and easier to understand), until I noticed that every call of qdev_try_create() and object_resolve_path() in the code would need an additional object_unref() call if we didn't use this pattern.

But it bothers me that this exceptional behavior is not documented on neither qdev_try_create() or object_resolve_path().

Quoted 1 line

Quoted 2 lines

Of course, the reference can go away via QMP. But that will only happen after the caller would have called object_unref itself.

But the caller won't ever call object_unref() because it doesn't own any reference, right? In this case, can we clarify the rules about how long can callers safely expect the object to stay around? Can the object be disposed in another thread? Can it be disposed only when some specific events happen?

Quoted 47 lines · Paolo Bonzini

Quoted 30 lines · Daniel P. Berrange

Quoted 5 lines

Paolo told me on previous posting that object_property_add_child() holds a reference on 'obj' for as long as it is registered in the object hierarchy composition. So it sufficient to rely on that long term reference, and let the caller dispose of the object by calling object_unparent(obj) when finally done.

For an example of the same pattern:

DeviceState *qdev_try_create(BusState *bus, const char *type)

{
    DeviceState *dev;

    if (object_class_by_name(type) == NULL) {
        return NULL;
    }
    dev = DEVICE(object_new(type));
    if (!dev) {
        return NULL;
    }

    if (!bus) {
        bus = sysbus_get_default();
    }

    qdev_set_parent_bus(dev, bus);
    object_unref(OBJECT(dev));
    return dev;
}

Quoted 5 lines

Effectively this is idea as GObject's "floating reference". qdev_set_parent_bus (in qdev_try_create) and object_property_add_child (in Daniel's patches) "sink" the floating reference by doing object_unref. If we had floating references, the object would be returned to the caller unref'ed anyway.

I was agreeing with Andreas at first (because it would make the reference ownership rules simpler and easier to understand), until I noticed that every call of qdev_try_create() and object_resolve_path() in the code would need an additional object_unref() call if we didn't use this pattern.

But it bothers me that this exceptional behavior is not documented on neither qdev_try_create() or object_resolve_path().

Quoted 1 line

Quoted 8 lines

Quoted 2 lines

Of course, the reference can go away via QMP. But that will only happen after the caller would have called object_unref itself.

But the caller won't ever call object_unref() because it doesn't own any reference, right? In this case, can we clarify the rules about how long can callers safely expect the object to stay around? Can the object be disposed in another thread? Can it be disposed only when some specific events happen?

In the inline docs for object_new_with_props I wrote

* The returned object will have one stable reference maintained
* for as long as it is present in the object hierarchy.

We could expand it to explicitly say that 'object_unparent' is required to remove the object from the hierarchy and free it.

Quoted 47 lines · Eduardo Habkost

Quoted 30 lines · Paolo Bonzini

Quoted 5 lines · Daniel P. Berrange

Paolo told me on previous posting that object_property_add_child() holds a reference on 'obj' for as long as it is registered in the object hierarchy composition. So it sufficient to rely on that long term reference, and let the caller dispose of the object by calling object_unparent(obj) when finally done.

For an example of the same pattern:

DeviceState *qdev_try_create(BusState *bus, const char *type)

{
    DeviceState *dev;

    if (object_class_by_name(type) == NULL) {
        return NULL;
    }
    dev = DEVICE(object_new(type));
    if (!dev) {
        return NULL;
    }

    if (!bus) {
        bus = sysbus_get_default();
    }

    qdev_set_parent_bus(dev, bus);
    object_unref(OBJECT(dev));
    return dev;
}

Quoted 5 lines

Effectively this is idea as GObject's "floating reference". qdev_set_parent_bus (in qdev_try_create) and object_property_add_child (in Daniel's patches) "sink" the floating reference by doing object_unref. If we had floating references, the object would be returned to the caller unref'ed anyway.

I was agreeing with Andreas at first (because it would make the reference ownership rules simpler and easier to understand), until I noticed that every call of qdev_try_create() and object_resolve_path() in the code would need an additional object_unref() call if we didn't use this pattern.

But it bothers me that this exceptional behavior is not documented on neither qdev_try_create() or object_resolve_path().

Quoted 1 line

Quoted 8 lines

Quoted 2 lines

Of course, the reference can go away via QMP. But that will only happen after the caller would have called object_unref itself.

But the caller won't ever call object_unref() because it doesn't own any reference, right? In this case, can we clarify the rules about how long can callers safely expect the object to stay around? Can the object be disposed in another thread? Can it be disposed only when some specific events happen?

In the inline docs for object_new_with_props I wrote

* The returned object will have one stable reference maintained
* for as long as it is present in the object hierarchy.

We could expand it to explicitly say that 'object_unparent' is required to remove the object from the hierarchy and free it.

Quoted 62 lines · Daniel P. Berrange

Quoted 47 lines · Eduardo Habkost

Quoted 30 lines · Paolo Bonzini

Quoted 5 lines · Daniel P. Berrange

Paolo told me on previous posting that object_property_add_child() holds a reference on 'obj' for as long as it is registered in the object hierarchy composition. So it sufficient to rely on that long term reference, and let the caller dispose of the object by calling object_unparent(obj) when finally done.

For an example of the same pattern:

DeviceState *qdev_try_create(BusState *bus, const char *type)

{
    DeviceState *dev;

    if (object_class_by_name(type) == NULL) {
        return NULL;
    }
    dev = DEVICE(object_new(type));
    if (!dev) {
        return NULL;
    }

    if (!bus) {
        bus = sysbus_get_default();
    }

    qdev_set_parent_bus(dev, bus);
    object_unref(OBJECT(dev));
    return dev;
}

Quoted 5 lines

Effectively this is idea as GObject's "floating reference". qdev_set_parent_bus (in qdev_try_create) and object_property_add_child (in Daniel's patches) "sink" the floating reference by doing object_unref. If we had floating references, the object would be returned to the caller unref'ed anyway.

I was agreeing with Andreas at first (because it would make the reference ownership rules simpler and easier to understand), until I noticed that every call of qdev_try_create() and object_resolve_path() in the code would need an additional object_unref() call if we didn't use this pattern.

But it bothers me that this exceptional behavior is not documented on neither qdev_try_create() or object_resolve_path().

Quoted 1 line

Quoted 8 lines

Quoted 2 lines

Of course, the reference can go away via QMP. But that will only happen after the caller would have called object_unref itself.

But the caller won't ever call object_unref() because it doesn't own any reference, right? In this case, can we clarify the rules about how long can callers safely expect the object to stay around? Can the object be disposed in another thread? Can it be disposed only when some specific events happen?

In the inline docs for object_new_with_props I wrote

* The returned object will have one stable reference maintained
* for as long as it is present in the object hierarchy.

Quoted 2 lines

We could expand it to explicitly say that 'object_unparent' is required to remove the object from the hierarchy and free it.

What's missing to me is some clarification on how long it is safe to assume that the object won't be removed from the hierarchy by other code.

Quoted 62 lines · Eduardo Habkost

Quoted 47 lines · Paolo Bonzini

Quoted 30 lines · Daniel P. Berrange

Quoted 5 lines

Paolo told me on previous posting that object_property_add_child() holds a reference on 'obj' for as long as it is registered in the object hierarchy composition. So it sufficient to rely on that long term reference, and let the caller dispose of the object by calling object_unparent(obj) when finally done.

For an example of the same pattern:

DeviceState *qdev_try_create(BusState *bus, const char *type)

{
    DeviceState *dev;

    if (object_class_by_name(type) == NULL) {
        return NULL;
    }
    dev = DEVICE(object_new(type));
    if (!dev) {
        return NULL;
    }

    if (!bus) {
        bus = sysbus_get_default();
    }

    qdev_set_parent_bus(dev, bus);
    object_unref(OBJECT(dev));
    return dev;
}

Quoted 5 lines

Effectively this is idea as GObject's "floating reference". qdev_set_parent_bus (in qdev_try_create) and object_property_add_child (in Daniel's patches) "sink" the floating reference by doing object_unref. If we had floating references, the object would be returned to the caller unref'ed anyway.

I was agreeing with Andreas at first (because it would make the reference ownership rules simpler and easier to understand), until I noticed that every call of qdev_try_create() and object_resolve_path() in the code would need an additional object_unref() call if we didn't use this pattern.

But it bothers me that this exceptional behavior is not documented on neither qdev_try_create() or object_resolve_path().

Quoted 1 line

Quoted 8 lines

Quoted 2 lines

Of course, the reference can go away via QMP. But that will only happen after the caller would have called object_unref itself.

But the caller won't ever call object_unref() because it doesn't own any reference, right? In this case, can we clarify the rules about how long can callers safely expect the object to stay around? Can the object be disposed in another thread? Can it be disposed only when some specific events happen?

In the inline docs for object_new_with_props I wrote

* The returned object will have one stable reference maintained
* for as long as it is present in the object hierarchy.

Quoted 2 lines

We could expand it to explicitly say that 'object_unparent' is required to remove the object from the hierarchy and free it.

What's missing to me is some clarification on how long it is safe to assume that the object won't be removed from the hierarchy by other code.

Am 20.05.2015 um 18:06 schrieb Eduardo Habkost:

Quoted 66 lines · Daniel P. Berrange

Quoted 64 lines · Eduardo Habkost

Quoted 47 lines · Paolo Bonzini

Quoted 30 lines · Daniel P. Berrange

Quoted 5 lines

Paolo told me on previous posting that object_property_add_child() holds a reference on 'obj' for as long as it is registered in the object hierarchy composition. So it sufficient to rely on that long term reference, and let the caller dispose of the object by calling object_unparent(obj) when finally done.

For an example of the same pattern:

DeviceState *qdev_try_create(BusState *bus, const char *type)

{
    DeviceState *dev;

    if (object_class_by_name(type) == NULL) {
        return NULL;
    }
    dev = DEVICE(object_new(type));
    if (!dev) {
        return NULL;
    }

    if (!bus) {
        bus = sysbus_get_default();
    }

    qdev_set_parent_bus(dev, bus);
    object_unref(OBJECT(dev));
    return dev;
}

Quoted 5 lines

Effectively this is idea as GObject's "floating reference". qdev_set_parent_bus (in qdev_try_create) and object_property_add_child (in Daniel's patches) "sink" the floating reference by doing object_unref. If we had floating references, the object would be returned to the caller unref'ed anyway.

I was agreeing with Andreas at first (because it would make the reference ownership rules simpler and easier to understand), until I noticed that every call of qdev_try_create() and object_resolve_path() in the code would need an additional object_unref() call if we didn't use this pattern.

But it bothers me that this exceptional behavior is not documented on neither qdev_try_create() or object_resolve_path().

Quoted 1 line

Quoted 8 lines

Quoted 2 lines

Of course, the reference can go away via QMP. But that will only happen after the caller would have called object_unref itself.

But the caller won't ever call object_unref() because it doesn't own any reference, right? In this case, can we clarify the rules about how long can callers safely expect the object to stay around? Can the object be disposed in another thread? Can it be disposed only when some specific events happen?

In the inline docs for object_new_with_props I wrote

* The returned object will have one stable reference maintained
* for as long as it is present in the object hierarchy.

We could expand it to explicitly say that 'object_unparent' is required to remove the object from the hierarchy and free it.

Quoted 3 lines

What's missing to me is some clarification on how long it is safe to assume that the object won't be removed from the hierarchy by other code.

There is no guarantee. Therefore any caller of object_resolve_path() must ref and unref as needed if doing more than a single operation, such as setting a link target.

Quoted 71 lines

Am 20.05.2015 um 18:06 schrieb Eduardo Habkost:

Quoted 69 lines · Daniel P. Berrange

Quoted 57 lines · Eduardo Habkost

Quoted 47 lines · Paolo Bonzini

Quoted 30 lines · Daniel P. Berrange

Quoted 5 lines

Paolo told me on previous posting that object_property_add_child() holds a reference on 'obj' for as long as it is registered in the object hierarchy composition. So it sufficient to rely on that long term reference, and let the caller dispose of the object by calling object_unparent(obj) when finally done.

For an example of the same pattern:

DeviceState *qdev_try_create(BusState *bus, const char *type)

{
    DeviceState *dev;

    if (object_class_by_name(type) == NULL) {
        return NULL;
    }
    dev = DEVICE(object_new(type));
    if (!dev) {
        return NULL;
    }

    if (!bus) {
        bus = sysbus_get_default();
    }

    qdev_set_parent_bus(dev, bus);
    object_unref(OBJECT(dev));
    return dev;
}

Quoted 5 lines

Effectively this is idea as GObject's "floating reference". qdev_set_parent_bus (in qdev_try_create) and object_property_add_child (in Daniel's patches) "sink" the floating reference by doing object_unref. If we had floating references, the object would be returned to the caller unref'ed anyway.

I was agreeing with Andreas at first (because it would make the reference ownership rules simpler and easier to understand), until I noticed that every call of qdev_try_create() and object_resolve_path() in the code would need an additional object_unref() call if we didn't use this pattern.changes

But it bothers me that this exceptional behavior is not documented on neither qdev_try_create() or object_resolve_path().

Quoted 1 line

Quoted 8 lines

Quoted 2 lines

Of course, the reference can go away via QMP. But that will only happen after the caller would have called object_unref itself.

But the caller won't ever call object_unref() because it doesn't own any reference, right? In this case, can we clarify the rules about how long can callers safely expect the object to stay around? Can the object be disposed in another thread? Can it be disposed only when some specific events happen?

Quoted 7 lines

In the inline docs for object_new_with_props I wrote

* The returned object will have one stable reference maintained
* for as long as it is present in the object hierarchy.

We could expand it to explicitly say that 'object_unparent' is required to remove the object from the hierarchy and free it.

What's missing to me is some clarification on how long it is safe to assume that the object won't be removed from the hierarchy by other code.

Quoted 3 lines

There is no guarantee. Therefore any caller of object_resolve_path() must ref and unref as needed if doing more than a single operation, such as setting a link target.

So, does that mean zero guarantee, or guarantee for "a single operation"? It can't be both.

(And how exactly "a single operation" should be defined?)

Am 20.05.2015 um 18:22 schrieb Eduardo Habkost:

Quoted 79 lines · Andreas Färber

Quoted 67 lines

Am 20.05.2015 um 18:06 schrieb Eduardo Habkost:

Quoted 66 lines · Daniel P. Berrange

Quoted 64 lines · Eduardo Habkost

Quoted 47 lines · Paolo Bonzini

Quoted 30 lines · Daniel P. Berrange

Quoted 5 lines

Paolo told me on previous posting that object_property_add_child() holds a reference on 'obj' for as long as it is registered in the object hierarchy composition. So it sufficient to rely on that long term reference, and let the caller dispose of the object by calling object_unparent(obj) when finally done.

For an example of the same pattern:

DeviceState *qdev_try_create(BusState *bus, const char *type)

{
    DeviceState *dev;

    if (object_class_by_name(type) == NULL) {
        return NULL;
    }
    dev = DEVICE(object_new(type));
    if (!dev) {
        return NULL;
    }

    if (!bus) {
        bus = sysbus_get_default();
    }

    qdev_set_parent_bus(dev, bus);
    object_unref(OBJECT(dev));
    return dev;
}

Quoted 5 lines

Effectively this is idea as GObject's "floating reference". qdev_set_parent_bus (in qdev_try_create) and object_property_add_child (in Daniel's patches) "sink" the floating reference by doing object_unref. If we had floating references, the object would be returned to the caller unref'ed anyway.

I was agreeing with Andreas at first (because it would make the reference ownership rules simpler and easier to understand), until I noticed that every call of qdev_try_create() and object_resolve_path() in the code would need an additional object_unref() call if we didn't use this pattern.changes

But it bothers me that this exceptional behavior is not documented on neither qdev_try_create() or object_resolve_path().

Quoted 1 line

Quoted 8 lines

Quoted 2 lines

Of course, the reference can go away via QMP. But that will only happen after the caller would have called object_unref itself.

But the caller won't ever call object_unref() because it doesn't own any reference, right? In this case, can we clarify the rules about how long can callers safely expect the object to stay around? Can the object be disposed in another thread? Can it be disposed only when some specific events happen?

In the inline docs for object_new_with_props I wrote

* The returned object will have one stable reference maintained
* for as long as it is present in the object hierarchy.

We could expand it to explicitly say that 'object_unparent' is required to remove the object from the hierarchy and free it.

Quoted 7 lines

Quoted 3 lines

What's missing to me is some clarification on how long it is safe to assume that the object won't be removed from the hierarchy by other code.

There is no guarantee. Therefore any caller of object_resolve_path() must ref and unref as needed if doing more than a single operation, such as setting a link target.

So, does that mean zero guarantee, or guarantee for "a single operation"? It can't be both.

Quoted 1 line

(And how exactly "a single operation" should be defined?)

I see your point, after all object_ref() is a single instruction, too.

We could of course change it to return a ref'ed object, but that would require a lot of code review and probably unrefs. Mostly for objects that don't go away, but still.