Make qemu-img/qemu-nbd/qemu-io CLI more flexible

The QMP monitor code has two helper methods object_add and qmp_object_del that are called from several places in the code (QMP, HMP and main emulator startup).

The HMP and main emulator startup code also share further logic that extracts the qom-type & id values from a qdict.

We soon need to use this logic from qemu-img, qemu-io and qemu-nbd too, but don't want those to depend on the monitor, nor do we want to duplicate the code.

To avoid this, move some code out of qmp.c and hmp.c adding new methods to qom/object_interfaces.c

- user_creatable_add - takes a QDict holding a full object definition & instantiates it
- user_creatable_add_type - takes an ID, type name, and QDict holding object properties & instantiates it
- user_creatable_add_opts - takes a QemuOpts holding a full object definition & instantiates it
- user_creatable_add_opts_foreach - variant on user_creatable_add_opts which can be directly used in conjunction with qemu_opts_foreach.
- user_creatable_del - takes an ID and deletes the corresponding object

The existing code is updated to use these new methods.

Reviewed-by Eric Blake <[email protected]>

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

---
 hmp.c                           |  52 +++---------
 include/monitor/monitor.h       |   3 -
 include/qom/object_interfaces.h |  92 +++++++++++++++++++++
 qmp.c                           |  76 ++----------------
 qom/object_interfaces.c         | 174 ++++++++++++++++++++++++++++++++++++++++
 vl.c                            |  68 ++--------------
 6 files changed, 290 insertions(+), 175 deletions(-)

hmp.c +11 -41

index c6419da..41fb9ca 100644
@@ -30,6 +30,7 @@
 #include "qapi/string-output-visitor.h"
 #include "qapi/util.h"
 #include "qapi-visit.h"
+#include "qom/object_interfaces.h"
 #include "ui/console.h"
 #include "block/qapi.h"
 #include "qemu-io.h"

@@ -1655,58 +1656,27 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
 void hmp_object_add(Monitor *mon, const QDict *qdict)
 {
     Error *err = NULL;
-    Error *err_end = NULL;
     QemuOpts *opts;
-    char *type = NULL;
-    char *id = NULL;
     OptsVisitor *ov;
-    QDict *pdict;
-    Visitor *v;
+    Object *obj = NULL;
 
     opts = qemu_opts_from_qdict(qemu_find_opts("object"), qdict, &err);
     if (err) {
-        goto out;
+        hmp_handle_error(mon, &err);
+        return;
     }
 
     ov = opts_visitor_new(opts);
-    pdict = qdict_clone_shallow(qdict);
-    v = opts_get_visitor(ov);
-
-    visit_start_struct(v, NULL, NULL, 0, &err);
-    if (err) {
-        goto out_clean;
-    }
-
-    qdict_del(pdict, "qom-type");
-    visit_type_str(v, "qom-type", &type, &err);
-    if (err) {
-        goto out_end;
-    }
+    obj = user_creatable_add(qdict, opts_get_visitor(ov), &err);
+    opts_visitor_cleanup(ov);
+    qemu_opts_del(opts);
 
-    qdict_del(pdict, "id");
-    visit_type_str(v, "id", &id, &err);
     if (err) {
-        goto out_end;
+        hmp_handle_error(mon, &err);
     }
-
-    object_add(type, id, pdict, v, &err);
-
-out_end:
-    visit_end_struct(v, &err_end);
-    if (!err && err_end) {
-        qmp_object_del(id, NULL);
+    if (obj) {
+        object_unref(obj);
     }
-    error_propagate(&err, err_end);
-out_clean:
-    opts_visitor_cleanup(ov);
-
-    QDECREF(pdict);
-    qemu_opts_del(opts);
-    g_free(id);
-    g_free(type);
-
-out:
-    hmp_handle_error(mon, &err);
 }
 
 void hmp_getfd(Monitor *mon, const QDict *qdict)

@@ -1934,7 +1904,7 @@ void hmp_object_del(Monitor *mon, const QDict *qdict)
     const char *id = qdict_get_str(qdict, "id");
     Error *err = NULL;
 
-    qmp_object_del(id, &err);
+    user_creatable_del(id, &err);
     hmp_handle_error(mon, &err);
 }
 

include/monitor/monitor.h +0 -3

index 91b95ae..aa0f373 100644
@@ -43,9 +43,6 @@ void monitor_read_command(Monitor *mon, int show_prompt);
 int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
                           void *opaque);
 
-void object_add(const char *type, const char *id, const QDict *qdict,
-                Visitor *v, Error **errp);
-
 AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
                                 bool has_opaque, const char *opaque,
                                 Error **errp);

include/qom/object_interfaces.h +92 -0

index 283ae0d..d579746 100644
@@ -2,6 +2,8 @@
 #define OBJECT_INTERFACES_H
 
 #include "qom/object.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/visitor.h"
 
 #define TYPE_USER_CREATABLE "user-creatable"
 

@@ -72,4 +74,94 @@ void user_creatable_complete(Object *obj, Error **errp);
  * from implements USER_CREATABLE interface.
  */
 bool user_creatable_can_be_deleted(UserCreatable *uc, Error **errp);
