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

[Stefan Hajnoczi]

On Mon, Jul 31, 2017 at 12:54:40PM +0300, Manos Pitsidianakis wrote:
> ThrottleGroup is converted to an object. This will allow the future
> throttle block filter drive easy creation and configuration of throttle
> groups in QMP and cli.
> 
> A new QAPI struct, ThrottleLimits, is introduced to provide a shared
> struct for all throttle configuration needs in QMP.
> 
> ThrottleGroups can be created via CLI as
>     -object throttle-group,id=foo,x-iops-total=100,x-..
> where x-* are individual limit properties. Since we can't add non-scalar
> properties in -object this interface must be used instead. However,
> setting these properties must be disabled after initialization because
> certain combinations of limits are forbidden and thus configuration
> changes should be done in one transaction. The individual properties
> will go away when support for non-scalar values in CLI is implemented
> and thus are marked as experimental.
> 
> ThrottleGroup also has a `limits` property that uses the ThrottleLimits
> struct.  It can be used to create ThrottleGroups or set the
> configuration in existing groups as follows:
> 
> { "execute": "object-add",
>   "arguments": {
>     "qom-type": "throttle-group",
>     "id": "foo",
>     "props" : {
>       "limits": {
>           "iops-total": 100
>       }
>     }
>   }
> }
> { "execute" : "qom-set",
>     "arguments" : {
>         "path" : "foo",
>         "property" : "limits",
>         "value" : {
>             "iops-total" : 99
>         }
>     }
> }
> 
> This also means a group's configuration can be fetched with qom-get.
> 
> ThrottleGroups can be anonymous which means they can't get accessed by
> other users ie they will always be units instead of group (Because they
> have one ThrottleGroupMember).

blockdev.c automatically assigns -drive id= to the group name if
throttling.group= wasn't given.  So who will use anonymous single-drive
mode?

