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 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 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
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
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
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
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
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
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:
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
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
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
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
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
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
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
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: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 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 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
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
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 +++-
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
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
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
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
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
0 in ?? ()
Fix it by checking that the connection coroutine is non-null before
trying to enter it. If it is null, no entering is needed, as the
connection is probably going down anyway.
Signed-off-by: Roman Kagan
Reviewed-by: Vladimir Sementsov-Ogievskiy
---
block/nbd.c | 16 +---
1 fil
the connection.
Fix it by turning every negative return from qio_channel_read_all into
-EIO returned from nbd_read. Apparently that was the original behavior,
but got broken later. Also adjust nbd_readXX to follow.
Fixes: e6798f06a6 ("nbd: generalize usage of nbd_read")
Signed-off-by: R
ed backtraces in log messages
- add r-b by Vladimir
Roman Kagan (3):
block/nbd: only detach existing iochannel from aio_context
block/nbd: only enter connection coroutine if it's present
nbd: make nbd_read* return -EIO on error
include/block/nbd.h | 7 ---
block/nbd.c
it by checking that the iochannel is non-null before trying to
detach it from the aio_context. If it is null, no detaching is needed,
and it will get reattached in the proper aio_context once the connection
is reestablished.
Signed-off-by: Roman Kagan
Reviewed-by: Vladimir Sementsov-Ogievskiy
---
block
On Fri, Jan 29, 2021 at 08:51:39AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 28.01.2021 23:14, Roman Kagan wrote:
> > During the final phase of migration the NBD reconnection logic may
> > encounter situations it doesn't expect during regular operation.
> >
> >
On Fri, Jan 29, 2021 at 08:37:13AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 28.01.2021 23:14, Roman Kagan wrote:
> > When the reconnect in NBD client is in progress, the iochannel used for
> > NBD connection doesn't exist. Therefore an attempt to detach it from
>
to
detach it from the aio_context. If it is null, no detaching is needed,
and it will get reattached in the proper aio_context once the connection
is reestablished.
Signed-off-by: Roman Kagan
---
block/nbd.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/block/nbd.c b/bl
lib/x86_64-linux-gnu/libpthread.so.0
Fix it by checking that the connection coroutine is non-null before
trying to enter it. If it is null, no entering is needed, as the
connection is probably going down anyway.
Signed-off-by: Roman Kagan
---
block/nbd.c | 16 +---
1 file changed
the connection.
Fix it by turning every negative return from qio_channel_read_all into
-EIO returned from nbd_read. Apparently that was the original behavior,
but got broken later. Also adjust nbd_readXX to follow.
Fixes: e6798f06a6 ("nbd: generalize usage of nbd_read")
Signed-off-by: R
delay" runs a stress load (fio with big queue depth)
in the guest on that drive and is migrated (e.g. to a file), while the
nbd server is SIGKILL-ed and restarted every second.
See the individual patches for specific crash conditions and more
detailed explanations.
Roman Kagan (3):
block/nbd: o
On Fri, May 29, 2020 at 01:55:08AM +0300, Roman Kagan wrote:
> BlockConf includes several properties counted in bytes.
>
> Enhance their handling in some aspects, specifically
>
> - accept common size suffixes (k, m)
> - perform consistency checks on the values
> -
On Fri, May 29, 2020 at 11:53:26AM +0200, Markus Armbruster wrote:
> Roman Kagan writes:
>
> > Several block device properties related to blocksize configuration must
> > be in certain relationship WRT each other: physical block must be no
> > smaller than logical block;
Logical and physical block sizes in QEMU are limited to 32 KiB.
This appears unnecessarily tight, and we've seen bigger block sizes
handy at times.
Lift the limitation up to 2 MiB which appears to be good enough for
everybody, and matches the qcow2 cluster size limit.
Signed-off-by: Roman Kagan
and virtio-blk
devices as an uint16_t in units of logical blocks, introduce an
additional check in blkconf_blocksizes to prevent its silent truncation.
Signed-off-by: Roman Kagan
---
v7 -> v8:
- replace stringify with %u in the error message [Eric]
- fix wording in the log [Eric]
include
, add corresponding consistency
checks to blkconf_blocksizes, adjusting its signature to communicate
possible error to the caller. Also remove the now redundant consistency
checks from the specific devices.
Signed-off-by: Roman Kagan
Reviewed-by: Eric Blake
Reviewed-by: Paul Durrant
---
include
Add getter for size32, and use it for blocksize, too.
In its human-readable branch, it reports approximate size in
human-readable units next to the exact byte value, like the getter for
64bit size does.
Adjust the expected test output accordingly.
Signed-off-by: Roman Kagan
Reviewed-by: Eric
It appears convenient to be able to specify physical_block_size and
logical_block_size using common size suffixes.
Teach the blocksize property setter to interpret them. Also express the
upper and lower limits in the respective units.
Signed-off-by: Roman Kagan
Reviewed-by: Eric Blake
---
hw
property type in a followup commit).
The getter for size32 is left out for a separate patch as its benefit is
less obvious, and it affects test output; for now the regular uint32
getter is used.
Signed-off-by: Roman Kagan
---
v7 -> v8:
- replace stringify with %u in the error message [Eric]
-
e limit in the log and comment [Eric]
v1 -> v2:
- cap the property at 2 MiB [Eric]
- accept size suffixes
Roman Kagan (8):
virtio-blk: store opt_io_size with correct size
block: consolidate blocksize properties consistency checks
qdev-properties: blocksize: use same limits in code a
The width of opt_io_size in virtio_blk_config is 32bit. However, it's
written with virtio_stw_p; this may result in value truncation, and on
big-endian systems with legacy virtio in completely bogus readings in
the guest.
Use the appropriate accessor to store it.
Signed-off-by: Roman Kagan
1 - 100 of 195 matches
Mail list logo
