Re: [Qemu-devel] [PATCH v11 1/4] block: add the blockio limits command line support

[Kevin Wolf]

Am 02.11.2011 07:01, schrieb Zhi Yong Wu:
> Signed-off-by: Zhi Yong Wu <address@hidden>
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> ---
>  block.c         |   40 ++++++++++++++++++++++++++++++++++++++++
>  block.h         |    4 ++++
>  block_int.h     |   29 +++++++++++++++++++++++++++++
>  blockdev.c      |   44 ++++++++++++++++++++++++++++++++++++++++++++
>  qemu-config.c   |   24 ++++++++++++++++++++++++
>  qemu-options.hx |    1 +
>  6 files changed, 142 insertions(+), 0 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 9bb236c..c70f86d 100644
> --- a/block.c
> +++ b/block.c
> @@ -30,6 +30,7 @@
>  #include "qjson.h"
>  #include "qemu-coroutine.h"
>  #include "qmp-commands.h"
> +#include "qemu-timer.h"
>  
>  #ifdef CONFIG_BSD
>  #include <sys/types.h>
> @@ -105,6 +106,37 @@ int is_windows_drive(const char *filename)
>  }
>  #endif
>  
> +/* throttling disk I/O limits */
> +static void bdrv_block_timer(void *opaque)
> +{
> +    BlockDriverState *bs = opaque;
> +
> +    qemu_co_queue_next(&bs->throttled_reqs);
> +}

This runs only one request, whereas in previous versions all queued
requests were resubmitted. Is this change intentional?

> diff --git a/block_int.h b/block_int.h
> index f4547f6..0abe843 100644
> --- a/block_int.h
> +++ b/block_int.h
> @@ -34,6 +34,12 @@
>  #define BLOCK_FLAG_ENCRYPT   1
>  #define BLOCK_FLAG_COMPAT6   4
>  
> +#define BLOCK_IO_LIMIT_READ     0
> +#define BLOCK_IO_LIMIT_WRITE    1
> +#define BLOCK_IO_LIMIT_TOTAL    2
> +
> +#define BLOCK_IO_SLICE_TIME     100000000
> +
>  #define BLOCK_OPT_SIZE          "size"
>  #define BLOCK_OPT_ENCRYPT       "encryption"
>  #define BLOCK_OPT_COMPAT6       "compat6"
> @@ -50,6 +56,16 @@ typedef struct AIOPool {
>      BlockDriverAIOCB *free_aiocb;
>  } AIOPool;
>  
> +typedef struct BlockIOLimit {
> +    int64_t bps[3];
> +    int64_t iops[3];
> +} BlockIOLimit;
> +
> +typedef struct BlockIODisp {
> +    uint64_t bytes[2];
> +    uint64_t ios[2];
> +} BlockIODisp;

I would prefer a different name. Something like BlockIOBaseValue maybe
(no, I don't really like this either, I'm open for suggestions)

Kevin