+
+/**
+ * user_creatable_add:
+ * @qdict: the object definition
+ * @v: the visitor
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * Create an instance of the user creatable object whose type
+ * is defined in @qdict by the 'qom-type' field, placing it
+ * in the object composition tree with name provided by the
+ * 'id' field. The remaining fields in @qdict are used to
+ * initialize the object properties.
+ *
+ * Returns: the newly created object or NULL on error
+ */
+Object *user_creatable_add(const QDict *qdict,
+                           Visitor *v, Error **errp);
+
+/**
+ * user_creatable_add_type:
+ * @type: the object type name
+ * @id: the unique ID for the object
+ * @qdict: the object properties
+ * @v: the visitor
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * Create an instance of the user creatable object @type, placing
+ * it in the object composition tree with name @id, initializing
+ * it with properties from @qdict
+ *
+ * Returns: the newly created object or NULL on error
+ */
+Object *user_creatable_add_type(const char *type, const char *id,
+                                const QDict *qdict,
+                                Visitor *v, Error **errp);
+
+/**
+ * user_creatable_add_opts:
+ * @opts: the object definition
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * Create an instance of the user creatable object whose type
+ * is defined in @opts by the 'qom-type' option, placing it
+ * in the object composition tree with name provided by the
+ * 'id' field. The remaining options in @opts are used to
+ * initialize the object properties.
+ *
+ * Returns: the newly created object or NULL on error
+ */
+Object *user_creatable_add_opts(QemuOpts *opts, Error **errp);
+
+
+/**
+ * user_creatable_add_opts_predicate:
+ * @type: the QOM type to be added
+ *
+ * A callback function to determine whether an object
+ * of type @type should be created. Instances of this
+ * callback should be passed to user_creatable_add_opts_foreach
+ */
+typedef bool (*user_creatable_add_opts_predicate)(const char *type);
+
+/**
+ * user_creatable_add_opts_foreach:
+ * @opaque: a user_creatable_add_opts_predicate callback or NULL
+ * @opts: options to create
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * An iterator callback to be used in conjunction with
+ * the qemu_opts_foreach() method for creating a list of
+ * objects from a set of QemuOpts
+ *
+ * The @opaque parameter can be passed a user_creatable_add_opts_predicate
+ * callback to filter which types of object are created during iteration.
+ *
+ * Returns: 0 on success, -1 on error
+ */
+int user_creatable_add_opts_foreach(void *opaque,
+                                    QemuOpts *opts, Error **errp);
+
+/**
+ * user_creatable_del:
+ * @id: the unique ID for the object
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * Delete an instance of the user creatable object identified
+ * by @id.
+ */
+void user_creatable_del(const char *id, Error **errp);
+
 #endif

qmp.c +7 -69

index 6ae7230..9a93d5e 100644
@@ -633,65 +633,13 @@ void qmp_add_client(const char *protocol, const char *fdname,
     close(fd);
 }
 
-void object_add(const char *type, const char *id, const QDict *qdict,
-                Visitor *v, Error **errp)
-{
-    Object *obj;
-    ObjectClass *klass;
-    const QDictEntry *e;
-    Error *local_err = NULL;
-
-    klass = object_class_by_name(type);
-    if (!klass) {
-        error_setg(errp, "invalid object type: %s", type);
-        return;
-    }
-
-    if (!object_class_dynamic_cast(klass, TYPE_USER_CREATABLE)) {
-        error_setg(errp, "object type '%s' isn't supported by object-add",
-                   type);
-        return;
-    }
-
-    if (object_class_is_abstract(klass)) {
-        error_setg(errp, "object type '%s' is abstract", type);
-        return;
-    }
-
-    obj = object_new(type);
-    if (qdict) {
-        for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
-            object_property_set(obj, v, e->key, &local_err);
-            if (local_err) {
-                goto out;
-            }
-        }
-    }
-
-    object_property_add_child(object_get_objects_root(),
-                              id, obj, &local_err);
-    if (local_err) {
-        goto out;
-    }
-
-    user_creatable_complete(obj, &local_err);
-    if (local_err) {
-        object_property_del(object_get_objects_root(),
-                            id, &error_abort);
-        goto out;
-    }
-out:
-    if (local_err) {
-        error_propagate(errp, local_err);
-    }
-    object_unref(obj);
-}
 
 void qmp_object_add(const char *type, const char *id,
                     bool has_props, QObject *props, Error **errp)
 {
     const QDict *pdict = NULL;
     QmpInputVisitor *qiv;
+    Object *obj;
 
     if (props) {
         pdict = qobject_to_qdict(props);

@@ -702,27 +650,17 @@ void qmp_object_add(const char *type, const char *id,
     }
 
     qiv = qmp_input_visitor_new(props);
-    object_add(type, id, pdict, qmp_input_get_visitor(qiv), errp);
+    obj = user_creatable_add_type(type, id, pdict,
+                                  qmp_input_get_visitor(qiv), errp);
     qmp_input_visitor_cleanup(qiv);
+    if (obj) {
+        object_unref(obj);
+    }
 }
 
 void qmp_object_del(const char *id, Error **errp)
 {
-    Object *container;
-    Object *obj;
-
-    container = object_get_objects_root();
-    obj = object_resolve_path_component(container, id);
-    if (!obj) {
-        error_setg(errp, "object id not found");
-        return;
-    }
-
-    if (!user_creatable_can_be_deleted(USER_CREATABLE(obj), errp)) {
-        error_setg(errp, "%s is in use, can not be deleted", id);
-        return;
-    }
-    object_unparent(obj);
+    user_creatable_del(id, errp);
 }
 
 MemoryDeviceInfoList *qmp_query_memory_devices(Error **errp)

qom/object_interfaces.c +174 -0

index f1218f0..c2f6e29 100644
@@ -1,6 +1,9 @@
 #include "qemu/osdep.h"
 #include "qom/object_interfaces.h"
 #include "qemu/module.h"
+#include "qapi-visit.h"
+#include "qapi/qmp-output-visitor.h"
+#include "qapi/opts-visitor.h"
 
 void user_creatable_complete(Object *obj, Error **errp)
 {

@@ -31,6 +34,177 @@ bool user_creatable_can_be_deleted(UserCreatable *uc, Error **errp)
     }
 }
 
