Re: [Qemu-devel] [Qemu-block] [PATCH RFC v3 4/8] block: convert ThrottleGroup to object with QOM

[Manos Pitsidianakis <el13635@mail.ntua.gr>]

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

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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]