Re: [Qemu-devel] [PATCH V8 3/6] throttle: move out function to reuse the code

[Alberto Garcia]

On Mon 07 Aug 2017 02:37:35 PM CEST, Pradeep Jagadeesh wrote:
> This patch move out the throttle code to util/throttle.c to maximize
> the reusability of the code.The same code is also used by fsdev.
>
> Signed-off-by: Pradeep Jagadeesh <address@hidden>

> +    iothrottle = qapi_BlockIOThrottle_base(arg);
> +    throttle_set_io_limits(&cfg, iothrottle);

Since you only use it once, you can spare the definition of the local
iothrottle variable with

     throttle_set_io_limits(&cfg, qapi_BlockIOThrottle_base(arg));

but it's ok with me if you decide to leave it as it is now.

> --- a/include/qemu/throttle-options.h
> +++ b/include/qemu/throttle-options.h
> @@ -11,6 +11,7 @@
>  #define THROTTLE_OPTIONS_H
>  
>  #include "typedefs.h"
> +#include "qapi-types.h"
>  
>  #define THROTTLE_OPTS \
>            { \
> @@ -93,4 +94,6 @@
>  
>  void throttle_parse_options(ThrottleConfig *, QemuOpts *);
>  
> +void throttle_set_io_limits(ThrottleConfig *, IOThrottle *);
> +

Please remove the empty line between those two prototypes.

> +/* set the throttle limits

Or rather "Initialize a throttle config from an IOThrottle structure"

> + *
> + * @arg: iothrottle limits
> + * @cfg: throttle configuration
> + */
> +void throttle_set_io_limits(ThrottleConfig *cfg, IOThrottle *arg)
> +{

With those changes (the first one is optional),

Reviewed-by: Alberto Garcia <address@hidden>

Berto