+
+Object *user_creatable_add(const QDict *qdict,
+                           Visitor *v, Error **errp)
+{
+    char *type = NULL;
+    char *id = NULL;
+    Object *obj = NULL;
+    Error *local_err = NULL, *end_err = NULL;
+    QDict *pdict;
+
+    pdict = qdict_clone_shallow(qdict);
+
+    visit_start_struct(v, NULL, NULL, 0, &local_err);
+    if (local_err) {
+        goto out;
+    }
+
+    qdict_del(pdict, "qom-type");
+    visit_type_str(v, "qom-type", &type, &local_err);
+    if (local_err) {
+        goto out_visit;
+    }
+
+    qdict_del(pdict, "id");
+    visit_type_str(v, "id", &id, &local_err);
+    if (local_err) {
+        goto out_visit;
+    }
+
+    obj = user_creatable_add_type(type, id, pdict, v, &local_err);
+    if (local_err) {
+        goto out_visit;
+    }
+
+ out_visit:
+    visit_end_struct(v, &end_err);
+    if (end_err) {
+        error_propagate(&local_err, end_err);
+        if (obj) {
+            user_creatable_del(id, NULL);
+        }
+        goto out;
+    }
+
+out:
+    QDECREF(pdict);
+    g_free(id);
+    g_free(type);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        object_unref(obj);
+        return NULL;
+    }
+    return obj;
+}
+
+
+Object *user_creatable_add_type(const char *type, const char *id,
+                                const QDict *qdict,
+                                Visitor *v, Error **errp)
+{
+    Object *obj;
+    ObjectClass *klass;
+    const QDictEntry *e;
+    Error *local_err = NULL;
+
+    klass = object_class_by_name(type);
+    if (!klass) {
+        error_setg(errp, "invalid object type: %s", type);
+        return NULL;
+    }
+
+    if (!object_class_dynamic_cast(klass, TYPE_USER_CREATABLE)) {
+        error_setg(errp, "object type '%s' isn't supported by object-add",
+                   type);
+        return NULL;
+    }
+
+    if (object_class_is_abstract(klass)) {
+        error_setg(errp, "object type '%s' is abstract", type);
+        return NULL;
+    }
+
+    obj = object_new(type);
+    if (qdict) {
+        for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
+            object_property_set(obj, v, e->key, &local_err);
+            if (local_err) {
+                goto out;
+            }
+        }
+    }
+
+    object_property_add_child(object_get_objects_root(),
+                              id, obj, &local_err);
+    if (local_err) {
+        goto out;
+    }
+
+    user_creatable_complete(obj, &local_err);
+    if (local_err) {
+        object_property_del(object_get_objects_root(),
+                            id, &error_abort);
+        goto out;
+    }
+out:
+    if (local_err) {
+        error_propagate(errp, local_err);
+        object_unref(obj);
+        return NULL;
+    }
+    return obj;
+}
+
+
+Object *user_creatable_add_opts(QemuOpts *opts, Error **errp)
+{
+    OptsVisitor *ov;
+    QDict *pdict;
+    Object *obj = NULL;
+
+    ov = opts_visitor_new(opts);
+    pdict = qemu_opts_to_qdict(opts, NULL);
+
+    obj = user_creatable_add(pdict, opts_get_visitor(ov), errp);
+    opts_visitor_cleanup(ov);
+    QDECREF(pdict);
+    return obj;
+}
+
+
+int user_creatable_add_opts_foreach(void *opaque, QemuOpts *opts, Error **errp)
+{
+    bool (*type_predicate)(const char *) = opaque;
+    Object *obj = NULL;
+    const char *type;
+
+    type = qemu_opt_get(opts, "qom-type");
+    if (type && type_predicate &&
+        !type_predicate(type)) {
+        return 0;
+    }
+
+    obj = user_creatable_add_opts(opts, errp);
+    if (!obj) {
+        return -1;
+    }
+    object_unref(obj);
+    return 0;
+}
+
+
+void user_creatable_del(const char *id, Error **errp)
+{
+    Object *container;
+    Object *obj;
+
+    container = object_get_objects_root();
+    obj = object_resolve_path_component(container, id);
+    if (!obj) {
+        error_setg(errp, "object '%s' not found", id);
+        return;
+    }
+
+    if (!user_creatable_can_be_deleted(USER_CREATABLE(obj), errp)) {
+        error_setg(errp, "object '%s' is in use, can not be deleted", id);
+        return;
+    }
+    object_unparent(obj);
+}
+
 static void register_types(void)
 {
     static const TypeInfo uc_interface_info = {

vl.c +6 -62

index 175ebcc..9bd881e 100644
@@ -2816,64 +2816,6 @@ static bool object_create_delayed(const char *type)
 }
 
 
-static int object_create(void *opaque, QemuOpts *opts, Error **errp)
-{
-    Error *err = NULL;
-    Error *err_end = NULL;
-    char *type = NULL;
-    char *id = NULL;
-    OptsVisitor *ov;
-    QDict *pdict;
-    bool (*type_predicate)(const char *) = opaque;
-    Visitor *v;
-
-    ov = opts_visitor_new(opts);
-    pdict = qemu_opts_to_qdict(opts, NULL);
-    v = opts_get_visitor(ov);
-
-    visit_start_struct(v, NULL, NULL, 0, &err);
-    if (err) {
-        goto out;
-    }
-
-    qdict_del(pdict, "qom-type");
-    visit_type_str(v, "qom-type", &type, &err);
-    if (err) {
-        goto out;
-    }
-    if (!type_predicate(type)) {
-        visit_end_struct(v, NULL);
-        goto out;
-    }
-
-    qdict_del(pdict, "id");
-    visit_type_str(v, "id", &id, &err);
-    if (err) {
-        goto out_end;
-    }
-
-    object_add(type, id, pdict, v, &err);
-
-out_end:
-    visit_end_struct(v, &err_end);
-    if (!err && err_end) {
-        qmp_object_del(id, NULL);
-    }
-    error_propagate(&err, err_end);
-
-out:
-    opts_visitor_cleanup(ov);
-
-    QDECREF(pdict);
-    g_free(id);
-    g_free(type);
-    if (err) {
-        error_report_err(err);
-        return -1;
-    }
-    return 0;
-}
-
 static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size,
                                MachineClass *mc)
 {

@@ -4299,8 +4241,9 @@ int main(int argc, char **argv, char **envp)
     socket_init();
 
     if (qemu_opts_foreach(qemu_find_opts("object"),
-                          object_create,
-                          object_create_initial, NULL)) {
+                          user_creatable_add_opts_foreach,
+                          object_create_initial, &err)) {
+        error_report_err(err);
         exit(1);
     }
 

