Re: [Qemu-devel] [PATCH 4/9] block/nbd-client: save first fatal error in nbd_iter_error

2018-02-16 Thread Eric Blake

On 02/15/2018 07:51 AM, Vladimir Sementsov-Ogievskiy wrote:

It is ok, that fatal error hides previous not fatal, but hiding
first fatal error is a bad feature.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  block/nbd-client.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)



Can this trigger in 2.11 clients with structured reads to the point 
where we'd want to add qemu-stable in CC?  Or did you only hit it once 
block_status was added to the mix?


Note that comparing 2.11 server with 2.11 client doesn't help - because 
2.11 as server only sends a single chunk per structured read (as the 
only structured reply); it wasn't until commit 418638d3 that we have a 
qemu server that can send multiple chunks, and it looks like you need 
multiple chunks before multiple errors becomes a possibility.


At any rate,

Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



[Qemu-devel] [PATCH 4/9] block/nbd-client: save first fatal error in nbd_iter_error

2018-02-15 Thread Vladimir Sementsov-Ogievskiy
It is ok, that fatal error hides previous not fatal, but hiding
first fatal error is a bad feature.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/nbd-client.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index 9206652e45..b1cbe95b13 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -481,6 +481,7 @@ static coroutine_fn int nbd_co_receive_one_chunk(
 
 typedef struct NBDReplyChunkIter {
 int ret;
+bool fatal;
 Error *err;
 bool done, only_structured;
 } NBDReplyChunkIter;
@@ -490,11 +491,12 @@ static void nbd_iter_error(NBDReplyChunkIter *iter, bool 
fatal,
 {
 assert(ret < 0);
 
-if (fatal || iter->ret == 0) {
+if ((fatal && !iter->fatal) || iter->ret == 0) {
 if (iter->ret != 0) {
 error_free(iter->err);
 iter->err = NULL;
 }
+iter->fatal = fatal;
 iter->ret = ret;
 error_propagate(&iter->err, *local_err);
 } else {
-- 
2.11.1