Re: [Qemu-devel] [PATCH v4 1/3] block: Support Archipelago as a QEMU block backend

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1864 bytes --]

On Mon, Jun 23, 2014 at 11:17:16AM +0300, Chrysostomos Nanakos wrote:
> On 06/20/2014 05:33 PM, Stefan Hajnoczi wrote:
> >On Thu, Jun 19, 2014 at 05:48:46PM +0300, Chrysostomos Nanakos wrote:
> >
> >>+                        if (!segreq->failed) {
> >>+                            reqdata->aio_cb->ret = segreq->count;
> >>+                            archipelago_finish_aiocb(reqdata);
> >>+                        }
> >What does segreq->failed mean?  We should always finish the I/O request,
> >otherwise the upper layers will run out of resources as we leak
> >failed requests.
> 
> Yes you are right.
> If a request fails while submitting it to Archipelago
> archipelago_aio_segmented_rw()
> will return -EIO to qemu_archipelago_aio_rw() which will return NULL to
> .bdrv_aio_readv/_write(). Now if all requests to Archipelago have succeeded
> in submission and one or all of them haven't been serviced (partial
> read/write) from Archipelago, archipelago_finish_aiocb() will fail the
> request. The last one wasn't implemented in this patch, v5 series has the
> appropriate changes.
> 
> Is this a proper and accepted approach along with the removal of the pipe
> code and the introduction of the QEMU "bottom-half" scheduled in
> archipelago_finish_aiocb()?

Sounds good.

Please also take a look at
http://wiki.qemu.org/Documentation/QemuIoTests

This is the test suite we use for QEMU block drivers.  It includes many
basic I/O tests and also some specialized tests for advanced features
like snapshotting.

For an example of how to add support, take a look at the commit which
added NFS support:
http://git.qemu.org/?p=qemu.git;a=commitdiff;h=170632dbc9f75217861dd8bf2e6da3c269a1ba18

Please do something similar so you can run the test suite like this:

  cd tests/qemu-iotests
  ./check -archipelago

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]