> Signed-off-by: Manos Pitsidianakis <address@hidden>
> ---
> Notes:
>     Note: I tested Markus Armbruster's patch in <address@hidden>
>     on master and I can use this syntax successfuly:
>     -object '{ "qom-type" : "throttle-group", "id" : "foo", "props" : { 
> "limits" \
>     : { "iops-total" : 1000 } } }'
>     If this gets merged using -object will be a little more verbose but at 
> least we
>     won't have seperate properties, which is a good thing, so the x-* should 
> be
>     dropped.
> 
>  block/throttle-groups.c         | 402 
> ++++++++++++++++++++++++++++++++++++----
>  include/block/throttle-groups.h |   3 +
>  include/qemu/throttle-options.h |  59 ++++--
>  include/qemu/throttle.h         |   3 +
>  qapi/block-core.json            |  48 +++++
>  tests/test-throttle.c           |   1 +
>  util/throttle.c                 | 151 +++++++++++++++
>  7 files changed, 617 insertions(+), 50 deletions(-)
> 
> diff --git a/block/throttle-groups.c b/block/throttle-groups.c
> index f711a3dc62..b9c5036e44 100644
> --- a/block/throttle-groups.c
> +++ b/block/throttle-groups.c
> @@ -25,9 +25,17 @@
>  #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"
> +#include "qapi-visit.h"
> +#include "qom/object.h"
> +#include "qom/object_interfaces.h"
> +
> +static void throttle_group_obj_init(Object *obj);
> +static void throttle_group_obj_complete(UserCreatable *obj, Error **errp);
>  
>  /* The ThrottleGroup structure (with its ThrottleState) is shared
>   * among different ThrottleGroupMembers and it's independent from
> @@ -54,6 +62,10 @@
>   * that ThrottleGroupMember has throttled requests in the queue.
>   */
>  typedef struct ThrottleGroup {
> +    Object parent_obj;
> +
> +    /* refuse individual property change if initialization is complete */
> +    bool is_initialized;
>      char *name; /* This is constant during the lifetime of the group */
>  
>      QemuMutex lock; /* This lock protects the following four fields */
> @@ -63,8 +75,7 @@ typedef struct ThrottleGroup {
>      bool any_timer_armed[2];
>      QEMUClockType clock_type;
>  
> -    /* These two are protected by the global throttle_groups_lock */
> -    unsigned refcount;
> +    /* This is protected by the global throttle_groups_lock */
>      QTAILQ_ENTRY(ThrottleGroup) list;
>  } ThrottleGroup;
>  
> @@ -77,7 +88,8 @@ static QTAILQ_HEAD(, ThrottleGroup) throttle_groups =
>   * If no ThrottleGroup is found with the given name a new one is
>   * created.
>   *
> - * @name: the name of the ThrottleGroup
> + * @name: the name of the ThrottleGroup, NULL means a new anonymous group 
> will
> + *        be created.
>   * @ret:  the ThrottleState member of the ThrottleGroup
>   */
>  ThrottleState *throttle_group_incref(const char *name)
> @@ -87,32 +99,30 @@ ThrottleState *throttle_group_incref(const char *name)
>  
>      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)) {
> -            tg = iter;
> -            break;
> +    if (name) {
> +        /* Look for an existing group with that name */
> +        QTAILQ_FOREACH(iter, &throttle_groups, list) {
> +            if (!g_strcmp0(name, iter->name)) {
> +                tg = iter;
> +                break;
> +            }
>          }
>      }
>  
>      /* Create a new one if not found */
>      if (!tg) {
> -        tg = g_new0(ThrottleGroup, 1);
> +        /* new ThrottleGroup obj will have a refcnt = 1 */
> +        tg = THROTTLE_GROUP(object_new(TYPE_THROTTLE_GROUP));
>          tg->name = g_strdup(name);
> -        tg->clock_type = QEMU_CLOCK_REALTIME;
> -
> -        if (qtest_enabled()) {
> -            /* For testing block IO throttling only */
> -            tg->clock_type = QEMU_CLOCK_VIRTUAL;
> -        }
> -        qemu_mutex_init(&tg->lock);
> -        throttle_init(&tg->ts);
> -        QLIST_INIT(&tg->head);
> -
> -        QTAILQ_INSERT_TAIL(&throttle_groups, tg, list);
> +        throttle_group_obj_complete((UserCreatable *)tg, &error_abort);
>      }
>  
> -    tg->refcount++;
> +    qemu_mutex_lock(&tg->lock);
> +    if (!QLIST_EMPTY(&tg->head)) {
> +        /* only ref if the group is not empty */
> +        object_ref(OBJECT(tg));
> +    }
> +    qemu_mutex_unlock(&tg->lock);

How do the refcounts work in various cases (legacy -drive
throttling.group and -object throttle-group with 0, 1, or multiple
drives)?

It's not obvious to me that this code works in all cases.

>  
>      qemu_mutex_unlock(&throttle_groups_lock);
>  
> @@ -129,15 +139,7 @@ ThrottleState *throttle_group_incref(const char *name)
>  void throttle_group_unref(ThrottleState *ts)
>  {
>      ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
> -
> -    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);
> +    object_unref(OBJECT(tg));
>  }
>  
>  /* Get the name from a ThrottleGroupMember's group. The name (and the 
> pointer)
> @@ -572,9 +574,347 @@ void 
> throttle_group_detach_aio_context(ThrottleGroupMember *tgm)
>      throttle_timers_detach_aio_context(tt);
>  }
>  
> +#undef THROTTLE_OPT_PREFIX
> +#define THROTTLE_OPT_PREFIX "x-"
> +#define DOUBLE 0
> +#define UINT64 1
> +#define UNSIGNED 2
> +
> +typedef struct {
> +    const char *name;
> +    BucketType type;
> +    int data_type;
> +    const ptrdiff_t offset; /* offset in LeakyBucket struct. */
> +} ThrottleParamInfo;
> +
> +static ThrottleParamInfo properties[] = {
> +{
> +    THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_TOTAL,
> +    THROTTLE_OPS_TOTAL, DOUBLE, offsetof(LeakyBucket, avg),
> +},
> +{
> +    THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_TOTAL_MAX,
> +    THROTTLE_OPS_TOTAL, DOUBLE, offsetof(LeakyBucket, max),
> +},
> +{
> +    THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_TOTAL_MAX_LENGTH,
> +    THROTTLE_OPS_TOTAL, UNSIGNED, offsetof(LeakyBucket, burst_length),
> +},
> +{
> +    THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_READ,
> +    THROTTLE_OPS_READ, DOUBLE, offsetof(LeakyBucket, avg),
> +},
> +{
> +    THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_READ_MAX,
> +    THROTTLE_OPS_READ, DOUBLE, offsetof(LeakyBucket, max),
> +},
> +{
> +    THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_READ_MAX_LENGTH,
> +    THROTTLE_OPS_READ, UNSIGNED, offsetof(LeakyBucket, burst_length),
> +},
> +{
> +    THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_WRITE,
> +    THROTTLE_OPS_WRITE, DOUBLE, offsetof(LeakyBucket, avg),
> +},
> +{
> +    THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_WRITE_MAX,
> +    THROTTLE_OPS_WRITE, DOUBLE, offsetof(LeakyBucket, max),
> +},
> +{
> +    THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_WRITE_MAX_LENGTH,
> +    THROTTLE_OPS_WRITE, UNSIGNED, offsetof(LeakyBucket, burst_length),
> +},
> +{
> +    THROTTLE_OPT_PREFIX QEMU_OPT_BPS_TOTAL,
> +    THROTTLE_BPS_TOTAL, DOUBLE, offsetof(LeakyBucket, avg),
> +},
> +{
> +    THROTTLE_OPT_PREFIX QEMU_OPT_BPS_TOTAL_MAX,
> +    THROTTLE_BPS_TOTAL, DOUBLE, offsetof(LeakyBucket, max),
> +},
> +{
> +    THROTTLE_OPT_PREFIX QEMU_OPT_BPS_TOTAL_MAX_LENGTH,
> +    THROTTLE_BPS_TOTAL, UNSIGNED, offsetof(LeakyBucket, burst_length),
> +},
> +{
> +    THROTTLE_OPT_PREFIX QEMU_OPT_BPS_READ,
> +    THROTTLE_BPS_READ, DOUBLE, offsetof(LeakyBucket, avg),
> +},
> +{
> +    THROTTLE_OPT_PREFIX QEMU_OPT_BPS_READ_MAX,
> +    THROTTLE_BPS_READ, DOUBLE, offsetof(LeakyBucket, max),
> +},
> +{
> +    THROTTLE_OPT_PREFIX QEMU_OPT_BPS_READ_MAX_LENGTH,
> +    THROTTLE_BPS_READ, UNSIGNED, offsetof(LeakyBucket, burst_length),
> +},
> +{
> +    THROTTLE_OPT_PREFIX QEMU_OPT_BPS_WRITE,
> +    THROTTLE_BPS_WRITE, DOUBLE, offsetof(LeakyBucket, avg),
> +},
> +{
> +    THROTTLE_OPT_PREFIX QEMU_OPT_BPS_WRITE_MAX,
> +    THROTTLE_BPS_WRITE, DOUBLE, offsetof(LeakyBucket, max),
> +},
> +{
> +    THROTTLE_OPT_PREFIX QEMU_OPT_BPS_WRITE_MAX_LENGTH,
> +    THROTTLE_BPS_WRITE, UNSIGNED, offsetof(LeakyBucket, burst_length),
> +},
> +{
> +    THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_SIZE,
> +    0, UINT64, offsetof(ThrottleConfig, op_size),
> +}
> +};
> +
> +static void throttle_group_obj_init(Object *obj)
> +{
> +    ThrottleGroup *tg = THROTTLE_GROUP(obj);
> +
> +    tg->clock_type = QEMU_CLOCK_REALTIME;
> +    if (qtest_enabled()) {
> +        /* For testing block IO throttling only */
> +        tg->clock_type = QEMU_CLOCK_VIRTUAL;
> +    }
> +    tg->is_initialized = false;
> +    QTAILQ_INSERT_TAIL(&throttle_groups, tg, list);

Is the lock taken when the object-add QMP command or -object
command-line argument are used?

In any case, please do not assume that throttle_groups_lock is held for
Object->init().  It should be possible to create new instances at any
time.

> +    qemu_mutex_init(&tg->lock);
> +    throttle_init(&tg->ts);
> +    QLIST_INIT(&tg->head);
> +}
> +
> +static void throttle_group_obj_complete(UserCreatable *obj, Error **errp)
> +{
> +    ThrottleGroup *tg = THROTTLE_GROUP(obj), *iter;
> +    ThrottleConfig *cfg = &tg->ts.cfg;
> +
> +    /* set group name to object id if it exists */
> +    if (!tg->name && tg->parent_obj.parent) {
> +        tg->name = object_get_canonical_path_component(OBJECT(obj));
> +    }
> +
> +    if (tg->name) {
> +        /* error if name is duplicate */
> +        QTAILQ_FOREACH(iter, &throttle_groups, list) {

Same locking issue: is throttle_groups_lock held here in all cases?  I
think it's not held in the object-add QMP and -object throttle-group
cases.

> +            if (!g_strcmp0(tg->name, iter->name) && tg != iter) {
> +                error_setg(errp, "A group with this name already exists");
> +                return;
> +            }
> +        }
> +    }
> +
> +    /* unfix buckets to check validity */
> +    throttle_get_config(&tg->ts, cfg);
> +    if (!throttle_is_valid(cfg, errp)) {
> +        return;
> +    }
> +    /* fix buckets again */
> +    throttle_config(&tg->ts, tg->clock_type, cfg);
> +
> +    tg->is_initialized = true;
> +}
> +
> +static void throttle_group_obj_finalize(Object *obj)
> +{
> +    ThrottleGroup *tg = THROTTLE_GROUP(obj);
> +    qemu_mutex_lock(&throttle_groups_lock);
> +    QTAILQ_REMOVE(&throttle_groups, tg, list);
> +    qemu_mutex_unlock(&throttle_groups_lock);
> +    qemu_mutex_destroy(&tg->lock);
> +    g_free(tg->name);
> +}
> +
> +static void throttle_group_set(Object *obj, Visitor *v, const char * name,
> +        void *opaque, Error **errp)
> +
> +{
> +    ThrottleGroup *tg = THROTTLE_GROUP(obj);
> +    ThrottleConfig cfg;
> +    ThrottleParamInfo *info = opaque;
> +    ThrottleLimits *arg = NULL;
> +    Error *local_err = NULL;
> +    int64_t value;
> +
> +    /* If we have finished initialization, don't accept individual property
> +     * changes through QOM. Throttle configuration limits must be set in one
> +     * transaction, as certain combinations are invalid.
> +     */
> +    if (tg->is_initialized) {
> +        error_setg(&local_err, "Property cannot be set after 
> initialization");
> +        goto fail;
> +    }
> +
> +    visit_type_int64(v, name, &value, &local_err);
> +    if (local_err) {
> +        goto fail;
> +    }
> +    if (value < 0) {
> +        error_setg(&local_err, "Property values cannot be negative");
> +        goto fail;
> +    }
> +
> +    cfg = tg->ts.cfg;
> +    switch (info->data_type) {
> +    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:
> +        {
> +            if (value > UINT_MAX) {
> +                error_setg(&local_err, "%s value must be in the"
> +                                       "range [0, %u]", info->name, 
> UINT_MAX);
> +                goto fail;
> +            }
> +            unsigned *field = (void *)&cfg.buckets[info->type] + 
> info->offset;
> +            *field = value;
> +        }
> +    }
> +
> +    tg->ts.cfg = cfg;
> +    goto ret;
> +
> +fail:
> +    error_propagate(errp, local_err);
> +ret:
> +    g_free(arg);
> +    return;

arg is unused, please drop it.

> +
> +}
> +
> +static void throttle_group_get(Object *obj, Visitor *v, const char *name,
> +        void *opaque, Error **errp)
> +{
> +    ThrottleGroup *tg = THROTTLE_GROUP(obj);
> +    ThrottleConfig cfg;
> +    ThrottleParamInfo *info = opaque;
> +    int64_t value;
> +
> +    cfg = tg->ts.cfg;
> +    switch (info->data_type) {
> +    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_set_limits(Object *obj, Visitor *v,
> +                                      const char *name, void *opaque,
> +                                      Error **errp)
> +
> +{
> +    ThrottleGroup *tg = THROTTLE_GROUP(obj);
> +    ThrottleConfig cfg;
> +    ThrottleLimits *arg = NULL;
> +    Error *local_err = NULL;
> +
> +    arg = g_new0(ThrottleLimits, 1);

Why does ThrottleLimits need to be allocated on the heap?

> +    visit_type_ThrottleLimits(v, name, &arg, &local_err);
> +    if (local_err) {
> +        goto fail;
> +    }
> +    qemu_mutex_lock(&tg->lock);
> +    throttle_get_config(&tg->ts, &cfg);
> +    throttle_limits_to_config(arg, &cfg, &local_err);
> +    if (local_err) {
> +        qemu_mutex_unlock(&tg->lock);
> +        goto fail;
> +    }
> +    throttle_config(&tg->ts, tg->clock_type, &cfg);
> +    qemu_mutex_unlock(&tg->lock);
> +
> +    goto ret;

error_propagate() is a nop if local_err is NULL so this goto is
unnecessary.

> +
> +fail:
> +    error_propagate(errp, local_err);
> +ret:
> +    g_free(arg);
> +    return;
> +}
> +
> +static void throttle_group_get_limits(Object *obj, Visitor *v,
> +                                      const char *name, void *opaque,
> +                                      Error **errp)
> +{
> +    ThrottleGroup *tg = THROTTLE_GROUP(obj);
> +    ThrottleConfig cfg;
> +    ThrottleLimits *arg = NULL;
> +
> +    arg = g_new0(ThrottleLimits, 1);
> +    qemu_mutex_lock(&tg->lock);
> +    throttle_get_config(&tg->ts, &cfg);
> +    qemu_mutex_unlock(&tg->lock);
> +    throttle_config_to_throttle_limits(&cfg, arg);
> +    visit_type_ThrottleLimits(v, name, &arg, errp);
> +    g_free(arg);
> +}
> +
> +static void throttle_group_obj_class_init(ObjectClass *klass, void 
> *class_data)
> +{
> +    size_t i = 0;
> +    UserCreatableClass *ucc = USER_CREATABLE_CLASS(klass);
> +
> +    ucc->complete = throttle_group_obj_complete;
> +    /* individual properties */
> +    for (i = 0; i < sizeof(properties) / sizeof(ThrottleParamInfo); i++) {
> +        object_class_property_add(klass,
> +                properties[i].name,
> +                "int",
> +                throttle_group_get,
> +                throttle_group_set,
> +                NULL, &properties[i],
> +                &error_abort);
> +    }
> +
> +    /* ThrottleLimits */
> +    object_class_property_add(klass,
> +                              "limits", "ThrottleLimits",
> +                              throttle_group_get_limits,
> +                              throttle_group_set_limits,
> +                              NULL, NULL,
> +                              &error_abort);
> +}
> +
> +static const TypeInfo throttle_group_info = {
> +   .name = TYPE_THROTTLE_GROUP,
> +   .parent = TYPE_OBJECT,
> +   .class_init = throttle_group_obj_class_init,
> +   .instance_size = sizeof(ThrottleGroup),
> +   .instance_init = throttle_group_obj_init,
> +   .instance_finalize = throttle_group_obj_finalize,
> +   .interfaces = (InterfaceInfo[]) {
> +       { TYPE_USER_CREATABLE },
> +       { }
> +   },
> +};
> +
>  static void throttle_groups_init(void)
>  {
>      qemu_mutex_init(&throttle_groups_lock);
> +    type_register_static(&throttle_group_info);
>  }
>  
> -block_init(throttle_groups_init);
> +type_init(throttle_groups_init);
> diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h
> index a0f27cac63..82f030523f 100644
> --- a/include/block/throttle-groups.h
> +++ b/include/block/throttle-groups.h
> @@ -53,6 +53,9 @@ typedef struct ThrottleGroupMember {
>  
>  } ThrottleGroupMember;
>  
> +#define TYPE_THROTTLE_GROUP "throttle-group"
> +#define THROTTLE_GROUP(obj) OBJECT_CHECK(ThrottleGroup, (obj), 
> TYPE_THROTTLE_GROUP)
> +
>  const char *throttle_group_get_name(ThrottleGroupMember *tgm);
>  
>  ThrottleState *throttle_group_incref(const char *name);
> diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h
> index 3133d1ca40..182b7896e1 100644
> --- a/include/qemu/throttle-options.h
> +++ b/include/qemu/throttle-options.h
> @@ -10,81 +10,102 @@
>  #ifndef THROTTLE_OPTIONS_H
>  #define THROTTLE_OPTIONS_H
>  
> +#define QEMU_OPT_IOPS_TOTAL "iops-total"
> +#define QEMU_OPT_IOPS_TOTAL_MAX "iops-total-max"
> +#define QEMU_OPT_IOPS_TOTAL_MAX_LENGTH "iops-total-max-length"
> +#define QEMU_OPT_IOPS_READ "iops-read"
> +#define QEMU_OPT_IOPS_READ_MAX "iops-read-max"
> +#define QEMU_OPT_IOPS_READ_MAX_LENGTH "iops-read-max-length"
> +#define QEMU_OPT_IOPS_WRITE "iops-write"
> +#define QEMU_OPT_IOPS_WRITE_MAX "iops-write-max"
> +#define QEMU_OPT_IOPS_WRITE_MAX_LENGTH "iops-write-max-length"
> +#define QEMU_OPT_BPS_TOTAL "bps-total"
> +#define QEMU_OPT_BPS_TOTAL_MAX "bps-total-max"
> +#define QEMU_OPT_BPS_TOTAL_MAX_LENGTH "bps-total-max-length"
> +#define QEMU_OPT_BPS_READ "bps-read"
> +#define QEMU_OPT_BPS_READ_MAX "bps-read-max"
> +#define QEMU_OPT_BPS_READ_MAX_LENGTH "bps-read-max-length"
> +#define QEMU_OPT_BPS_WRITE "bps-write"
> +#define QEMU_OPT_BPS_WRITE_MAX "bps-write-max"
> +#define QEMU_OPT_BPS_WRITE_MAX_LENGTH "bps-write-max-length"
> +#define QEMU_OPT_IOPS_SIZE "iops-size"
> +
> +#define THROTTLE_OPT_PREFIX "throttling."
>  #define THROTTLE_OPTS \
>            { \
> -            .name = "throttling.iops-total",\
> +            .name = THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_TOTAL,\
>              .type = QEMU_OPT_NUMBER,\
>              .help = "limit total I/O operations per second",\
>          },{ \
> -            .name = "throttling.iops-read",\
> +            .name = THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_READ,\
>              .type = QEMU_OPT_NUMBER,\
>              .help = "limit read operations per second",\
>          },{ \
> -            .name = "throttling.iops-write",\
> +            .name = THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_WRITE,\
>              .type = QEMU_OPT_NUMBER,\
>              .help = "limit write operations per second",\
>          },{ \
> -            .name = "throttling.bps-total",\
> +            .name = THROTTLE_OPT_PREFIX QEMU_OPT_BPS_TOTAL,\
>              .type = QEMU_OPT_NUMBER,\
>              .help = "limit total bytes per second",\
>          },{ \
> -            .name = "throttling.bps-read",\
> +            .name = THROTTLE_OPT_PREFIX QEMU_OPT_BPS_READ,\
>              .type = QEMU_OPT_NUMBER,\
>              .help = "limit read bytes per second",\
>          },{ \
> -            .name = "throttling.bps-write",\
> +            .name = THROTTLE_OPT_PREFIX QEMU_OPT_BPS_WRITE,\
>              .type = QEMU_OPT_NUMBER,\
>              .help = "limit write bytes per second",\
>          },{ \
> -            .name = "throttling.iops-total-max",\
> +            .name = THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_TOTAL_MAX,\
>              .type = QEMU_OPT_NUMBER,\
>              .help = "I/O operations burst",\
>          },{ \
> -            .name = "throttling.iops-read-max",\
> +            .name = THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_READ_MAX,\
>              .type = QEMU_OPT_NUMBER,\
>              .help = "I/O operations read burst",\
>          },{ \
> -            .name = "throttling.iops-write-max",\
> +            .name = THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_WRITE_MAX,\
>              .type = QEMU_OPT_NUMBER,\
>              .help = "I/O operations write burst",\
>          },{ \
> -            .name = "throttling.bps-total-max",\
> +            .name = THROTTLE_OPT_PREFIX QEMU_OPT_BPS_TOTAL_MAX,\
>              .type = QEMU_OPT_NUMBER,\
>              .help = "total bytes burst",\
>          },{ \
> -            .name = "throttling.bps-read-max",\
> +            .name = THROTTLE_OPT_PREFIX QEMU_OPT_BPS_READ_MAX,\
>              .type = QEMU_OPT_NUMBER,\
>              .help = "total bytes read burst",\
>          },{ \
> -            .name = "throttling.bps-write-max",\
> +            .name = THROTTLE_OPT_PREFIX QEMU_OPT_BPS_WRITE_MAX,\
>              .type = QEMU_OPT_NUMBER,\
>              .help = "total bytes write burst",\
>          },{ \
> -            .name = "throttling.iops-total-max-length",\
> +            .name = THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_TOTAL_MAX_LENGTH,\
>              .type = QEMU_OPT_NUMBER,\
>              .help = "length of the iops-total-max burst period, in seconds",\
>          },{ \
> -            .name = "throttling.iops-read-max-length",\
> +            .name = THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_READ_MAX_LENGTH,\
>              .type = QEMU_OPT_NUMBER,\
>              .help = "length of the iops-read-max burst period, in seconds",\
>          },{ \
> -            .name = "throttling.iops-write-max-length",\
> +            .name = THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_WRITE_MAX_LENGTH,\
>              .type = QEMU_OPT_NUMBER,\
>              .help = "length of the iops-write-max burst period, in seconds",\
>          },{ \
> -            .name = "throttling.bps-total-max-length",\
> +            .name = THROTTLE_OPT_PREFIX QEMU_OPT_BPS_TOTAL_MAX_LENGTH,\
>              .type = QEMU_OPT_NUMBER,\
>              .help = "length of the bps-total-max burst period, in seconds",\
>          },{ \
> -            .name = "throttling.bps-read-max-length",\
> +            .name = THROTTLE_OPT_PREFIX QEMU_OPT_BPS_READ_MAX_LENGTH,\
>              .type = QEMU_OPT_NUMBER,\
>              .help = "length of the bps-read-max burst period, in seconds",\
>          },{ \
> -            .name = "throttling.bps-write-max-length",\
> +            .name = THROTTLE_OPT_PREFIX QEMU_OPT_BPS_WRITE_MAX_LENGTH,\
>              .type = QEMU_OPT_NUMBER,\
>              .help = "length of the bps-write-max burst period, in seconds",\
>          },{ \
> -            .name = "throttling.iops-size",\
> +            .name = THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_SIZE,\
>              .type = QEMU_OPT_NUMBER,\
>              .help = "when limiting by iops max size of an I/O in bytes",\
>          }
> diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
> index d056008c18..17e750b12d 100644
> --- a/include/qemu/throttle.h
> +++ b/include/qemu/throttle.h
> @@ -152,5 +152,8 @@ bool throttle_schedule_timer(ThrottleState *ts,
>                               bool is_write);
>  
>  void throttle_account(ThrottleState *ts, bool is_write, uint64_t size);
> +void throttle_limits_to_config(ThrottleLimits *arg, ThrottleConfig *cfg,
> +                               Error **errp);
> +void throttle_config_to_throttle_limits(ThrottleConfig *cfg, ThrottleLimits 
> *var);
>  
>  #endif
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 833c602150..0bdc69aa5f 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1905,6 +1905,54 @@
>              '*iops_size': 'int', '*group': 'str' } }
>  
>  ##
> +# @ThrottleLimits:
> +#
> +# Limit parameters for throttling.
> +# Since some limit combinations are illegal, limits should always be set in 
> one
> +# transaction. All fields are optional. When setting limits, if a field is
> +# missing the current value is not changed.
> +#
> +# @iops-total:             limit total I/O operations per second
> +# @iops-total-max:         I/O operations burst
> +# @iops-total-max-length:  length of the iops-total-max burst period, in 
> seconds
> +#                          It must only be set if @iops-total-max is set as 
> well.
> +# @iops-read:              limit read operations per second
> +# @iops-read-max:          I/O operations read burst
> +# @iops-read-max-length:   length of the iops-read-max burst period, in 
> seconds
> +#                          It must only be set if @iops-read-max is set as 
> well.
> +# @iops-write:             limit write operations per second
> +# @iops-write-max:         I/O operations write burst
> +# @iops-write-max-length:  length of the iops-write-max burst period, in 
> seconds
> +#                          It must only be set if @iops-write-max is set as 
> well.
> +# @bps-total:              limit total bytes per second
> +# @bps-total-max:          total bytes burst
> +# @bps-total-max-length:   length of the bps-total-max burst period, in 
> seconds.
> +#                          It must only be set if @bps-total-max is set as 
> well.
> +# @bps-read:               limit read bytes per second
> +# @bps-read-max:           total bytes read burst
> +# @bps-read-max-length:    length of the bps-read-max burst period, in 
> seconds
> +#                          It must only be set if @bps-read-max is set as 
> well.
> +# @bps-write:              limit write bytes per second
> +# @bps-write-max:          total bytes write burst
> +# @bps-write-max-length:   length of the bps-write-max burst period, in 
> seconds
> +#                          It must only be set if @bps-write-max is set as 
> well.
> +# @iops-size:              when limiting by iops max size of an I/O in bytes
> +#
> +# Since: 2.11
> +##
> +{ 'struct': 'ThrottleLimits',
> +  'data': { '*iops-total' : 'int', '*iops-total-max' : 'int',
> +            '*iops-total-max-length' : 'int', '*iops-read' : 'int',
> +            '*iops-read-max' : 'int', '*iops-read-max-length' : 'int',
> +            '*iops-write' : 'int', '*iops-write-max' : 'int',
> +            '*iops-write-max-length' : 'int', '*bps-total' : 'int',
> +            '*bps-total-max' : 'int', '*bps-total-max-length' : 'int',
> +            '*bps-read' : 'int', '*bps-read-max' : 'int',
> +            '*bps-read-max-length' : 'int', '*bps-write' : 'int',
> +            '*bps-write-max' : 'int', '*bps-write-max-length' : 'int',
> +            '*iops-size' : 'int' } }
> +
> +##
>  # @block-stream:
>  #
>  # Copy data from a backing file into a block device.
> diff --git a/tests/test-throttle.c b/tests/test-throttle.c
> index 57cf5ba711..0ea9093eee 100644
> --- a/tests/test-throttle.c
> +++ b/tests/test-throttle.c
> @@ -662,6 +662,7 @@ int main(int argc, char **argv)
>      qemu_init_main_loop(&error_fatal);
>      ctx = qemu_get_aio_context();
>      bdrv_init();
> +    module_call_init(MODULE_INIT_QOM);
>  
>      do {} while (g_main_context_iteration(NULL, false));
>  
> diff --git a/util/throttle.c b/util/throttle.c
> index b2a52b8b34..99fd73157b 100644
> --- a/util/throttle.c
> +++ b/util/throttle.c
> @@ -502,3 +502,154 @@ void throttle_account(ThrottleState *ts, bool is_write, 
> uint64_t size)
>      }
>  }
>  
> +/* return a ThrottleConfig based on the options in a ThrottleLimits
> + *
> + * @arg:    the ThrottleLimits object to read from
> + * @cfg:    the ThrottleConfig to edit
> + * @errp:   error object
> + */
> +void throttle_limits_to_config(ThrottleLimits *arg, ThrottleConfig *cfg,
> +                               Error **errp)
> +{
> +    if (arg->has_bps_total) {
> +        cfg->buckets[THROTTLE_BPS_TOTAL].avg = arg->bps_total;
> +    }
> +    if (arg->has_bps_read) {
> +        cfg->buckets[THROTTLE_BPS_READ].avg  = arg->bps_read;
> +    }
> +    if (arg->has_bps_write) {
> +        cfg->buckets[THROTTLE_BPS_WRITE].avg = arg->bps_write;
> +    }
> +
> +    if (arg->has_iops_total) {
> +        cfg->buckets[THROTTLE_OPS_TOTAL].avg = arg->iops_total;
> +    }
> +    if (arg->has_iops_read) {
> +        cfg->buckets[THROTTLE_OPS_READ].avg  = arg->iops_read;
> +    }
> +    if (arg->has_iops_write) {
> +        cfg->buckets[THROTTLE_OPS_WRITE].avg = arg->iops_write;
> +    }
> +
> +    if (arg->has_bps_total_max) {
> +        cfg->buckets[THROTTLE_BPS_TOTAL].max = arg->bps_total_max;
> +    }
> +    if (arg->has_bps_read_max) {
> +        cfg->buckets[THROTTLE_BPS_READ].max = arg->bps_read_max;
> +    }
> +    if (arg->has_bps_write_max) {
> +        cfg->buckets[THROTTLE_BPS_WRITE].max = arg->bps_write_max;
> +    }
> +    if (arg->has_iops_total_max) {
> +        cfg->buckets[THROTTLE_OPS_TOTAL].max = arg->iops_total_max;
> +    }
> +    if (arg->has_iops_read_max) {
> +        cfg->buckets[THROTTLE_OPS_READ].max = arg->iops_read_max;
> +    }
> +    if (arg->has_iops_write_max) {
> +        cfg->buckets[THROTTLE_OPS_WRITE].max = arg->iops_write_max;
> +    }
> +
> +    if (arg->has_bps_total_max_length) {
> +        if (arg->bps_total_max_length > UINT_MAX) {
> +            error_setg(errp, "bps-total-max-length value must be in"
> +                    " the range [0, %u]", UINT_MAX);
> +            return;
> +        }
> +        cfg->buckets[THROTTLE_BPS_TOTAL].burst_length = 
> arg->bps_total_max_length;
> +    }
> +    if (arg->has_bps_read_max_length) {
> +        if (arg->bps_read_max_length > UINT_MAX) {
> +            error_setg(errp, "bps-read-max-length value must be in"
> +                    " the range [0, %u]", UINT_MAX);
> +            return;
> +        }
> +        cfg->buckets[THROTTLE_BPS_READ].burst_length = 
> arg->bps_read_max_length;
> +    }
> +    if (arg->has_bps_write_max_length) {
> +        if (arg->bps_write_max_length > UINT_MAX) {
> +            error_setg(errp, "bps-write-max-length value must be in"
> +                    " the range [0, %u]", UINT_MAX);
> +            return;
> +        }
> +        cfg->buckets[THROTTLE_BPS_WRITE].burst_length = 
> arg->bps_write_max_length;
> +    }
> +    if (arg->has_iops_total_max_length) {
> +        if (arg->iops_total_max_length > UINT_MAX) {
> +            error_setg(errp, "iops-total-max-length value must be in"
> +                    " the range [0, %u]", UINT_MAX);
> +            return;
> +        }
> +        cfg->buckets[THROTTLE_OPS_TOTAL].burst_length = 
> arg->iops_total_max_length;
> +    }
> +    if (arg->has_iops_read_max_length) {
> +        if (arg->iops_read_max_length > UINT_MAX) {
> +            error_setg(errp, "iops-read-max-length value must be in"
> +                    " the range [0, %u]", UINT_MAX);
> +            return;
> +        }
> +        cfg->buckets[THROTTLE_OPS_READ].burst_length = 
> arg->iops_read_max_length;
> +    }
> +    if (arg->has_iops_write_max_length) {
> +        if (arg->iops_write_max_length > UINT_MAX) {
> +            error_setg(errp, "iops-write-max-length value must be in"
> +                    " the range [0, %u]", UINT_MAX);
> +            return;
> +        }
> +        cfg->buckets[THROTTLE_OPS_WRITE].burst_length = 
> arg->iops_write_max_length;
> +    }
> +
> +    if (arg->has_iops_size) {
> +        cfg->op_size = arg->iops_size;
> +    }
> +
> +    throttle_is_valid(cfg, errp);
> +}
> +
> +/* write the options of a ThrottleConfig to a ThrottleLimits
> + *
> + * @cfg:    the ThrottleConfig to read from
> + * @var:    the ThrottleLimits to write to
> + */
> +void throttle_config_to_throttle_limits(ThrottleConfig *cfg, ThrottleLimits 
> *var)
> +{
> +    var->bps_total               = cfg->buckets[THROTTLE_BPS_TOTAL].avg;
> +    var->bps_read                = cfg->buckets[THROTTLE_BPS_READ].avg;
> +    var->bps_write               = cfg->buckets[THROTTLE_BPS_WRITE].avg;
> +    var->iops_total              = cfg->buckets[THROTTLE_OPS_TOTAL].avg;
> +    var->iops_read               = cfg->buckets[THROTTLE_OPS_READ].avg;
> +    var->iops_write              = cfg->buckets[THROTTLE_OPS_WRITE].avg;
> +    var->bps_total_max           = cfg->buckets[THROTTLE_BPS_TOTAL].max;
> +    var->bps_read_max            = cfg->buckets[THROTTLE_BPS_READ].max;
> +    var->bps_write_max           = cfg->buckets[THROTTLE_BPS_WRITE].max;
> +    var->iops_total_max          = cfg->buckets[THROTTLE_OPS_TOTAL].max;
> +    var->iops_read_max           = cfg->buckets[THROTTLE_OPS_READ].max;
> +    var->iops_write_max          = cfg->buckets[THROTTLE_OPS_WRITE].max;
> +    var->bps_total_max_length    = 
> cfg->buckets[THROTTLE_BPS_TOTAL].burst_length;
> +    var->bps_read_max_length     = 
> cfg->buckets[THROTTLE_BPS_READ].burst_length;
> +    var->bps_write_max_length    = 
> cfg->buckets[THROTTLE_BPS_WRITE].burst_length;
> +    var->iops_total_max_length   = 
> cfg->buckets[THROTTLE_OPS_TOTAL].burst_length;
> +    var->iops_read_max_length    = 
> cfg->buckets[THROTTLE_OPS_READ].burst_length;
> +    var->iops_write_max_length   = 
> cfg->buckets[THROTTLE_OPS_WRITE].burst_length;
> +    var->iops_size               = cfg->op_size;
> +
> +    var->has_bps_total = true;
> +    var->has_bps_read = true;
> +    var->has_bps_write = true;
> +    var->has_iops_total = true;
> +    var->has_iops_read = true;
> +    var->has_iops_write = true;
> +    var->has_bps_total_max = true;
> +    var->has_bps_read_max = true;
> +    var->has_bps_write_max = true;
> +    var->has_iops_total_max = true;
> +    var->has_iops_read_max = true;
> +    var->has_iops_write_max = true;
> +    var->has_bps_read_max_length = true;
> +    var->has_bps_total_max_length = true;
> +    var->has_bps_write_max_length = true;
> +    var->has_iops_total_max_length = true;
> +    var->has_iops_read_max_length = true;
> +    var->has_iops_write_max_length = true;
> +    var->has_iops_size = true;
> +}
> -- 
> 2.11.0
>

signature.asc
Description: PGP signature