@@ -4417,8 +4360,9 @@ int main(int argc, char **argv, char **envp)
     }
 
     if (qemu_opts_foreach(qemu_find_opts("object"),
-                          object_create,
-                          object_create_delayed, NULL)) {
+                          user_creatable_add_opts_foreach,
+                          object_create_delayed, &err)) {
+        error_report_err(err);
         exit(1);
     }

This commit regresses error message quality from

    $ qemu-system-x86_64 -nodefaults -display none -object secret,id=sec0,data=letmein,format=raw,foo=bar
    qemu-system-x86_64: -object secret,id=sec0,data=letmein,format=raw,foo=bar: Property '.foo' not found

to just

qemu-system-x86_64: Property '.foo' not found

Clue: cur_loc points to garbage.

    (gdb) p cur_loc
    $1 = (Location *) 0x7fffffffdc10
    (gdb) p *cur_loc
    $2 = {kind = (unknown: 4294958128), num = 32767, 
      ptr = 0x555555b804a2 <error_report_err+44>, prev = 0x5555565d2770 <std_loc>}

Looks like cur_loc is dangling. Happens when you forget to loc_pop() a Location before it dies. This one is on the stack.

*Might* be release critical.

For comparison, this is how it looks before the patch:

    (gdb) p cur_loc
    $1 = (Location *) 0x7fffffffdc10
    (gdb) p *cur_loc
    $2 = {kind = LOC_CMDLINE, num = 2, ptr = 0x7fffffffe018, prev = 
        0x5555565d2770 <std_loc>}

Reported-by Eric Blake <[email protected]>

Quoted 633 lines · Daniel P. Berrange

The QMP monitor code has two helper methods object_add and qmp_object_del that are called from several places in the code (QMP, HMP and main emulator startup).

The HMP and main emulator startup code also share further logic that extracts the qom-type & id values from a qdict.

We soon need to use this logic from qemu-img, qemu-io and qemu-nbd too, but don't want those to depend on the monitor, nor do we want to duplicate the code.

To avoid this, move some code out of qmp.c and hmp.c adding new methods to qom/object_interfaces.c

- user_creatable_add - takes a QDict holding a full object definition & instantiates it
- user_creatable_add_type - takes an ID, type name, and QDict holding object properties & instantiates it
- user_creatable_add_opts - takes a QemuOpts holding a full object definition & instantiates it
- user_creatable_add_opts_foreach - variant on user_creatable_add_opts which can be directly used in conjunction with qemu_opts_foreach.
- user_creatable_del - takes an ID and deletes the corresponding object

The existing code is updated to use these new methods.

Reviewed-by Eric Blake <[email protected]>

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

---
 hmp.c                           |  52 +++---------
 include/monitor/monitor.h       |   3 -
 include/qom/object_interfaces.h |  92 +++++++++++++++++++++
 qmp.c                           |  76 ++----------------
 qom/object_interfaces.c         | 174 ++++++++++++++++++++++++++++++++++++++++
 vl.c                            |  68 ++--------------
 6 files changed, 290 insertions(+), 175 deletions(-)

hmp.c +11 -41

index c6419da..41fb9ca 100644
@@ -30,6 +30,7 @@
 #include "qapi/string-output-visitor.h"
 #include "qapi/util.h"
 #include "qapi-visit.h"
+#include "qom/object_interfaces.h"
 #include "ui/console.h"
 #include "block/qapi.h"
 #include "qemu-io.h"

@@ -1655,58 +1656,27 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
 void hmp_object_add(Monitor *mon, const QDict *qdict)
 {
     Error *err = NULL;
-    Error *err_end = NULL;
     QemuOpts *opts;
-    char *type = NULL;
-    char *id = NULL;
     OptsVisitor *ov;
-    QDict *pdict;
-    Visitor *v;
+    Object *obj = NULL;
 
     opts = qemu_opts_from_qdict(qemu_find_opts("object"), qdict, &err);
     if (err) {
-        goto out;
+        hmp_handle_error(mon, &err);
+        return;
     }
 
     ov = opts_visitor_new(opts);
-    pdict = qdict_clone_shallow(qdict);
-    v = opts_get_visitor(ov);
-
-    visit_start_struct(v, NULL, NULL, 0, &err);
-    if (err) {
-        goto out_clean;
-    }
-
-    qdict_del(pdict, "qom-type");
-    visit_type_str(v, "qom-type", &type, &err);
-    if (err) {
-        goto out_end;
-    }
+    obj = user_creatable_add(qdict, opts_get_visitor(ov), &err);
+    opts_visitor_cleanup(ov);
+    qemu_opts_del(opts);
 
-    qdict_del(pdict, "id");
-    visit_type_str(v, "id", &id, &err);
     if (err) {
-        goto out_end;
+        hmp_handle_error(mon, &err);
     }
-
-    object_add(type, id, pdict, v, &err);
-
-out_end:
-    visit_end_struct(v, &err_end);
-    if (!err && err_end) {
-        qmp_object_del(id, NULL);
+    if (obj) {
+        object_unref(obj);
     }
-    error_propagate(&err, err_end);
-out_clean:
-    opts_visitor_cleanup(ov);
-
-    QDECREF(pdict);
-    qemu_opts_del(opts);
-    g_free(id);
-    g_free(type);
-
-out:
-    hmp_handle_error(mon, &err);
 }
 
 void hmp_getfd(Monitor *mon, const QDict *qdict)

@@ -1934,7 +1904,7 @@ void hmp_object_del(Monitor *mon, const QDict *qdict)
     const char *id = qdict_get_str(qdict, "id");
     Error *err = NULL;
 
-    qmp_object_del(id, &err);
+    user_creatable_del(id, &err);
     hmp_handle_error(mon, &err);
 }
 

