Re: [Qemu-devel] [PATCH v2 1/1] The codes V2 for QEMU disk I/O limits.

[Stefan Hajnoczi]

On Thu, Jul 28, 2011 at 9:20 AM, Stefan Hajnoczi <address@hidden> wrote:
> On Thu, Jul 28, 2011 at 6:43 AM, Zhi Yong Wu <address@hidden> wrote:
>> On Wed, Jul 27, 2011 at 8:58 PM, Stefan Hajnoczi <address@hidden> wrote:
>>> On Wed, Jul 27, 2011 at 11:17 AM, Zhi Yong Wu <address@hidden> wrote:
>>>> On Wed, Jul 27, 2011 at 3:26 AM, Marcelo Tosatti <address@hidden> wrote:
>>>>> On Tue, Jul 26, 2011 at 04:59:06PM +0800, Zhi Yong Wu wrote:
>>>>>> Welcome to give me your comments, thanks.
>>>>>>
>>>>>> Signed-off-by: Zhi Yong Wu <address@hidden>
>>>>>> ---
>>>>>>  Makefile.objs     |    2 +-
>>>>>>  block.c           |  288 
>>>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++--
>>>>>>  block.h           |    1 -
>>>>>>  block/blk-queue.c |  116 +++++++++++++++++++++
>>>>>>  block/blk-queue.h |   70 +++++++++++++
>>>>>>  block_int.h       |   28 +++++
>>>>>>  blockdev.c        |   21 ++++
>>>>>>  qemu-config.c     |   24 +++++
>>>>>>  qemu-option.c     |   17 +++
>>>>>>  qemu-option.h     |    1 +
>>>>>>  qemu-options.hx   |    1 +
>>>>>>  11 files changed, 559 insertions(+), 10 deletions(-)
>>>>>>  create mode 100644 block/blk-queue.c
>>>>>>  create mode 100644 block/blk-queue.h
>>>>>>
>>>>>> diff --git a/Makefile.objs b/Makefile.objs
>>>>>> index 9f99ed4..06f2033 100644
>>>>>> --- a/Makefile.objs
>>>>>> +++ b/Makefile.objs
>>>>>> @@ -23,7 +23,7 @@ block-nested-y += raw.o cow.o qcow.o vdi.o vmdk.o 
>>>>>> cloop.o dmg.o bochs.o vpc.o vv
>>>>>>  block-nested-y += qcow2.o qcow2-refcount.o qcow2-cluster.o 
>>>>>> qcow2-snapshot.o qcow2-cache.o
>>>>>>  block-nested-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o 
>>>>>> qed-cluster.o
>>>>>>  block-nested-y += qed-check.o
>>>>>> -block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
>>>>>> +block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o 
>>>>>> blk-queue.o
>>>>>>  block-nested-$(CONFIG_WIN32) += raw-win32.o
>>>>>>  block-nested-$(CONFIG_POSIX) += raw-posix.o
>>>>>>  block-nested-$(CONFIG_CURL) += curl.o
>>>>>> diff --git a/block.c b/block.c
>>>>>> index 24a25d5..e54e59c 100644
>>>>>> --- a/block.c
>>>>>> +++ b/block.c
>>>>>> @@ -29,6 +29,9 @@
>>>>>>  #include "module.h"
>>>>>>  #include "qemu-objects.h"
>>>>>>
>>>>>> +#include "qemu-timer.h"
>>>>>> +#include "block/blk-queue.h"
>>>>>> +
>>>>>>  #ifdef CONFIG_BSD
>>>>>>  #include <sys/types.h>
>>>>>>  #include <sys/stat.h>
>>>>>> @@ -58,6 +61,13 @@ static int bdrv_read_em(BlockDriverState *bs, int64_t 
>>>>>> sector_num,
>>>>>>  static int bdrv_write_em(BlockDriverState *bs, int64_t sector_num,
>>>>>>                           const uint8_t *buf, int nb_sectors);
>>>>>>
>>>>>> +static bool bdrv_exceed_bps_limits(BlockDriverState *bs, int nb_sectors,
>>>>>> +        bool is_write, double elapsed_time, uint64_t *wait);
>>>>>> +static bool bdrv_exceed_iops_limits(BlockDriverState *bs, bool is_write,
>>>>>> +        double elapsed_time, uint64_t *wait);
>>>>>> +static bool bdrv_exceed_io_limits(BlockDriverState *bs, int nb_sectors,
>>>>>> +        bool is_write, uint64_t *wait);
>>>>>> +
>>>>>>  static QTAILQ_HEAD(, BlockDriverState) bdrv_states =
>>>>>>      QTAILQ_HEAD_INITIALIZER(bdrv_states);
>>>>>>
>>>>>> @@ -90,6 +100,20 @@ int is_windows_drive(const char *filename)
>>>>>>  }
>>>>>>  #endif
>>>>>>
>>>>>> +static int bdrv_io_limits_enable(BlockIOLimit *io_limits)
>>>>>> +{
>>>>>> +    if ((io_limits->bps[0] == 0)
>>>>>> +         && (io_limits->bps[1] == 0)
>>>>>> +         && (io_limits->bps[2] == 0)
>>>>>> +         && (io_limits->iops[0] == 0)
>>>>>> +         && (io_limits->iops[1] == 0)
>>>>>> +         && (io_limits->iops[2] == 0)) {
>>>>>> +        return 0;
>>>>>> +    }
>>>>>> +
>>>>>> +    return 1;
>>>>>> +}
>>>>>> +
>>>>>>  /* check if the path starts with "<protocol>:" */
>>>>>>  static int path_has_protocol(const char *path)
>>>>>>  {
>>>>>> @@ -167,6 +191,28 @@ void path_combine(char *dest, int dest_size,
>>>>>>      }
>>>>>>  }
>>>>>>
>>>>>> +static void bdrv_block_timer(void *opaque)
>>>>>> +{
>>>>>> +    BlockDriverState *bs = opaque;
>>>>>> +    BlockQueue *queue = bs->block_queue;
>>>>>> +
>>>>>> +    while (!QTAILQ_EMPTY(&queue->requests)) {
>>>>>> +        BlockIORequest *request;
>>>>>> +        int ret;
>>>>>> +
>>>>>> +        request = QTAILQ_FIRST(&queue->requests);
>>>>>> +        QTAILQ_REMOVE(&queue->requests, request, entry);
>>>>>> +
>>>>>> +        ret = qemu_block_queue_handler(request);
>>>>>> +        if (ret == 0) {
>>>>>> +            QTAILQ_INSERT_HEAD(&queue->requests, request, entry);
>>>>>> +            break;
>>>>>> +        }
>>>>>> +
>>>>>> +        qemu_free(request);
>>>>>> +    }
>>>>>> +}
>>>>>> +
>>>>>>  void bdrv_register(BlockDriver *bdrv)
>>>>>>  {
>>>>>>      if (!bdrv->bdrv_aio_readv) {
>>>>>> @@ -642,6 +688,15 @@ int bdrv_open(BlockDriverState *bs, const char 
>>>>>> *filename, int flags,
>>>>>>              bs->change_cb(bs->change_opaque, CHANGE_MEDIA);
>>>>>>      }
>>>>>>
>>>>>> +    /* throttling disk I/O limits */
>>>>>> +    if (bdrv_io_limits_enable(&bs->io_limits)) {
>>>>>> +        bs->block_queue = qemu_new_block_queue();
>>>>>> +        bs->block_timer = qemu_new_timer_ns(vm_clock, bdrv_block_timer, 
>>>>>> bs);
>>>>>> +
>>>>>> +        bs->slice_start[0] = qemu_get_clock_ns(vm_clock);
>>>>>> +        bs->slice_start[1] = qemu_get_clock_ns(vm_clock);
>>>>>> +    }
>>>>>> +
>>>>>
>>>>> It should be possible to tune the limits on the flight, please introduce
>>>>> QMP commands for that.
>>>> Yeah, I am working on this.
>>>
>>> It's worth mentioning that the I/O limits commands can use Supriya's
>>> new block_set command for changing block device parameters at runtime.
>>>  So I think the runtime limits changing can be a separate patch.
>> Since I/O limits commands will depend on Supriya's block_set patch,
>> when will her patch be merged into qemu.git?
>
> The block_set patch is on the mailing list with an ongoing discussion.
>  Until agreement is reached it will not be merged - hard to tell when
> exactly that will be, but hopefully over the next few days.
>
> You can always add temporary QMP/HMP commands in order to test setting
> limits at runtime.  It should be easy to switch to a different QMP/HMP
> command later, if necessary.

Just to clarify: adding temporary QMP/HMP commands in your personal
git repo can be useful for development and testing.  It will let you
continue fleshing out things like block device hotplug.  Of course
qemu.git can only merge commands that we intend to support for the
future, so a final QMP/HMP interface needs to be written at some point
but it does not stop you from making progress.

Stefan