qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH v7 5/6] block: add throttle block filter driver


From: Alberto Garcia
Subject: Re: [Qemu-block] [PATCH v7 5/6] block: add throttle block filter driver
Date: Tue, 22 Aug 2017 16:16:26 +0200
User-agent: Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (i586-pc-linux-gnu)

On Tue 22 Aug 2017 12:15:34 PM CEST, Manos Pitsidianakis wrote:
>  ##
> +# @BlockdevOptionsThrottle:
> +#
> +# Driver specific block device options for the throttle driver
> +#
> +# @throttle-group:   the name of the throttle-group object to use. It
> +#                    must already exist.
> +# @file:             reference to or definition of the data source block 
> device
> +# Since: 2.11
> +##
> +{ 'struct': 'BlockdevOptionsThrottle',
> +  'data': { '*throttle-group': 'str',
> +            'file' : 'BlockdevRef'
> +             } }
> +##

I guess throttle-group is not optional here anymore ?

> @@ -548,6 +548,11 @@ void throttle_group_unregister_tgm(ThrottleGroupMember 
> *tgm)
>      ThrottleGroupMember *token;
>      int i;
>  
> +    if (!ts) {
> +        /* Discard uninitialized tgm */
> +        return;
> +    }
> +

Is this change needed in case throttle_configure_tgm() fails?

> +static int throttle_configure_tgm(BlockDriverState *bs,
> +                                  ThrottleGroupMember *tgm,
> +                                  QDict *options, Error **errp)
> +{
> +    int ret;
> +    const char *group_name;
> +    Error *local_err = NULL;
> +    QemuOpts *opts = qemu_opts_create(&throttle_opts, NULL, 0, &error_abort);
> +
> +    qemu_opts_absorb_qdict(opts, options, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        ret = -EINVAL;
> +        goto fin;
> +    }
> +
> +    group_name = qemu_opt_get(opts, QEMU_OPT_THROTTLE_GROUP_NAME);
> +    if (!group_name) {
> +        error_setg(errp, "Please specify a throttle group.");
> +        ret = EINVAL;
> +        goto fin;
> +    } else if (!throttle_group_exists(group_name)) {
> +        error_setg(errp, "Throttle group '%s' does not exist.", group_name);
> +        ret = EINVAL;
> +        goto fin;
> +    }

It should be -EINVAL (negative value) in both cases.

Berto



reply via email to

[Prev in Thread] Current Thread [Next in Thread]