From: Kevin Wolf <kwolf@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org, Nicholas Thomas <nick@bytemark.co.uk>
Subject: Re: [Qemu-devel] [PATCH 00/12] nbd improvements
Date: Fri, 09 Sep 2011 12:42:57 +0200 [thread overview]
Message-ID: <4E69EDB1.8020000@redhat.com> (raw)
In-Reply-To: <4E69EA89.6090706@redhat.com>
Am 09.09.2011 12:29, schrieb Paolo Bonzini:
> On 09/09/2011 11:00 AM, Kevin Wolf wrote:
>> There is anonther patch enabling AIO for NBD on the list [1], by
>> Nicholas Thomas (CCed), that lacked review so far. Can you guys please
>> review each others approach and then converge on a solution? I guess
>> Paolo's patches 1-7 can be applied in any case, probably causing minor
>> conflicts, but for the rest we need to decide which one to pick.
>
> Stefan also pointed me to Nicholas's patches yesterday. I would go with
> mine, if only because his work predates coroutines (at least the older
> versions) and are much more complex.
>
> On the other hand, Nicholas's work includes timeout and reconnect. I'm
> not sure how complicated it is to include it in my series, but probably
> not much. With coroutines, preserving the list of outstanding I/O
> requests is done implicitly by the CoMutex, so you basically have to
> check errno for ECONNRESET and similar errors, reconnect, and retry
> issuing the current request only.
>
> Timeout can be done with a QEMUTimer that shuts down the socket
> (shutdown(2) I mean). This triggers an EPIPE when writing, or a
> zero-sized read when reading. The timeout can be set every time the
> coroutine is (re)entered, and reset before exiting nbd_co_readv/writev.
>
> What do you think?
I haven't really had a look at your patches yet (I hope to do so later
today), but from the patch descriptions I would think that indeed your
patches could be the better base for a converged series.
What I would like you and Nick to do is to see what is missing from your
series compared to his one (you describe a couple of things, but let's
see if Nick knows anything else) and to agree on the next steps. I think
that a possible way is to merge your series and do the other
improvements on top, but as I said I haven't really looked into the
details yet.
Kevin
next prev parent reply other threads:[~2011-09-09 10:40 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-08 15:24 [Qemu-devel] [PATCH 00/12] nbd improvements Paolo Bonzini
2011-09-08 15:24 ` [Qemu-devel] [PATCH 01/12] nbd: support feature negotiation Paolo Bonzini
2011-09-08 15:24 ` [Qemu-devel] [PATCH 02/12] nbd: sync API definitions with upstream Paolo Bonzini
2011-09-12 14:15 ` Kevin Wolf
2011-09-12 15:08 ` Paolo Bonzini
2011-09-08 15:24 ` [Qemu-devel] [PATCH 03/12] nbd: support NBD_SET_FLAGS ioctl Paolo Bonzini
2011-09-08 15:24 ` [Qemu-devel] [PATCH 04/12] nbd: add support for NBD_CMD_FLUSH Paolo Bonzini
2011-09-13 13:52 ` Kevin Wolf
2011-09-13 15:13 ` Paolo Bonzini
2011-09-08 15:24 ` [Qemu-devel] [PATCH 05/12] nbd: add support for NBD_CMD_FLAG_FUA Paolo Bonzini
2011-09-13 13:55 ` Kevin Wolf
2011-09-08 15:24 ` [Qemu-devel] [PATCH 06/12] nbd: support NBD_CMD_TRIM in the server Paolo Bonzini
2011-09-13 13:58 ` Kevin Wolf
2011-09-13 15:14 ` Paolo Bonzini
2011-09-14 15:44 ` Christoph Hellwig
2011-09-14 16:25 ` Paolo Bonzini
2011-09-08 15:25 ` [Qemu-devel] [PATCH 07/12] sheepdog: add coroutine_fn markers Paolo Bonzini
2011-09-08 15:25 ` [Qemu-devel] [PATCH 08/12] add socket_set_block Paolo Bonzini
2011-09-08 15:25 ` [Qemu-devel] [PATCH 09/12] sheepdog: move coroutine send/recv function to generic code Paolo Bonzini
2011-09-09 4:53 ` MORITA Kazutaka
2011-09-09 8:11 ` [Qemu-devel] [PATCH v2 " Paolo Bonzini
2011-09-13 0:28 ` MORITA Kazutaka
2011-09-13 14:14 ` Kevin Wolf
2011-09-13 15:16 ` Paolo Bonzini
2011-09-13 15:36 ` Kevin Wolf
2011-09-13 15:38 ` Paolo Bonzini
2011-09-08 15:25 ` [Qemu-devel] [PATCH 10/12] block: add bdrv_co_flush support Paolo Bonzini
2011-09-08 15:25 ` [Qemu-devel] [PATCH 11/12] nbd: switch to asynchronous operation Paolo Bonzini
2011-09-09 14:52 ` Nicholas Thomas
2011-09-09 15:03 ` Paolo Bonzini
2011-09-09 15:34 ` Paolo Bonzini
2011-09-08 15:25 ` [Qemu-devel] [PATCH 12/12] nbd: split requests Paolo Bonzini
2011-09-09 14:52 ` Nicholas Thomas
2011-09-09 15:33 ` Paolo Bonzini
2011-09-09 9:00 ` [Qemu-devel] [PATCH 00/12] nbd improvements Kevin Wolf
2011-09-09 10:29 ` Paolo Bonzini
2011-09-09 10:42 ` Kevin Wolf [this message]
2011-09-09 10:50 ` Nicholas Thomas
2011-09-09 11:00 ` Paolo Bonzini
2011-09-09 11:04 ` Kevin Wolf
2011-09-09 14:51 ` Nicholas Thomas
2011-09-14 9:50 ` Kevin Wolf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4E69EDB1.8020000@redhat.com \
--to=kwolf@redhat.com \
--cc=nick@bytemark.co.uk \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.