On Tue, Sep 27, 2022 at 08:04:11AM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé writes:
> > On Mon, Sep 26, 2022 at 12:59:40PM +0300, Denis Plotnikov wrote:
> >> Example of result:
> >>
> >> ./qemu/scripts/qmp/qmp-shell /tmp/qmp.socket
> >>
> >> (QEMU) query-status
> >>
On Tue, Sep 20, 2022 at 06:10:08PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> On 9/20/22 17:47, Markus Armbruster wrote:
> > Vladimir Sementsov-Ogievskiy writes:
> >
> > > For now we only log the vhost device error, when virtqueue is actually
> > > stopped. Let's add a QAPI event, which makes
On Thu, Jul 21, 2022 at 05:05:38PM +0100, Mark Cave-Ayland wrote:
> On 21/07/2022 16:56, Daniel P. Berrangé wrote:
>
> > On Thu, Jul 21, 2022 at 04:51:51PM +0100, Mark Cave-Ayland wrote:
> > > On 21/07/2022 15:28, Roman Kagan wrote:
> > >
> > > (lots cut)
On Wed, Jul 20, 2022 at 02:21:38PM +0100, Mark Cave-Ayland wrote:
> On 20/07/2022 12:00, Roman Kagan wrote:
>
> > On Wed, Jul 20, 2022 at 11:44:26AM +0100, Daniel P. Berrangé wrote:
> > > On Wed, Jul 20, 2022 at 01:25:55PM +0300, Roman Kagan wrote:
> > > > It
On Wed, Jul 20, 2022 at 12:04:58PM +0100, Daniel P. Berrangé wrote:
> On Wed, Jul 20, 2022 at 02:00:16PM +0300, Roman Kagan wrote:
> > On Wed, Jul 20, 2022 at 11:44:26AM +0100, Daniel P. Berrangé wrote:
> > > On Wed, Jul 20, 2022 at 01:25:55PM +0300, Roman Kagan wrote:
>
On Wed, Jul 20, 2022 at 11:44:26AM +0100, Daniel P. Berrangé wrote:
> On Wed, Jul 20, 2022 at 01:25:55PM +0300, Roman Kagan wrote:
> > It's possible to create non-working configurations by attaching a device
> > to a derivative of PCIe slot (pcie-root-port, ioh3420, etc) and
> &
) slot #4; for the latter, only a positive
testcase for slot #4 is included.
Signed-off-by: Roman Kagan
---
v2 -> v3:
- do not use qtest-single stuff [Thomas]
v1 -> v2:
- use object_dynamic_cast (without assert) [Vladimir]
- add explaining comment [Michael]
- add tests
hw/pci/pci_br
On Tue, Jul 19, 2022 at 12:42:47PM +0200, Thomas Huth wrote:
> On 19/07/2022 10.01, Roman Kagan wrote:
> > +#include "qemu/osdep.h"
> > +#include "libqtest-single.h"
>
> Do you really need libqtest-single.h here? libqtest.h should be enough,
> shouldn
) slot #4; for the latter, only a positive
testcase for slot #4 is included.
Signed-off-by: Roman Kagan
---
v1 -> v2:
- use object_dynamic_cast (without assert) [Vladimir]
- add explaining comment [Michael]
- add tests
(I've only had a chance to run tests against x86; hope I didn't mess
them
On Thu, Jul 07, 2022 at 01:19:18AM -0400, Michael S. Tsirkin wrote:
> On Wed, Jul 06, 2022 at 10:43:12PM +0300, Roman Kagan wrote:
> > On Wed, Jul 06, 2022 at 09:38:39PM +0300, Vladimir Sementsov-Ogievskiy
> > wrote:
> > > On 7/4/22 13:25, Roman Kagan wrote:
> > &
On Wed, Jul 06, 2022 at 09:38:39PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> On 7/4/22 13:25, Roman Kagan wrote:
> > It's possible to create non-working configurations by attaching a device
> > to a derivative of PCIe slot (pcie-root-port, ioh3420, etc) and
> > specifyin
such configurations and only allow addr=0 on the
secondary bus of a PCIe slot.
Signed-off-by: Roman Kagan
---
hw/pci/pci_bridge.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
index da34c8ebcd..8b38d5ad3d 100644
--- a/hw/pci/pci_bridge.c
+++ b/hw/pci
+
> 2 files changed, 38 insertions(+)
Reviewed-by: Roman Kagan
;
> Signed-off-by: Konstantin Khlebnikov
As long as you pick this series over from Konstantin you need to append
your s-o-b.
Other than that,
Reviewed-by: Roman Kagan
On Tue, Jun 21, 2022 at 01:55:25PM +0200, Markus Armbruster wrote:
> Roman Kagan writes:
>
> > On Mon, May 30, 2022 at 06:04:32PM +0300, Roman Kagan wrote:
> >> On Mon, May 30, 2022 at 01:28:17PM +0200, Markus Armbruster wrote:
> >> > Roman Kagan writes:
>
On Mon, May 30, 2022 at 06:04:32PM +0300, Roman Kagan wrote:
> On Mon, May 30, 2022 at 01:28:17PM +0200, Markus Armbruster wrote:
> > Roman Kagan writes:
> >
> > > On Wed, May 25, 2022 at 12:54:47PM +0200, Markus Armbruster wrote:
> > >> Konstantin Khlebnikov
On Tue, May 24, 2022 at 01:30:54PM -0400, Emanuele Giuseppe Esposito wrote:
> It seems that aio_wait_kick always required a memory barrier
> or atomic operation in the caller, but nobody actually
> took care of doing it.
>
> Let's put the barrier in the function instead, and pair it
> with
On Mon, May 30, 2022 at 01:28:17PM +0200, Markus Armbruster wrote:
> Roman Kagan writes:
>
> > On Wed, May 25, 2022 at 12:54:47PM +0200, Markus Armbruster wrote:
> >> Konstantin Khlebnikov writes:
> >>
> >> > This event represents device runtime errors
On Wed, May 25, 2022 at 12:54:47PM +0200, Markus Armbruster wrote:
> Konstantin Khlebnikov writes:
>
> > This event represents device runtime errors to give time and
> > reason why device is broken.
>
> Can you give an or more examples of the "device runtime errors" you have
> in mind?
On Sun, Nov 28, 2021 at 04:47:20PM -0500, Michael S. Tsirkin wrote:
> On Thu, Nov 11, 2021 at 06:33:44PM +0300, Roman Kagan wrote:
> > Error propagation between the generic vhost code and the specific backends
> > is
> > not quite consistent: some places follow &qu
On Wed, Nov 17, 2021 at 11:13:27PM +0500, Valentin Sinitsyn wrote:
> On 17.11.2021 19:46, Daniil Tatianin wrote:
> > -/*
> > - * AMDVi event structure
> > - *0:15 -> DeviceID
> > - *55:63 -> event type + miscellaneous info
> > - *63:127 -> related address
> Did you mean 64:127?
On Fri, Nov 12, 2021 at 09:32:31PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Add Den and Roman (his new address)
Thanks, I missed it on the list indeed.
> 06.11.2021 16:41, Philippe Mathieu-Daudé wrote:
> > This is the 4th time I send this patch. Is the VMBus infrastructure
> > used /
On Fri, Nov 12, 2021 at 12:37:59PM +0100, Kevin Wolf wrote:
> Am 12.11.2021 um 08:39 hat Roman Kagan geschrieben:
> > On Thu, Nov 11, 2021 at 06:52:30PM +0100, Kevin Wolf wrote:
> > > Am 11.11.2021 um 16:33 hat Roman Kagan geschrieben:
> > > > vhost-user-blk real
On Fri, Nov 12, 2021 at 12:24:06PM +0400, Marc-André Lureau wrote:
> Hi
>
> On Thu, Nov 11, 2021 at 7:44 PM Roman Kagan wrote:
>
> > As its name suggests, ChardevClass.chr_sync_read is supposed to do a
> > blocking read. The only implementation of it, tcp_chr
On Fri, Nov 12, 2021 at 09:56:17AM +, Daniel P. Berrangé wrote:
> On Fri, Nov 12, 2021 at 10:46:46AM +0300, Roman Kagan wrote:
> > On Thu, Nov 11, 2021 at 06:59:43PM +0100, Philippe Mathieu-Daudé wrote:
> > > On 11/11/21 16:33, Roman Kagan wrote:
> > > > Fi
On Thu, Nov 11, 2021 at 03:14:56PM -0500, Michael S. Tsirkin wrote:
> On Thu, Nov 11, 2021 at 06:33:44PM +0300, Roman Kagan wrote:
> > Error propagation between the generic vhost code and the specific backends
> > is
> > not quite consistent: some places follow &qu
On Thu, Nov 11, 2021 at 06:59:43PM +0100, Philippe Mathieu-Daudé wrote:
> On 11/11/21 16:33, Roman Kagan wrote:
> > Fix the (hypothetical) potential problem when the value parsed out of
> > the vhost module parameter in sysfs overflows the return value from
> > vhost_
On Thu, Nov 11, 2021 at 06:52:30PM +0100, Kevin Wolf wrote:
> Am 11.11.2021 um 16:33 hat Roman Kagan geschrieben:
> > vhost-user-blk realize only attempts to reconnect if the previous
> > connection attempt failed on "a problem with the connection and not an
> > error rel
is ok, instead of taking recovery actions
(break and reestablish the vhost-user connection, cancel migration, etc)
before it's too late.
To fix this, consolidate on the convention to return negated errno on
failures throughout generic vhost, and use it for error propagation.
Signed-off-by: Roman
it in qemu_chr_fe_read_all; instead place an assertion
that it doesn't fail with EAGAIN.
Signed-off-by: Roman Kagan
---
chardev/char-fe.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index 7789f7be9c..f94efe928e 100644
--- a/chardev/char
the error codes
wherever it's appropriate.
Signed-off-by: Roman Kagan
---
hw/virtio/vhost-vdpa.c | 37 +++--
1 file changed, 23 insertions(+), 14 deletions(-)
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 0d8051426c..a3b885902a 100644
--- a/hw
Fix the only callsite that doesn't propagate the error code from the
generic vhost code.
Signed-off-by: Roman Kagan
---
hw/block/vhost-user-blk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index f9b17f6813
return convention with the other
two vhost backends, kernel and vdpa, and will therefore allow for
consistent error propagation in the generic vhost code (in a followup
patch).
Signed-off-by: Roman Kagan
---
hw/virtio/vhost-user.c | 401 +++--
1 file changed, 223
tcp_chr_recv communicates the specific error condition to the caller via
errno. However, after setting it, it may call into some system calls or
library functions which can clobber the errno.
Avoid this by moving the errno assignment to the end of the function.
Signed-off-by: Roman Kagan
Fix the (hypothetical) potential problem when the value parsed out of
the vhost module parameter in sysfs overflows the return value from
vhost_kernel_memslots_limit.
Signed-off-by: Roman Kagan
---
hw/virtio/vhost-backend.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw
trigger an immediate connection drop and reconnection, leaving it in a
broken state.
Rework error propagation to always return negated errno on errors and
correctly pass it up the stack.
Roman Kagan (10):
vhost-user-blk: reconnect on any error during realize
chardev/char-socket: tcp_chr_recv:
This patch amends a527e312b5 "vhost-user-blk: Implement reconnection
during realize".
Signed-off-by: Roman Kagan
---
hw/block/vhost-user-blk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index ba13cb87e
After the return from tcp_chr_recv, tcp_chr_sync_read calls into a
function which eventually makes a system call and may clobber errno.
Make a copy of errno right after tcp_chr_recv and restore the errno on
return from tcp_chr_sync_read.
Signed-off-by: Roman Kagan
---
chardev/char-socket.c | 3
Almost all VhostOps methods in kernel_ops follow the convention of
returning negated errno on error.
Adjust the only one that doesn't.
Signed-off-by: Roman Kagan
---
hw/virtio/vhost-backend.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/virtio/vhost-backend.c b/hw
On Mon, Oct 04, 2021 at 11:11:00AM -0400, Michael S. Tsirkin wrote:
> On Mon, Oct 04, 2021 at 06:07:29PM +0300, Denis Plotnikov wrote:
> > It might be useful for the cases when a slow block layer should be replaced
> > with a more performant one on running VM without stopping, i.e. with very
> >
On Thu, May 13, 2021 at 11:04:37PM +0200, Paolo Bonzini wrote:
> On 12/05/21 09:15, Vladimir Sementsov-Ogievskiy wrote:
> > > >
> > >
> > > I don't understand. Why doesn't aio_co_enter go through the ctx !=
> > > qemu_get_current_aio_context() branch and just do aio_co_schedule?
> > > That was
On Mon, Apr 19, 2021 at 10:34:49AM +0100, Daniel P. Berrangé wrote:
> On Fri, Apr 16, 2021 at 11:08:59AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> > Detecting monitor by current coroutine works bad when we are not in
> > coroutine context. And that's exactly so in nbd reconnect code, where
> >
s(+), 42 deletions(-)
Reviewed-by: Roman Kagan
On Fri, Apr 16, 2021 at 11:08:56AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Now, when thread can do negotiation and retry, it may run relatively
> long. We need a mechanism to stop it, when user is not interested in
> result anymore. So, on nbd_client_connection_release() let's shutdown
> the
On Fri, Apr 16, 2021 at 11:08:55AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Add an option for thread to retry connection until success. We'll use
> nbd/client-connection both for reconnect and for initial connection in
> nbd_open(), so we need a possibility to use same NBDClientConnection
>
On Fri, Apr 16, 2021 at 11:08:54AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Add arguments and logic to support nbd negotiation in the same thread
> after successful connection.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> include/block/nbd.h | 9 +++-
> block/nbd.c
On Fri, Apr 16, 2021 at 11:08:53AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> nbd/client-connection.c | 94 ++---
> 1 file changed, 42 insertions(+), 52 deletions(-)
>
> diff --git
On Fri, Apr 16, 2021 at 11:08:52AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> We now have bs-independent connection API, which consists of four
> functions:
>
> nbd_client_connection_new()
> nbd_client_connection_unref()
> nbd_co_establish_connection()
>
On Fri, Apr 16, 2021 at 11:08:51AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> block/nbd.c | 43 ++-
> 1 file changed, 26 insertions(+), 17 deletions(-)
>
> diff --git a/block/nbd.c b/block/nbd.c
>
nged, 68 insertions(+), 69 deletions(-)
Reviewed-by: Roman Kagan
On Fri, Apr 16, 2021 at 11:08:46AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> We don't need all these states. The code refactored to use two boolean
> variables looks simpler.
Indeed.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> block/nbd.c | 125
On Fri, Apr 16, 2021 at 11:08:45AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Instead of connect_bh, bh_ctx and wait_connect fields we can live with
> only one link to waiting coroutine, protected by mutex.
>
> So new logic is:
>
> nbd_co_establish_connection() sets wait_co under mutex,
On Fri, Apr 16, 2021 at 11:08:44AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> With the following patch we want to call wake coroutine from thread.
> And it doesn't work with aio_co_wake:
> Assume we have no iothreads.
> Assume we have a coroutine A, which waits in the yield point for
> external
On Fri, Apr 16, 2021 at 11:08:42AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> block/nbd.c | 2 ++
> 1 file changed, 2 insertions(+)
Reviewed-by: Roman Kagan
On Thu, Apr 22, 2021 at 01:27:22AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 21.04.2021 17:00, Roman Kagan wrote:
> > On Fri, Apr 16, 2021 at 11:08:40AM +0300, Vladimir Sementsov-Ogievskiy
> > wrote:
> > > @@ -2305,20 +2301,23 @@ static int nbd_open(BlockDriverState *b
On Fri, Apr 16, 2021 at 11:08:40AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> We have two "return error" paths in nbd_open() after
> nbd_process_options(). Actually we should call nbd_clear_bdrvstate()
> on these paths. Interesting that nbd_process_options() calls
> nbd_clear_bdrvstate() by
re-patch, on first hunk we'll just crash if thr is NULL,
> on second hunk it's safe to return -1, and using thr when
> s->connect_thread is already zeroed is obviously wrong.
>
> block/nbd.c | 11 +++
> 1 file changed, 11 insertions(+)
Can we please get it merged in 6.0 as it's a genuine crasher fix?
Reviewed-by: Roman Kagan
On Sat, Apr 10, 2021 at 12:56:34PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 10.04.2021 11:38, Vladimir Sementsov-Ogievskiy wrote:
> > 10.04.2021 11:06, Vladimir Sementsov-Ogievskiy wrote:
> > > 09.04.2021 19:04, Roman Kagan wrote:
> > > > Simplify lifet
Simplify lifetime management of BDRVNBDState->connection_thread by
delaying the possible cleanup of it until the BDRVNBDState itself goes
away.
This also fixes possible use-after-free in nbd_co_establish_connection
when it races with nbd_co_establish_connection_cancel.
Signed-off-by: Roman Ka
nbd_free_connect_thread leaks the channel object if it hasn't been
stolen.
Unref it and fix the leak.
Signed-off-by: Roman Kagan
---
block/nbd.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/block/nbd.c b/block/nbd.c
index c26dc5a54f..d86df3afcb 100644
--- a/block/nbd.c
+++ b/block/nbd.c
A couple of bugfixes to block/nbd that look appropriate for 6.0.
Roman Kagan (2):
block/nbd: fix channel object leak
block/nbd: ensure ->connection_thread is always valid
block/nbd.c | 59 +++--
1 file changed, 30 insertions(+), 29 deleti
On Thu, Apr 08, 2021 at 06:54:30PM +0300, Roman Kagan wrote:
> On Thu, Apr 08, 2021 at 05:08:20PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> > With the following patch we want to call aio_co_wake() from thread.
> > And it works bad.
> > Assume we have no iothreads.
> >
On Thu, Apr 08, 2021 at 05:08:17PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
>
> This substitutes "[PATCH 00/14] nbd: move reconnect-thread to separate file"
> Supersedes: <20210407104637.36033-1-vsement...@virtuozzo.com>
>
> I want to simplify block/nbd.c which is overcomplicated
> nbd/meson.build | 1 +
> 4 files changed, 204 insertions(+), 167 deletions(-)
> create mode 100644 nbd/client-connection.c
Reviewed-by: Roman Kagan
ov-Ogievskiy
> ---
> block/nbd.c | 15 +--
> 1 file changed, 9 insertions(+), 6 deletions(-)
Reviewed-by: Roman Kagan
ementsov-Ogievskiy
> ---
> block/nbd.c | 127 ++--
> 1 file changed, 63 insertions(+), 64 deletions(-)
[To other reviewers: in addition to renaming there's one blank line
removed, hence the difference between (+) and (-)]
Reviewed-by: Roman Kagan
> 1 file changed, 7 insertions(+), 9 deletions(-)
Reviewed-by: Roman Kagan
> ---
> block/nbd.c | 49 +++--
> 1 file changed, 31 insertions(+), 18 deletions(-)
Reviewed-by: Roman Kagan
---
> 1 file changed, 27 insertions(+), 76 deletions(-)
Reviewed-by: Roman Kagan
connection API out of
> nbd.c (which is overcomplicated now).
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> block/nbd.c | 49 +--------
> 1 file changed, 9 insertions(+), 40 deletions(-)
Reviewed-by: Roman Kagan
context was qemu_aio_context, an iothread would just schedule the
coroutine there, while a "dumb" thread would try lock the context
potentially resulting in a deadlock. This patch makes "dumb" threads
and iothreads behave identically when entering a coroutine on a foreign
context.
You may want to rephrase the log message to that end.
Anyway
Reviewed-by: Roman Kagan
On Thu, Apr 08, 2021 at 05:08:19PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> These fields are write-only. Drop them.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> block/nbd.c | 12 ++--
> 1 file changed, 2 insertions(+), 10 deletions(-)
Reviewed-by: Roman Kagan
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> block/nbd.c | 62 +--------
> 1 file changed, 20 insertions(+), 42 deletions(-)
Reviewed-by: Roman Kagan
On Wed, Apr 07, 2021 at 01:46:29PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Add personal state NBDConnectThread for connect-thread and
> nbd_connect_thread_start() function. Next step would be moving
> connect-thread to a separate file.
>
> Note that we stop publishing thr->sioc during
>
On Wed, Apr 07, 2021 at 01:46:26PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> The field is used only to free it. Let's drop it for now for
> simplicity.
Well, it's *now* (after your patch 2) only used to free it. This makes
the reconnect process even further concealed from the user: the client
On Wed, Apr 07, 2021 at 01:46:25PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> We are going to refactor connection logic to make it more
> understandable. Every bit that we can simplify in advance will help.
> Drop errp for now, it's unused anyway. We'll probably reimplement it in
> future.
On Wed, Apr 07, 2021 at 01:46:24PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> The field is actually unused. Let's make things a bit simpler dropping
> it and corresponding logic.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> block/nbd.c | 9 ++---
> 1 file changed, 2
On Wed, Mar 17, 2021 at 11:35:31AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 15.03.2021 09:06, Roman Kagan wrote:
> > The reconnection logic doesn't need to stop while in a drained section.
> > Moreover it has to be active during the drained section, as the requests
>
On Wed, Mar 17, 2021 at 11:35:31AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 15.03.2021 09:06, Roman Kagan wrote:
> > The reconnection logic doesn't need to stop while in a drained section.
> > Moreover it has to be active during the drained section, as the requests
>
On Tue, Mar 16, 2021 at 09:37:13PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 16.03.2021 19:08, Roman Kagan wrote:
> > On Mon, Mar 15, 2021 at 11:15:44PM +0300, Vladimir Sementsov-Ogievskiy
> > wrote:
> > > 15.03.2021 09:06, Roman Kagan wrote:
> > > > As the
On Tue, Mar 16, 2021 at 09:09:12PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 16.03.2021 19:03, Roman Kagan wrote:
> > On Mon, Mar 15, 2021 at 11:10:14PM +0300, Vladimir Sementsov-Ogievskiy
> > wrote:
> > > 15.03.2021 09:06, Roman Kagan wrote:
> > > > The rec
On Tue, Mar 16, 2021 at 09:41:36AM -0500, Eric Blake wrote:
> On 3/15/21 1:06 AM, Roman Kagan wrote:
> > The reconnection logic doesn't need to stop while in a drained section.
> > Moreover it has to be active during the drained section, as the requests
> > that
On Mon, Mar 15, 2021 at 11:10:14PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 15.03.2021 09:06, Roman Kagan wrote:
> > The reconnection logic doesn't need to stop while in a drained section.
> > Moreover it has to be active during the drained section, as the requests
>
On Mon, Mar 15, 2021 at 11:15:44PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 15.03.2021 09:06, Roman Kagan wrote:
> > As the reconnect logic no longer interferes with drained sections, it
> > appears unnecessary to explicitly manipulate the in_flight counter.
> >
> &
On Mon, Mar 15, 2021 at 10:45:39PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 15.03.2021 09:06, Roman Kagan wrote:
> > The reconnection logic doesn't need to stop while in a drained section.
> > Moreover it has to be active during the drained section, as the requests
>
On Mon, Mar 15, 2021 at 06:40:12PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 15.03.2021 09:06, Roman Kagan wrote:
> > When the NBD connection is being torn down, the connection thread gets
> > canceled and "detached", meaning it is about to get freed.
>
On Mon, Mar 15, 2021 at 07:41:32PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 15.03.2021 09:06, Roman Kagan wrote:
> > Document (via a comment and an assert) that
> > nbd_client_detach_aio_context and nbd_client_attach_aio_context_bh run
> > in the desired aio_contex
ion thread data.
To prevent this, revalidate the ->connect_thread pointer in
nbd_co_establish_connection_cancel before using after the the yield.
Signed-off-by: Roman Kagan
---
block/nbd.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/block/nbd.c b/block/nbd.c
index c26dc5a54
the
reconnection logic on entry and starts it over on exit. However, this
patch paves the way to keeping the reconnection process active across
the drained section (in a followup patch).
Signed-off-by: Roman Kagan
---
block/nbd.c | 44 ++--
1 file changed, 42 insertions
Use nbd_client_connecting_wait uniformly all over the block/nbd.c.
While at this, drop the redundant check for nbd_client_connecting_wait
in reconnect_delay_timer_init, as all its callsites do this check too.
Signed-off-by: Roman Kagan
---
block/nbd.c | 34 +++---
1
Cosmetic: adjust the comment and the return value in
nbd_co_establish_connection where it's entered while the connection
thread is still running.
Signed-off-by: Roman Kagan
---
block/nbd.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index
As the reconnect logic no longer interferes with drained sections, it
appears unnecessary to explicitly manipulate the in_flight counter.
Fixes: 5ad81b4946 ("nbd: Restrict connection_co reentrance")
Signed-off-by: Roman Kagan
---
block/nbd.c | 6 --
nbd/client.c | 2 --
2 files
Document (via a comment and an assert) that
nbd_client_detach_aio_context and nbd_client_attach_aio_context_bh run
in the desired aio_context.
Signed-off-by: Roman Kagan
---
block/nbd.c | 12
1 file changed, 12 insertions(+)
diff --git a/block/nbd.c b/block/nbd.c
index 1d8edb5b21
with the drained section in the reconnection code.
Fixes: 5ad81b4946 ("nbd: Restrict connection_co reentrance")
Fixes: 8c517de24a ("block/nbd: fix drain dead-lock because of nbd
reconnect-delay")
Signed-off-by: Roman Kagan
---
block/nbd.c | 79 +++-
t;nbd: Restrict
connection_co reentrance"); as I've missed the point of that commit I'd
appreciate more scrutiny in this area.
Roman Kagan (7):
block/nbd: avoid touching freed connect_thread
block/nbd: use uniformly nbd_client_connecting_wait
block/nbd: assert attach/detach runs in
On Fri, Mar 12, 2021 at 03:35:25PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 10.03.2021 12:32, Roman Kagan wrote:
> > NBD connect coroutine takes an extra in_flight reference as if it's a
> > request handler. This prevents drain from completion until the
> > connection
in
.bdrv_{attach,detach}_aio_context callbacks.
Fixes: 5ad81b4946 ("nbd: Restrict connection_co reentrance")
Signed-off-by: Roman Kagan
---
This patch passes the regular make check but fails some extra iotests,
in particular 277. It obviously lacks more robust interaction with the
connect
On Wed, Feb 03, 2021 at 05:44:34PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 03.02.2021 17:21, Roman Kagan wrote:
> > On Wed, Feb 03, 2021 at 04:10:41PM +0300, Vladimir Sementsov-Ogievskiy
> > wrote:
> > > 03.02.2021 13:53, Roman Kagan wrote:
> > > > On T
On Wed, Feb 03, 2021 at 04:10:41PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 03.02.2021 13:53, Roman Kagan wrote:
> > On Thu, Sep 03, 2020 at 10:02:58PM +0300, Vladimir Sementsov-Ogievskiy
> > wrote:
> > > We pause reconnect process during drained section. So, if we
On Thu, Sep 03, 2020 at 10:02:58PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> We pause reconnect process during drained section. So, if we have some
> requests, waiting for reconnect we should cancel them, otherwise they
> deadlock the drained section.
>
> How to reproduce:
>
> 1. Create an
1 - 100 of 673 matches
Mail list logo
