| Message ID | 20170623124700.1389-5-el13635@mail.ntua.gr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. > +typedef struct ThrottleGroupClass { > + /* private */ > + ObjectClass parent_class; > + /* public */ > +} ThrottleGroupClass; This is unused? > + > + > +#define DOUBLE 0 > +#define UINT64 1 > +#define UNSIGNED 2 > + > +typedef struct { > + BucketType type; > + int size; /* field size */ sizeof(double) == sizeof(uint64_t) == 8. This is a datatype field, not a size. > +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); > + > +ret: > + error_propagate(errp, local_error); > + return; > + > +} > + > +static void throttle_group_set(Object *obj, Visitor *v, const char * name, > + void *opaque, Error **errp) > + > +{ > + ThrottleGroup *tg = THROTTLE_GROUP(obj); > + ThrottleConfig cfg = tg->ts.cfg; > + Error *local_err = NULL; > + ThrottleParamInfo *info = opaque; > + int64_t value; What happens if this property is set while QEMU is already running? > + > + visit_type_int64(v, name, &value, &local_err); > + > + if (local_err) { > + goto out; > + } > + if (value < 0) { > + error_setg(&local_err, "%s value must be in range [0, %"PRId64"]", > + "iops-total", INT64_MAX); /* change option string */ iops-total? :) > + goto out; > + } This doesn't validate inputs properly for non int64_t types. I'm also worried that the command-line bps=,iops=,... options do not have unsigned or double types. Input ranges and validation should match the QEMU command-line (I know this is a bit of a pain with QOM since the property types are different from QEMU option types).
On Mon, Jun 26, 2017 at 03:52:34PM +0100, Stefan Hajnoczi wrote: >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. Thanks, QOM was difficult to figure out in general. >> + >> + >> +#define DOUBLE 0 >> +#define UINT64 1 >> +#define UNSIGNED 2 >> + >> +typedef struct { >> + BucketType type; >> + int size; /* field size */ > >sizeof(double) == sizeof(uint64_t) == 8. > >This is a datatype field, not a size. > >> +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); >> + >> +ret: >> + error_propagate(errp, local_error); >> + return; >> + >> +} >> + >> +static void throttle_group_set(Object *obj, Visitor *v, const char * name, >> + void *opaque, Error **errp) >> + >> +{ >> + ThrottleGroup *tg = THROTTLE_GROUP(obj); >> + ThrottleConfig cfg = tg->ts.cfg; >> + Error *local_err = NULL; >> + ThrottleParamInfo *info = opaque; >> + int64_t value; > >What happens if this property is set while QEMU is already running? I assume you mean setting a property while a group has active members and requests? My best answer would be "don't do that". I couldn't figure a way to do this cleanly. Individual limit changes may render a ThrottleConfig invalid, so it should not be allowed. ThrottleGroups and throttle nodes should be destroyed and recreated to change limits with this version, but in the next it will be done through block_set_io_throttle() which is the existing way to change limits and check for validity. This was discussed in the proposal about the new syntax we had on the list. As we also talked about on IRC, clock_type should be a ThrottleGroup field in order to set a group's configuration without passing a ThrottleGroupMember, so changing this will make group configuring possible on QMP. > >> + >> + visit_type_int64(v, name, &value, &local_err); >> + >> + if (local_err) { >> + goto out; >> + } >> + if (value < 0) { >> + error_setg(&local_err, "%s value must be in range [0, %"PRId64"]", >> + "iops-total", INT64_MAX); /* change option string */ > >iops-total? :) I even left a comment to remind myself to change this... pah. > >> + goto out; >> + } > >This doesn't validate inputs properly for non int64_t types. > >I'm also worried that the command-line bps=,iops=,... options do not >have unsigned or double types. Input ranges and validation should match >the QEMU command-line (I know this is a bit of a pain with QOM since the >property types are different from QEMU option types). I don't know what should be done about this, to be honest, except for manually checking the limits for each datatype in the QOM setters.
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.
On Mon, Jun 26, 2017 at 06:24:09PM +0300, Manos Pitsidianakis wrote: > On Mon, Jun 26, 2017 at 03:52:34PM +0100, Stefan Hajnoczi wrote: > > > +static void throttle_group_set(Object *obj, Visitor *v, const char * name, > > > + void *opaque, Error **errp) > > > + > > > +{ > > > + ThrottleGroup *tg = THROTTLE_GROUP(obj); > > > + ThrottleConfig cfg = tg->ts.cfg; > > > + Error *local_err = NULL; > > > + ThrottleParamInfo *info = opaque; > > > + int64_t value; > > > > What happens if this property is set while QEMU is already running? > > I assume you mean setting a property while a group has active members and > requests? My best answer would be "don't do that". I couldn't figure a way > to do this cleanly. Individual limit changes may render a ThrottleConfig > invalid, so it should not be allowed. ThrottleGroups and throttle nodes > should be destroyed and recreated to change limits with this version, but in > the next it will be done through block_set_io_throttle() which is the > existing way to change limits and check for validity. This was discussed in > the proposal about the new syntax we had on the list. Please ask on IRC or the mailing list if you have questions. If you are aware of a limitation and don't know the answer then submitting the code without any comment or question is dangerous. Reviewers may miss the problem :) and broken code gets merged. UserCreatableClass has a ->complete() callback. You can use this to perform final initialization and then "freeze" the properties by setting a bool flag. The property accessor functions can do: if (tg->init_complete) { error_setg(errp, "Property modification is not allowed at run-time"); return; } Something like this is used by backends/hostmem.c:host_memory_backend_set_size(), for example. > > > > > + goto out; > > > + } > > > > This doesn't validate inputs properly for non int64_t types. > > > > I'm also worried that the command-line bps=,iops=,... options do not > > have unsigned or double types. Input ranges and validation should match > > the QEMU command-line (I know this is a bit of a pain with QOM since the > > property types are different from QEMU option types). > > I don't know what should be done about this, to be honest, except for > manually checking the limits for each datatype in the QOM setters. I believe all throttling parameter types are int64_t in QemuOpts. If we want to be compatible with the command-line parameters then they should also be int64_t here instead of unsigned int or double. This approach makes sense from the QMP user perspective. QMP clients shouldn't have to deal with different data types depending on which throttling API they use. Let's keep it consistent - there's no real drawback to using int64_t.
On Mon, Jun 26, 2017 at 07:58:32PM +0300, Manos Pitsidianakis wrote: > 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. I suggest all throttle group objects are visible in QOM. Who are the non-QOM users? I have CCed Pradeep who has been working on virtio-9p throttling. Pradeep: You may be interested in this series, which makes the throttle group a QOM object (-object throttle-group,id=group0,bps=1235678). In other words, groups are becoming first-class objects instead of being hidden behind the member devices that use them. Stefan
On Mon 26 Jun 2017 06:58:32 PM CEST, Manos Pitsidianakis wrote: > 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. As Manos says accesses to the throttle_groups list need to be locked. What I don't like at first sight is the code duplication in throttle_group_incref() and throttle_group_obj_complete(). >>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. I haven't looked into this yet but it could be a solution. Berto
On Tue, Jun 27, 2017 at 06:05:55PM +0200, Alberto Garcia wrote: >On Mon 26 Jun 2017 06:58:32 PM CEST, Manos Pitsidianakis wrote: >> 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. > >As Manos says accesses to the throttle_groups list need to be locked. > >What I don't like at first sight is the code duplication in >throttle_group_incref() and throttle_group_obj_complete(). > >>>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. > >I haven't looked into this yet but it could be a solution. If throttle_groups_register_blk() uses QOM instead of calling throttle_group_incref() the duplication could be eliminated. Then all of throttle-groups.c uses QOM internally. I don't see any reason why not do this. >Berto >
On Tue, Jun 27, 2017 at 06:05:55PM +0200, Alberto Garcia wrote: > On Mon 26 Jun 2017 06:58:32 PM CEST, Manos Pitsidianakis wrote: > > 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. > > As Manos says accesses to the throttle_groups list need to be locked. Explicit locking is only necessary if the list is accessed outside the QEMU global mutex. If the monitor is the only thing that accesses the list then a lock is not necessary. Anyway, this point might be moot if every ThrottleGroup is a QOM object and we drop this code in favor of using QOM APIs to find and iterate over objects. Stefan
diff --git a/block/throttle-groups.c b/block/throttle-groups.c index 7883cbb511..60079dc8ea 100644 --- a/block/throttle-groups.c +++ b/block/throttle-groups.c @@ -25,9 +25,11 @@ #include "qemu/osdep.h" #include "sysemu/block-backend.h" #include "block/throttle-groups.h" +#include "qemu/throttle-options.h" #include "qemu/queue.h" #include "qemu/thread.h" #include "sysemu/qtest.h" +#include "qapi/error.h" /* The ThrottleGroup structure (with its ThrottleState) is shared * among different ThrottleGroupMembers and it's independent from @@ -54,6 +56,7 @@ * that BlockBackend has throttled requests in the queue. */ typedef struct ThrottleGroup { + Object parent_obj; char *name; /* This is constant during the lifetime of the group */ QemuMutex lock; /* This lock protects the following four fields */ @@ -562,3 +565,351 @@ static void throttle_groups_init(void) } block_init(throttle_groups_init); + + +static bool throttle_group_exists(const char *name) +{ + ThrottleGroup *iter; + bool ret = false; + + qemu_mutex_lock(&throttle_groups_lock); + /* Look for an existing group with that name */ + QTAILQ_FOREACH(iter, &throttle_groups, list) { + if (!strcmp(name, iter->name)) { + ret = true; + break; + } + } + + qemu_mutex_unlock(&throttle_groups_lock); + return ret; +} + +typedef struct ThrottleGroupClass { + /* private */ + ObjectClass parent_class; + /* public */ +} ThrottleGroupClass; + + +#define DOUBLE 0 +#define UINT64 1 +#define UNSIGNED 2 + +typedef struct { + BucketType type; + int size; /* field size */ + ptrdiff_t offset; /* offset in LeakyBucket struct. */ +} ThrottleParamInfo; + +static ThrottleParamInfo throttle_iops_total_info = { + THROTTLE_OPS_TOTAL, DOUBLE, offsetof(LeakyBucket, avg), +}; + +static ThrottleParamInfo throttle_iops_total_max_info = { + THROTTLE_OPS_TOTAL, DOUBLE, offsetof(LeakyBucket, max), +}; + +static ThrottleParamInfo throttle_iops_total_max_length_info = { + THROTTLE_OPS_TOTAL, UNSIGNED, offsetof(LeakyBucket, burst_length), +}; + +static ThrottleParamInfo throttle_iops_read_info = { + THROTTLE_OPS_READ, DOUBLE, offsetof(LeakyBucket, avg), +}; + +static ThrottleParamInfo throttle_iops_read_max_info = { + THROTTLE_OPS_READ, DOUBLE, offsetof(LeakyBucket, max), +}; + +static ThrottleParamInfo throttle_iops_read_max_length_info = { + THROTTLE_OPS_READ, UNSIGNED, offsetof(LeakyBucket, burst_length), +}; + +static ThrottleParamInfo throttle_iops_write_info = { + THROTTLE_OPS_WRITE, DOUBLE, offsetof(LeakyBucket, avg), +}; + +static ThrottleParamInfo throttle_iops_write_max_info = { + THROTTLE_OPS_WRITE, DOUBLE, offsetof(LeakyBucket, max), +}; + +static ThrottleParamInfo throttle_iops_write_max_length_info = { + THROTTLE_OPS_WRITE, UNSIGNED, offsetof(LeakyBucket, burst_length), +}; + +static ThrottleParamInfo throttle_bps_total_info = { + THROTTLE_BPS_TOTAL, DOUBLE, offsetof(LeakyBucket, avg), +}; + +static ThrottleParamInfo throttle_bps_total_max_info = { + THROTTLE_BPS_TOTAL, DOUBLE, offsetof(LeakyBucket, max), +}; + +static ThrottleParamInfo throttle_bps_total_max_length_info = { + THROTTLE_BPS_TOTAL, UNSIGNED, offsetof(LeakyBucket, burst_length), +}; + +static ThrottleParamInfo throttle_bps_read_info = { + THROTTLE_BPS_READ, DOUBLE, offsetof(LeakyBucket, avg), +}; + +static ThrottleParamInfo throttle_bps_read_max_info = { + THROTTLE_BPS_READ, DOUBLE, offsetof(LeakyBucket, max), +}; + +static ThrottleParamInfo throttle_bps_read_max_length_info = { + THROTTLE_BPS_READ, UNSIGNED, offsetof(LeakyBucket, burst_length), +}; + +static ThrottleParamInfo throttle_bps_write_info = { + THROTTLE_BPS_WRITE, DOUBLE, offsetof(LeakyBucket, avg), +}; + +static ThrottleParamInfo throttle_bps_write_max_info = { + THROTTLE_BPS_WRITE, DOUBLE, offsetof(LeakyBucket, max), +}; + +static ThrottleParamInfo throttle_bps_write_max_length_info = { + THROTTLE_BPS_WRITE, UNSIGNED, offsetof(LeakyBucket, burst_length), +}; + +static ThrottleParamInfo throttle_iops_size_info = { + 0, UINT64, offsetof(ThrottleConfig, op_size), +}; + + +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; + } + + 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); + +ret: + error_propagate(errp, local_error); + return; + +} + +static void throttle_group_set(Object *obj, Visitor *v, const char * name, + void *opaque, Error **errp) + +{ + ThrottleGroup *tg = THROTTLE_GROUP(obj); + ThrottleConfig cfg = tg->ts.cfg; + Error *local_err = NULL; + ThrottleParamInfo *info = opaque; + int64_t value; + + visit_type_int64(v, name, &value, &local_err); + + if (local_err) { + goto out; + } + if (value < 0) { + error_setg(&local_err, "%s value must be in range [0, %"PRId64"]", + "iops-total", INT64_MAX); /* change option string */ + goto out; + } + + switch (info->size) { + case UINT64: + { + uint64_t *field = (void *)&cfg.buckets[info->type] + info->offset; + *field = value; + } + break; + case DOUBLE: + { + double *field = (void *)&cfg.buckets[info->type] + info->offset; + *field = value; + } + break; + case UNSIGNED: + { + unsigned *field = (void *)&cfg.buckets[info->type] + info->offset; + *field = value; + } + } + + tg->ts.cfg = cfg; + +out: + error_propagate(errp, local_err); +} + +static void throttle_group_get(Object *obj, Visitor *v, const char *name, + void *opaque, Error **errp) +{ + ThrottleGroup *tg = THROTTLE_GROUP(obj); + ThrottleConfig cfg = tg->ts.cfg; + ThrottleParamInfo *info = opaque; + int64_t value; + + switch (info->size) { + case UINT64: + { + uint64_t *field = (void *)&cfg.buckets[info->type] + info->offset; + value = *field; + } + break; + case DOUBLE: + { + double *field = (void *)&cfg.buckets[info->type] + info->offset; + value = *field; + } + break; + case UNSIGNED: + { + unsigned *field = (void *)&cfg.buckets[info->type] + info->offset; + value = *field; + } + } + + visit_type_int64(v, name, &value, errp); + +} + +static void throttle_group_init(Object *obj) +{ + ThrottleGroup *tg = THROTTLE_GROUP(obj); + throttle_init(&tg->ts); +} + +static void throttle_group_class_init(ObjectClass *klass, void *class_data) +{ + UserCreatableClass *ucc = USER_CREATABLE_CLASS(klass); + + ucc->complete = throttle_group_obj_complete; + /* iops limits */ + object_class_property_add(klass, QEMU_OPT_IOPS_TOTAL, "int", + throttle_group_get, + throttle_group_set, + NULL, &throttle_iops_total_info, &error_abort); + object_class_property_add(klass, QEMU_OPT_IOPS_TOTAL_MAX, "int", + throttle_group_get, + throttle_group_set, + NULL, &throttle_iops_total_max_info, &error_abort); + object_class_property_add(klass, QEMU_OPT_IOPS_TOTAL_MAX_LENGTH, "int", + throttle_group_get, + throttle_group_set, + NULL, &throttle_iops_total_max_length_info, &error_abort); + object_class_property_add(klass, QEMU_OPT_IOPS_READ, "int", + throttle_group_get, + throttle_group_set, + NULL, &throttle_iops_read_info, &error_abort); + object_class_property_add(klass, QEMU_OPT_IOPS_READ_MAX, "int", + throttle_group_get, + throttle_group_set, + NULL, &throttle_iops_read_max_info, &error_abort); + object_class_property_add(klass, QEMU_OPT_IOPS_READ_MAX_LENGTH, "int", + throttle_group_get, + throttle_group_set, + NULL, &throttle_iops_read_max_length_info, &error_abort); + object_class_property_add(klass, QEMU_OPT_IOPS_WRITE, "int", + throttle_group_get, + throttle_group_set, + NULL, &throttle_iops_write_info, &error_abort); + object_class_property_add(klass, QEMU_OPT_IOPS_WRITE_MAX, "int", + throttle_group_get, + throttle_group_set, + NULL, &throttle_iops_write_max_info, &error_abort); + object_class_property_add(klass, QEMU_OPT_IOPS_WRITE_MAX_LENGTH, "int", + throttle_group_get, + throttle_group_set, + NULL, &throttle_iops_write_max_length_info, &error_abort); + /* bps limits */ + object_class_property_add(klass, QEMU_OPT_BPS_TOTAL, "int", + throttle_group_get, + throttle_group_set, + NULL, &throttle_bps_total_info, &error_abort); + object_class_property_add(klass, QEMU_OPT_BPS_TOTAL_MAX, "int", + throttle_group_get, + throttle_group_set, + NULL, &throttle_bps_total_max_info, &error_abort); + object_class_property_add(klass, QEMU_OPT_BPS_TOTAL_MAX_LENGTH, "int", + throttle_group_get, + throttle_group_set, + NULL, &throttle_bps_total_max_length_info, &error_abort); + object_class_property_add(klass, QEMU_OPT_BPS_READ, "int", + throttle_group_get, + throttle_group_set, + NULL, &throttle_bps_read_info, &error_abort); + object_class_property_add(klass, QEMU_OPT_BPS_READ_MAX, "int", + throttle_group_get, + throttle_group_set, + NULL, &throttle_bps_read_max_info, &error_abort); + object_class_property_add(klass, QEMU_OPT_BPS_READ_MAX_LENGTH, "int", + throttle_group_get, + throttle_group_set, + NULL, &throttle_bps_read_max_length_info, &error_abort); + object_class_property_add(klass, QEMU_OPT_BPS_WRITE, "int", + throttle_group_get, + throttle_group_set, + NULL, &throttle_bps_write_info, &error_abort); + object_class_property_add(klass, QEMU_OPT_BPS_WRITE_MAX, "int", + throttle_group_get, + throttle_group_set, + NULL, &throttle_bps_write_max_info, &error_abort); + object_class_property_add(klass, QEMU_OPT_BPS_WRITE_MAX_LENGTH, "int", + throttle_group_get, + throttle_group_set, + NULL, &throttle_bps_write_max_length_info, &error_abort); + /* rest */ + object_class_property_add(klass, QEMU_OPT_IOPS_SIZE, "int", + throttle_group_get, + throttle_group_set, + NULL, &throttle_iops_size_info, &error_abort); +} + + +static void throttle_group_finalize(Object *obj) +{ + ThrottleGroup *tg = THROTTLE_GROUP(obj); + + qemu_mutex_lock(&throttle_groups_lock); + if (--tg->refcount == 0) { + QTAILQ_REMOVE(&throttle_groups, tg, list); + qemu_mutex_destroy(&tg->lock); + g_free(tg->name); + g_free(tg); + } + qemu_mutex_unlock(&throttle_groups_lock); + +} + +static const TypeInfo throttle_group_info = { + .name = TYPE_THROTTLE_GROUP, + .parent = TYPE_OBJECT, + .class_init = throttle_group_class_init, + .instance_size = sizeof(ThrottleGroup), + .instance_init = throttle_group_init, + .instance_finalize = throttle_group_finalize, + .interfaces = (InterfaceInfo[]) { + { TYPE_USER_CREATABLE }, + { } + }, +}; + +static void throttle_group_register_types(void) +{ + qemu_mutex_init(&throttle_groups_lock); + type_register_static(&throttle_group_info); +} + +type_init(throttle_group_register_types); diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h index 3e92d4d4eb..dd56baeb35 100644 --- a/include/qemu/throttle.h +++ b/include/qemu/throttle.h @@ -28,6 +28,8 @@ #include "qemu-common.h" #include "qemu/timer.h" #include "qemu/coroutine.h" +#include "qom/object.h" +#include "qom/object_interfaces.h" #define THROTTLE_VALUE_MAX 1000000000000000LL @@ -180,4 +182,6 @@ typedef struct ThrottleGroupMember { } ThrottleGroupMember; +#define TYPE_THROTTLE_GROUP "throttling-group" +#define THROTTLE_GROUP(obj) OBJECT_CHECK(ThrottleGroup, (obj), TYPE_THROTTLE_GROUP) #endif
ThrottleGroup is converted to an object to allow easy runtime configuration of throttling filter nodes in the BDS graph using QOM. Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr> --- block/throttle-groups.c | 351 ++++++++++++++++++++++++++++++++++++++++++++++++ include/qemu/throttle.h | 4 + 2 files changed, 355 insertions(+)