[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/8] block: add bdrv_aio_stream
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH 1/8] block: add bdrv_aio_stream |
Date: |
Fri, 6 May 2011 16:47:34 +0100 |
On Fri, May 6, 2011 at 2:36 PM, Kevin Wolf <address@hidden> wrote:
> Am 06.05.2011 15:21, schrieb Stefan Hajnoczi:
>> On Fri, Apr 29, 2011 at 12:56 PM, Kevin Wolf <address@hidden> wrote:
>>> Am 27.04.2011 15:27, schrieb Stefan Hajnoczi:
>>>> +/**
>>>> + * Attempt to stream an image starting from sector_num.
>>>> + *
>>>> + * @sector_num - the first sector to start streaming from
>>>> + * @cb - block completion callback
>>>> + * @opaque - data to pass completion callback
>>>> + *
>>>> + * Returns NULL if the image format not support streaming, the image is
>>>> + * read-only, or no image is open.
>>>> + *
>>>> + * The intention of this function is for a user to execute it once with a
>>>> + * sector_num of 0 and then upon receiving a completion callback, to
>>>> remember
>>>> + * the number of sectors "streamed", and then to call this function again
>>>> with
>>>> + * an offset adjusted by the number of sectors previously streamed.
>>>> + *
>>>> + * This allows a user to progressive stream in an image at a pace that
>>>> makes
>>>> + * sense. In general, this function tries to do the smallest amount of
>>>> I/O
>>>> + * possible to do some useful work.
>>>> + *
>>>> + * This function only really makes sense in combination with a block
>>>> format
>>>> + * that supports copy on read and has it enabled. If copy on read is not
>>>> + * enabled, a block format driver may return NULL.
>>>> + */
>>>> +BlockDriverAIOCB *bdrv_aio_stream(BlockDriverState *bs, int64_t
>>>> sector_num,
>>>> + BlockDriverCompletionFunc *cb, void
>>>> *opaque)
>>>
>>> I think bdrv_aio_stream is a bad name for this. It only becomes image
>>> streaming because the caller repeatedly calls this function. What the
>>> function really does is copying some data from the backing file into the
>>> overlay image.
>>
>> That's true but bdrv_aio_copy_from_backing_file() is a bit long.
>
> bdrv_copy_backing() or something should be short enough and still
> describes what it's really doing.
>
>> The
>> special thing about this operation is that it takes a starting
>> sector_num but no length. The callback receives the nb_sectors. So
>> this operation isn't an ordinary [start, length) copy either so
>> bdrv_aio_stream() isn't that bad?
>
> Well, you're going to introduce nb_sectors anyway, so it's not really
> special any more.
I guess you're right. First I wasn't planning on passing nb_sectors
to this function since its the blockdev.c streaming loop that drives
the streaming process - we may not need nb_sectors here. But I guess
this is like a read(2) function and the block driver can return short
reads if that is convenient due to cluster sizes or other image format
internals. By passing in nb_sectors we avoid streaming too much at
the end.
Stefan