From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:53710) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QINFh-00069n-0E for qemu-devel@nongnu.org; Fri, 06 May 2011 11:47:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QINFg-0003gq-2y for qemu-devel@nongnu.org; Fri, 06 May 2011 11:47:36 -0400 Received: from mail-yx0-f173.google.com ([209.85.213.173]:57526) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QINFf-0003ge-S9 for qemu-devel@nongnu.org; Fri, 06 May 2011 11:47:36 -0400 Received: by yxk8 with SMTP id 8so1443319yxk.4 for ; Fri, 06 May 2011 08:47:35 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <4DC3F973.7020305@redhat.com> References: <1303910855-28999-1-git-send-email-stefanha@linux.vnet.ibm.com> <1303910855-28999-2-git-send-email-stefanha@linux.vnet.ibm.com> <4DBAA78B.3060706@redhat.com> <4DC3F973.7020305@redhat.com> Date: Fri, 6 May 2011 16:47:34 +0100 Message-ID: From: Stefan Hajnoczi Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 1/8] block: add bdrv_aio_stream List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Anthony Liguori , Stefan Hajnoczi , qemu-devel@nongnu.org On Fri, May 6, 2011 at 2:36 PM, Kevin Wolf wrote: > Am 06.05.2011 15:21, schrieb Stefan Hajnoczi: >> On Fri, Apr 29, 2011 at 12:56 PM, Kevin Wolf 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 wi= th 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 a= gain with >>>> + * an offset adjusted by the number of sectors previously streamed. >>>> + * >>>> + * This allows a user to progressive stream in an image at a pace tha= t makes >>>> + * sense. =A0In general, this function tries to do the smallest amoun= t 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. =A0If copy on read = is not >>>> + * enabled, a block format driver may return NULL. >>>> + */ >>>> +BlockDriverAIOCB *bdrv_aio_stream(BlockDriverState *bs, int64_t secto= r_num, >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0B= lockDriverCompletionFunc *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 th= e >>> 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. =A0The callback receives the nb_sectors. =A0So >> 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