On Fri, Apr 29, 2011 at 12:56 PM, Kevin Wolf <kw...@redhat.com> 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. 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? > I'm not sure how the caller would know how many sectors have been > copied. A BlockDriverCompletionFunc usually returns 0 on success, did > you change it here to use positive numbers for something else? At least > this must be documented somewhere, but I would suggest to add a > nb_sectors argument instead so that the caller decides how many sectors > to copy. Yes, I agree that a separate nb_sectors argument would be clearer. > If you say that it only makes sense with copy on read, should one think > of it as a read that throws the read data away? I think considering it a > copy function makes more sense and is independent of copy on read. I actually think the copy-on-read statement is an implementation detail. I can imagine doing essentially the same behavior without exposing copy on read to the user. But in QED streaming is based on copy-on-read. Let's remove this comment. Stefan