On Mon, Jun 26, 2017 at 03:52:34PM +0100, Stefan Hajnoczi wrote:
On Fri, Jun 23, 2017 at 03:46:56PM +0300, Manos Pitsidianakis wrote:
+static bool throttle_group_exists(const char *name)
+{
+    ThrottleGroup *iter;
+    bool ret = false;
+
+    qemu_mutex_lock(&throttle_groups_lock);

Not sure if this lock or the throttle_groups list are necessary.

Have you seen iothread.c:qmp_query_iothreads()?  All objects are put
into a container (the parent object), so you can just iterate over its
children.  There's no need for a separate list because QOM already has
all the objects.


+static void throttle_group_obj_complete(UserCreatable *obj, Error **errp)
+{
+    char *name = NULL;
+    Error *local_error = NULL;
+    ThrottleGroup *tg = THROTTLE_GROUP(obj);
+
+    name = object_get_canonical_path_component(OBJECT(obj));
+    if (throttle_group_exists(name)) {
+        error_setg(&local_error, "A throttle group with this name already \
+                                  exists.");
+        goto ret;
+    }

QOM should enforce unique id=<ID>.  I don't think this is necessary.

+
+    qemu_mutex_lock(&throttle_groups_lock);
+    tg->name = name;
+    qemu_mutex_init(&tg->lock);
+    QLIST_INIT(&tg->head);
+    QTAILQ_INSERT_TAIL(&throttle_groups, tg, list);
+    tg->refcount++;
+    qemu_mutex_unlock(&throttle_groups_lock);
+

Sorry for the multiple replies but I just remembered this.

This is necessary because throttle groups are created by other interfaces as well. Of course block/throttle-groups.c could use only QOM objects internally to eliminate the housekeeping.

Attachment: signature.asc
Description: PGP signature

Reply via email to