include/monitor/monitor.h +0 -3

index 91b95ae..aa0f373 100644
@@ -43,9 +43,6 @@ void monitor_read_command(Monitor *mon, int show_prompt);
 int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
                           void *opaque);
 
-void object_add(const char *type, const char *id, const QDict *qdict,
-                Visitor *v, Error **errp);
-
 AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
                                 bool has_opaque, const char *opaque,
                                 Error **errp);

include/qom/object_interfaces.h +92 -0

index 283ae0d..d579746 100644
@@ -2,6 +2,8 @@
 #define OBJECT_INTERFACES_H
 
 #include "qom/object.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/visitor.h"
 
 #define TYPE_USER_CREATABLE "user-creatable"
 

@@ -72,4 +74,94 @@ void user_creatable_complete(Object *obj, Error **errp);
  * from implements USER_CREATABLE interface.
  */
 bool user_creatable_can_be_deleted(UserCreatable *uc, Error **errp);
+
+/**
+ * user_creatable_add:
+ * @qdict: the object definition
+ * @v: the visitor
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * Create an instance of the user creatable object whose type
+ * is defined in @qdict by the 'qom-type' field, placing it
+ * in the object composition tree with name provided by the
+ * 'id' field. The remaining fields in @qdict are used to
+ * initialize the object properties.
+ *
+ * Returns: the newly created object or NULL on error
+ */
+Object *user_creatable_add(const QDict *qdict,
+                           Visitor *v, Error **errp);
+
+/**
+ * user_creatable_add_type:
+ * @type: the object type name
+ * @id: the unique ID for the object
+ * @qdict: the object properties
+ * @v: the visitor
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * Create an instance of the user creatable object @type, placing
+ * it in the object composition tree with name @id, initializing
+ * it with properties from @qdict
+ *
+ * Returns: the newly created object or NULL on error
+ */
+Object *user_creatable_add_type(const char *type, const char *id,
+                                const QDict *qdict,
+                                Visitor *v, Error **errp);
+
+/**
+ * user_creatable_add_opts:
+ * @opts: the object definition
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * Create an instance of the user creatable object whose type
+ * is defined in @opts by the 'qom-type' option, placing it
+ * in the object composition tree with name provided by the
+ * 'id' field. The remaining options in @opts are used to
+ * initialize the object properties.
+ *
+ * Returns: the newly created object or NULL on error
+ */
+Object *user_creatable_add_opts(QemuOpts *opts, Error **errp);
+
+
+/**
+ * user_creatable_add_opts_predicate:
+ * @type: the QOM type to be added
+ *
+ * A callback function to determine whether an object
+ * of type @type should be created. Instances of this
+ * callback should be passed to user_creatable_add_opts_foreach
+ */
+typedef bool (*user_creatable_add_opts_predicate)(const char *type);
+
+/**
+ * user_creatable_add_opts_foreach:
+ * @opaque: a user_creatable_add_opts_predicate callback or NULL
+ * @opts: options to create
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * An iterator callback to be used in conjunction with
+ * the qemu_opts_foreach() method for creating a list of
+ * objects from a set of QemuOpts
+ *
+ * The @opaque parameter can be passed a user_creatable_add_opts_predicate
+ * callback to filter which types of object are created during iteration.
+ *
+ * Returns: 0 on success, -1 on error
+ */
+int user_creatable_add_opts_foreach(void *opaque,
+                                    QemuOpts *opts, Error **errp);
+
+/**
+ * user_creatable_del:
+ * @id: the unique ID for the object
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * Delete an instance of the user creatable object identified
+ * by @id.
+ */
+void user_creatable_del(const char *id, Error **errp);
+
 #endif

qmp.c +7 -69

index 6ae7230..9a93d5e 100644
@@ -633,65 +633,13 @@ void qmp_add_client(const char *protocol, const char *fdname,
     close(fd);
 }
 
-void object_add(const char *type, const char *id, const QDict *qdict,
-                Visitor *v, Error **errp)
-{
-    Object *obj;
-    ObjectClass *klass;
-    const QDictEntry *e;
-    Error *local_err = NULL;
-
-    klass = object_class_by_name(type);
-    if (!klass) {
-        error_setg(errp, "invalid object type: %s", type);
-        return;
-    }
-
-    if (!object_class_dynamic_cast(klass, TYPE_USER_CREATABLE)) {
-        error_setg(errp, "object type '%s' isn't supported by object-add",
-                   type);
-        return;
-    }
-
-    if (object_class_is_abstract(klass)) {
-        error_setg(errp, "object type '%s' is abstract", type);
-        return;
-    }
-
-    obj = object_new(type);
-    if (qdict) {
-        for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
-            object_property_set(obj, v, e->key, &local_err);
-            if (local_err) {
-                goto out;
-            }
-        }
-    }
-
-    object_property_add_child(object_get_objects_root(),
-                              id, obj, &local_err);
-    if (local_err) {
-        goto out;
-    }
-
-    user_creatable_complete(obj, &local_err);
-    if (local_err) {
-        object_property_del(object_get_objects_root(),
-                            id, &error_abort);
-        goto out;
-    }
-out:
-    if (local_err) {
-        error_propagate(errp, local_err);
-    }
-    object_unref(obj);
-}
 
 void qmp_object_add(const char *type, const char *id,
                     bool has_props, QObject *props, Error **errp)
 {
     const QDict *pdict = NULL;
     QmpInputVisitor *qiv;
+    Object *obj;
 
     if (props) {
         pdict = qobject_to_qdict(props);

@@ -702,27 +650,17 @@ void qmp_object_add(const char *type, const char *id,
     }
 
     qiv = qmp_input_visitor_new(props);
-    object_add(type, id, pdict, qmp_input_get_visitor(qiv), errp);
+    obj = user_creatable_add_type(type, id, pdict,
+                                  qmp_input_get_visitor(qiv), errp);
     qmp_input_visitor_cleanup(qiv);
+    if (obj) {
+        object_unref(obj);
+    }
 }
 
 void qmp_object_del(const char *id, Error **errp)
 {
-    Object *container;
-    Object *obj;
-
-    container = object_get_objects_root();
-    obj = object_resolve_path_component(container, id);
-    if (!obj) {
-        error_setg(errp, "object id not found");
-        return;
-    }
-
-    if (!user_creatable_can_be_deleted(USER_CREATABLE(obj), errp)) {
-        error_setg(errp, "%s is in use, can not be deleted", id);
-        return;
-    }
-    object_unparent(obj);
+    user_creatable_del(id, errp);
 }
 
 MemoryDeviceInfoList *qmp_query_memory_devices(Error **errp)

qom/object_interfaces.c +174 -0

index f1218f0..c2f6e29 100644
@@ -1,6 +1,9 @@
 #include "qemu/osdep.h"
 #include "qom/object_interfaces.h"
 #include "qemu/module.h"
+#include "qapi-visit.h"
+#include "qapi/qmp-output-visitor.h"
+#include "qapi/opts-visitor.h"
 
 void user_creatable_complete(Object *obj, Error **errp)
 {

@@ -31,6 +34,177 @@ bool user_creatable_can_be_deleted(UserCreatable *uc, Error **errp)
     }
 }
 
+
+Object *user_creatable_add(const QDict *qdict,
+                           Visitor *v, Error **errp)
+{
+    char *type = NULL;
+    char *id = NULL;
+    Object *obj = NULL;
+    Error *local_err = NULL, *end_err = NULL;
+    QDict *pdict;
+
+    pdict = qdict_clone_shallow(qdict);
+
+    visit_start_struct(v, NULL, NULL, 0, &local_err);
+    if (local_err) {
+        goto out;
+    }
+
+    qdict_del(pdict, "qom-type");
+    visit_type_str(v, "qom-type", &type, &local_err);
+    if (local_err) {
+        goto out_visit;
+    }
+
+    qdict_del(pdict, "id");
+    visit_type_str(v, "id", &id, &local_err);
+    if (local_err) {
+        goto out_visit;
+    }
+
+    obj = user_creatable_add_type(type, id, pdict, v, &local_err);
+    if (local_err) {
+        goto out_visit;
+    }
+
+ out_visit:
+    visit_end_struct(v, &end_err);
+    if (end_err) {
+        error_propagate(&local_err, end_err);
+        if (obj) {
+            user_creatable_del(id, NULL);
+        }
+        goto out;
+    }
+
+out:
+    QDECREF(pdict);
+    g_free(id);
+    g_free(type);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        object_unref(obj);
+        return NULL;
+    }
+    return obj;
+}
+
+
+Object *user_creatable_add_type(const char *type, const char *id,
+                                const QDict *qdict,
+                                Visitor *v, Error **errp)
+{
+    Object *obj;
+    ObjectClass *klass;
+    const QDictEntry *e;
+    Error *local_err = NULL;
+
+    klass = object_class_by_name(type);
+    if (!klass) {
+        error_setg(errp, "invalid object type: %s", type);
+        return NULL;
+    }
+
+    if (!object_class_dynamic_cast(klass, TYPE_USER_CREATABLE)) {
+        error_setg(errp, "object type '%s' isn't supported by object-add",
+                   type);
+        return NULL;
+    }
+
+    if (object_class_is_abstract(klass)) {
+        error_setg(errp, "object type '%s' is abstract", type);
+        return NULL;
+    }
+
+    obj = object_new(type);
+    if (qdict) {
+        for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
+            object_property_set(obj, v, e->key, &local_err);
+            if (local_err) {
+                goto out;
+            }
+        }
+    }
+
+    object_property_add_child(object_get_objects_root(),
+                              id, obj, &local_err);
+    if (local_err) {
+        goto out;
+    }
+
+    user_creatable_complete(obj, &local_err);
+    if (local_err) {
+        object_property_del(object_get_objects_root(),
+                            id, &error_abort);
+        goto out;
+    }
+out:
+    if (local_err) {
+        error_propagate(errp, local_err);
+        object_unref(obj);
+        return NULL;
+    }
+    return obj;
+}
+
+
+Object *user_creatable_add_opts(QemuOpts *opts, Error **errp)
+{
+    OptsVisitor *ov;
+    QDict *pdict;
+    Object *obj = NULL;
+
+    ov = opts_visitor_new(opts);
+    pdict = qemu_opts_to_qdict(opts, NULL);
+
+    obj = user_creatable_add(pdict, opts_get_visitor(ov), errp);
+    opts_visitor_cleanup(ov);
+    QDECREF(pdict);
+    return obj;
+}
+
+
+int user_creatable_add_opts_foreach(void *opaque, QemuOpts *opts, Error **errp)
+{
+    bool (*type_predicate)(const char *) = opaque;
+    Object *obj = NULL;
+    const char *type;
+
+    type = qemu_opt_get(opts, "qom-type");
+    if (type && type_predicate &&
+        !type_predicate(type)) {
+        return 0;
+    }
+
+    obj = user_creatable_add_opts(opts, errp);
+    if (!obj) {
+        return -1;
+    }
+    object_unref(obj);
+    return 0;
+}
+
+
+void user_creatable_del(const char *id, Error **errp)
+{
+    Object *container;
+    Object *obj;
+
+    container = object_get_objects_root();
+    obj = object_resolve_path_component(container, id);
+    if (!obj) {
+        error_setg(errp, "object '%s' not found", id);
+        return;
+    }
+
+    if (!user_creatable_can_be_deleted(USER_CREATABLE(obj), errp)) {
+        error_setg(errp, "object '%s' is in use, can not be deleted", id);
+        return;
+    }
+    object_unparent(obj);
+}
+
 static void register_types(void)
 {
     static const TypeInfo uc_interface_info = {

vl.c +3 -60

index 175ebcc..9bd881e 100644
@@ -2816,64 +2816,6 @@ static bool object_create_delayed(const char *type)
 }
 
 
-static int object_create(void *opaque, QemuOpts *opts, Error **errp)
-{
-    Error *err = NULL;
-    Error *err_end = NULL;
-    char *type = NULL;
-    char *id = NULL;
-    OptsVisitor *ov;
-    QDict *pdict;
-    bool (*type_predicate)(const char *) = opaque;
-    Visitor *v;
-
-    ov = opts_visitor_new(opts);
-    pdict = qemu_opts_to_qdict(opts, NULL);
-    v = opts_get_visitor(ov);
-
-    visit_start_struct(v, NULL, NULL, 0, &err);
-    if (err) {
-        goto out;
-    }
-
-    qdict_del(pdict, "qom-type");
-    visit_type_str(v, "qom-type", &type, &err);
-    if (err) {
-        goto out;
-    }
-    if (!type_predicate(type)) {
-        visit_end_struct(v, NULL);
-        goto out;
-    }
-
-    qdict_del(pdict, "id");
-    visit_type_str(v, "id", &id, &err);
-    if (err) {
-        goto out_end;
-    }
-
-    object_add(type, id, pdict, v, &err);
-
-out_end:
-    visit_end_struct(v, &err_end);
-    if (!err && err_end) {
-        qmp_object_del(id, NULL);
-    }
-    error_propagate(&err, err_end);
-
-out:
-    opts_visitor_cleanup(ov);
-
-    QDECREF(pdict);
-    g_free(id);
-    g_free(type);
-    if (err) {
-        error_report_err(err);
-        return -1;
-    }
-    return 0;
-}
-
 static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size,
                                MachineClass *mc)
 {

@@ -4299,8 +4241,9 @@ int main(int argc, char **argv, char **envp)
     socket_init();
 
     if (qemu_opts_foreach(qemu_find_opts("object"),
-                          object_create,
-                          object_create_initial, NULL)) {
+                          user_creatable_add_opts_foreach,
+                          object_create_initial, &err)) {
+        error_report_err(err);
         exit(1);
     }
 

@@ -4417,8 +4360,9 @@ int main(int argc, char **argv, char **envp)
     }
 

vl.c int main(int argc, char **argv, char **envp) · line 4367

Quoted 8 lines

vl.c +3 -2

     if (qemu_opts_foreach(qemu_find_opts("object"),
-                          object_create,
-                          object_create_delayed, NULL)) {
+                          user_creatable_add_opts_foreach,
+                          object_create_delayed, &err)) {
+        error_report_err(err);
         exit(1);
     }

Quoted 8 lines · Markus Armbruster

This commit regresses error message quality from

    $ qemu-system-x86_64 -nodefaults -display none -object secret,id=sec0,data=letmein,format=raw,foo=bar
    qemu-system-x86_64: -object secret,id=sec0,data=letmein,format=raw,foo=bar: Property '.foo' not found

to just

qemu-system-x86_64: Property '.foo' not found

I'm not seeing that behaviour myself in current git master, nor immediately before or after 90998d58964cd17f8b0b03800b0a4508f8b543da is applied. I always just get

 $ ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -display none -object secret,id=sec0,data=letmein,format=raw,foo=bar
 qemu-system-x86_64: -object secret,id=sec0,data=letmein,format=raw,foo=bar: Property '.foo' not found

So it all appears to be working correctly. How reliably reproducable is it for you ? I'm testing on Fedora 23 x86_64 host and can't see the failure despite many invokations.

Quoted 12 lines

Clue: cur_loc points to garbage.

    (gdb) p cur_loc
    $1 = (Location *) 0x7fffffffdc10
    (gdb) p *cur_loc
    $2 = {kind = (unknown: 4294958128), num = 32767, 
      ptr = 0x555555b804a2 <error_report_err+44>, prev = 0x5555565d2770 <std_loc>}

Looks like cur_loc is dangling. Happens when you forget to loc_pop() a Location before it dies. This one is on the stack.

Quoted 1 line

*Might* be release critical.

This patch doesn't even touch any code which calls loc_push/loc_pop so I'm kind of surprised if this patch breaks it. Given that it looks like stack corruption though, I wonder if this commit has just exposed an already latent non-deterministic bug for you ? IOW root cause could be an earlier patch ?

Quoted 11 lines

vl.c int main(int argc, char **argv, char **envp) · line 4367

Quoted 11 lines

vl.c +3 -2

@@ -4417,8 +4360,9 @@ int main(int argc, char **argv, char **envp)
     }
 
     if (qemu_opts_foreach(qemu_find_opts("object"),
-                          object_create,
-                          object_create_delayed, NULL)) {
+                          user_creatable_add_opts_foreach,
+                          object_create_delayed, &err)) {
+        error_report_err(err);
         exit(1);
     }

Regards,
Daniel

Quoted 14 lines · Daniel P. Berrange

Quoted 8 lines · Markus Armbruster

This commit regresses error message quality from

    $ qemu-system-x86_64 -nodefaults -display none -object secret,id=sec0,data=letmein,format=raw,foo=bar
    qemu-system-x86_64: -object secret,id=sec0,data=letmein,format=raw,foo=bar: Property '.foo' not found

to just

qemu-system-x86_64: Property '.foo' not found

I'm not seeing that behaviour myself in current git master, nor immediately before or after 90998d58964cd17f8b0b03800b0a4508f8b543da is applied. I always just get

Quoted 6 lines

 $ ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -display none -object secret,id=sec0,data=letmein,format=raw,foo=bar
 qemu-system-x86_64: -object secret,id=sec0,data=letmein,format=raw,foo=bar: Property '.foo' not found

So it all appears to be working correctly. How reliably reproducable is it for you ? I'm testing on Fedora 23 x86_64 host and can't see the failure despite many invokations.

I'm reproducing it on my F23 machine, where 90998d58 indeed flips the behavior I'm seeing. Maybe it's a factor of which malloc engine is in use, or level of compiler optimization?

My config.status states:
exec '/home/eblake/qemu/configure' '--enable-kvm' '--enable-system' '--disable-user' '--target-list=x86_64-softmmu,ppc64-softmmu' '--enable-debug'

This commit regresses error message quality from

    $ qemu-system-x86_64 -nodefaults -display none -object secret,id=sec0,data=letmein,format=raw,foo=bar
    qemu-system-x86_64: -object secret,id=sec0,data=letmein,format=raw,foo=bar: Property '.foo' not found

to just

qemu-system-x86_64: Property '.foo' not found

Clue: cur_loc points to garbage.

    (gdb) p cur_loc
    $1 = (Location *) 0x7fffffffdc10
    (gdb) p *cur_loc
    $2 = {kind = (unknown: 4294958128), num = 32767, 
      ptr = 0x555555b804a2 <error_report_err+44>, prev = 0x5555565d2770 <std_loc>}

Looks like cur_loc is dangling. Happens when you forget to loc_pop() a Location before it dies. This one is on the stack.

*Might* be release critical.

For comparison, this is how it looks before the patch:

    (gdb) p cur_loc
    $1 = (Location *) 0x7fffffffdc10
    (gdb) p *cur_loc
    $2 = {kind = LOC_CMDLINE, num = 2, ptr = 0x7fffffffe018, prev = 
        0x5555565d2770 <std_loc>}

Reported-by Eric Blake <[email protected]>

This series of patches expands the syntax of the qemu-img, qemu-nbd and qemu-io commands to make them more flexible.

v0: https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg04365.html
v1: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04014.html
v2: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04354.html
v3: https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg03381.html
v4: https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg04984.html
v5: https://lists.gnu.org/archive/html/qemu-block/2016-02/msg00022.html

First all three gain a --object parameter, which allows instantiation of user creatable object types. The immediate use case is to allow for creation of the 'secret' object type to pass passwords for curl, iscsi and rbd drivers. For qemu-nbd this will also be needed to create TLS certificates for encryption support.

Then all three gain a '--image-opts' parameter which causes the positional filenames to be interepreted as option strings rather tha nplain filenames. This avoids the need to use the JSON syntax, or to add custom CLI args for each block backend option that exists. The immediate use case is to allow the user to specify the ID of the 'secret' object they just created.

Finally, there are a few small cleanup patches

Am 15.02.2016 um 15:33 hat Daniel P. Berrange geschrieben:

Quoted 24 lines

This series of patches expands the syntax of the qemu-img, qemu-nbd and qemu-io commands to make them more flexible.

v0: https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg04365.html
v1: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04014.html
v2: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04354.html
v3: https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg03381.html
v4: https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg04984.html
v5: https://lists.gnu.org/archive/html/qemu-block/2016-02/msg00022.html

First all three gain a --object parameter, which allows instantiation of user creatable object types. The immediate use case is to allow for creation of the 'secret' object type to pass passwords for curl, iscsi and rbd drivers. For qemu-nbd this will also be needed to create TLS certificates for encryption support.

Then all three gain a '--image-opts' parameter which causes the positional filenames to be interepreted as option strings rather tha nplain filenames. This avoids the need to use the JSON syntax, or to add custom CLI args for each block backend option that exists. The immediate use case is to allow the user to specify the ID of the 'secret' object they just created.

Quoted 1 line

Finally, there are a few small cleanup patches

Thanks, applied to the block branch.

Quoted 25 lines

Am 15.02.2016 um 15:33 hat Daniel P. Berrange geschrieben:

Quoted 24 lines

This series of patches expands the syntax of the qemu-img, qemu-nbd and qemu-io commands to make them more flexible.

v0: https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg04365.html
v1: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04014.html
v2: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04354.html
v3: https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg03381.html
v4: https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg04984.html
v5: https://lists.gnu.org/archive/html/qemu-block/2016-02/msg00022.html

First all three gain a --object parameter, which allows instantiation of user creatable object types. The immediate use case is to allow for creation of the 'secret' object type to pass passwords for curl, iscsi and rbd drivers. For qemu-nbd this will also be needed to create TLS certificates for encryption support.

Then all three gain a '--image-opts' parameter which causes the positional filenames to be interepreted as option strings rather tha nplain filenames. This avoids the need to use the JSON syntax, or to add custom CLI args for each block backend option that exists. The immediate use case is to allow the user to specify the ID of the 'secret' object they just created.

Quoted 3 lines

Quoted 1 line

Finally, there are a few small cleanup patches

Thanks, applied to the block branch.

The NBD patches just merged, so I've rebased this series and sent you a v7 to resolve the conflicts

Am 17.02.2016 um 11:10 hat Daniel P. Berrange geschrieben:

Quoted 26 lines · Kevin Wolf

Quoted 25 lines

Am 15.02.2016 um 15:33 hat Daniel P. Berrange geschrieben:

Quoted 24 lines

This series of patches expands the syntax of the qemu-img, qemu-nbd and qemu-io commands to make them more flexible.

v0: https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg04365.html
v1: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04014.html
v2: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04354.html
v3: https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg03381.html
v4: https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg04984.html
v5: https://lists.gnu.org/archive/html/qemu-block/2016-02/msg00022.html

First all three gain a --object parameter, which allows instantiation of user creatable object types. The immediate use case is to allow for creation of the 'secret' object type to pass passwords for curl, iscsi and rbd drivers. For qemu-nbd this will also be needed to create TLS certificates for encryption support.

Then all three gain a '--image-opts' parameter which causes the positional filenames to be interepreted as option strings rather tha nplain filenames. This avoids the need to use the JSON syntax, or to add custom CLI args for each block backend option that exists. The immediate use case is to allow the user to specify the ID of the 'secret' object they just created.

Quoted 6 lines

Quoted 3 lines

Quoted 1 line

Finally, there are a few small cleanup patches

Thanks, applied to the block branch.

The NBD patches just merged, so I've rebased this series and sent you a v7 to resolve the conflicts

*sigh* Okay, I'll drop this series and re-review sometime later.