[Qemu-devel] [PATCH for-4.0] qemu-img: Gracefully shutdown when map can't finish

Eric Blake posted 1 patch 5 years ago
Test docker-clang@ubuntu passed
Test docker-mingw@fedora passed
Test asan passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190326184043.7544-1-eblake@redhat.com
Maintainers: Kevin Wolf <kwolf@redhat.com>, Max Reitz <mreitz@redhat.com>
qemu-img.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
[Qemu-devel] [PATCH for-4.0] qemu-img: Gracefully shutdown when map can't finish
Posted by Eric Blake 5 years ago
Trying 'qemu-img map -f raw nbd://localhost:10809' causes the
NBD server to output a scary message:

qemu-nbd: Disconnect client, due to: Failed to read request: Unexpected end-of-file before all bytes were read

This is because the NBD client, being remote, has no way to expose a
human-readable map (the --output=json data is fine, however). But
because we exit(1) right after the message, causing the client to
bypass all block cleanup, the server sees the abrupt exit and warns,
whereas it would be silent had the client had a chance to send
NBD_CMD_DISC. Other protocols may have similar cleanup issues, where
failure to blk_unref() could cause unintended effects.

Signed-off-by: Eric Blake <eblake@redhat.com>
---

Since I found this while testing NBD, I'm happy to include it through
my tree.

 qemu-img.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 12f071a1ca2..55c599841a1 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2736,14 +2736,14 @@ static int img_info(int argc, char **argv)
     return 0;
 }

-static void dump_map_entry(OutputFormat output_format, MapEntry *e,
-                           MapEntry *next)
+static int dump_map_entry(OutputFormat output_format, MapEntry *e,
+                          MapEntry *next)
 {
     switch (output_format) {
     case OFORMAT_HUMAN:
         if (e->data && !e->has_offset) {
             error_report("File contains external, encrypted or compressed clusters.");
-            exit(1);
+            return -1;
         }
         if (e->data && !e->zero) {
             printf("%#-16"PRIx64"%#-16"PRIx64"%#-16"PRIx64"%s\n",
@@ -2776,6 +2776,7 @@ static void dump_map_entry(OutputFormat output_format, MapEntry *e,
         }
         break;
     }
+    return 0;
 }

 static int get_block_status(BlockDriverState *bs, int64_t offset,
@@ -2968,12 +2969,15 @@ static int img_map(int argc, char **argv)
         }

         if (curr.length > 0) {
-            dump_map_entry(output_format, &curr, &next);
+            ret = dump_map_entry(output_format, &curr, &next);
+            if (ret < 0) {
+                goto out;
+            }
         }
         curr = next;
     }

-    dump_map_entry(output_format, &curr, NULL);
+    ret = dump_map_entry(output_format, &curr, NULL);

 out:
     blk_unref(blk);
-- 
2.20.1


Re: [Qemu-devel] [Qemu-block] [PATCH for-4.0] qemu-img: Gracefully shutdown when map can't finish
Posted by John Snow 5 years ago

On 3/26/19 2:40 PM, Eric Blake wrote:
> Trying 'qemu-img map -f raw nbd://localhost:10809' causes the
> NBD server to output a scary message:
> 
> qemu-nbd: Disconnect client, due to: Failed to read request: Unexpected end-of-file before all bytes were read
> 
> This is because the NBD client, being remote, has no way to expose a
> human-readable map (the --output=json data is fine, however). But
> because we exit(1) right after the message, causing the client to
> bypass all block cleanup, the server sees the abrupt exit and warns,
> whereas it would be silent had the client had a chance to send
> NBD_CMD_DISC. Other protocols may have similar cleanup issues, where
> failure to blk_unref() could cause unintended effects.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>

Seems good; taking it through the NBD tree helps me keep track of them :)

Reviewed-by: John Snow <jsnow@redhat.com>

Re: [Qemu-devel] [PATCH for-4.0] qemu-img: Gracefully shutdown when map can't finish
Posted by Kevin Wolf 5 years ago
Am 26.03.2019 um 19:40 hat Eric Blake geschrieben:
> Trying 'qemu-img map -f raw nbd://localhost:10809' causes the
> NBD server to output a scary message:
> 
> qemu-nbd: Disconnect client, due to: Failed to read request: Unexpected end-of-file before all bytes were read
> 
> This is because the NBD client, being remote, has no way to expose a
> human-readable map (the --output=json data is fine, however). But
> because we exit(1) right after the message, causing the client to
> bypass all block cleanup, the server sees the abrupt exit and warns,
> whereas it would be silent had the client had a chance to send
> NBD_CMD_DISC. Other protocols may have similar cleanup issues, where
> failure to blk_unref() could cause unintended effects.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Re: [Qemu-devel] [PATCH for-4.0] qemu-img: Gracefully shutdown when map can't finish
Posted by Eric Blake 5 years ago
On 3/28/19 7:14 AM, Kevin Wolf wrote:
> Am 26.03.2019 um 19:40 hat Eric Blake geschrieben:
>> Trying 'qemu-img map -f raw nbd://localhost:10809' causes the
>> NBD server to output a scary message:
>>
>> qemu-nbd: Disconnect client, due to: Failed to read request: Unexpected end-of-file before all bytes were read
>>
>> This is because the NBD client, being remote, has no way to expose a
>> human-readable map (the --output=json data is fine, however). But
>> because we exit(1) right after the message, causing the client to
>> bypass all block cleanup, the server sees the abrupt exit and warns,
>> whereas it would be silent had the client had a chance to send
>> NBD_CMD_DISC. Other protocols may have similar cleanup issues, where
>> failure to blk_unref() could cause unintended effects.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
> 
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Thanks; queued on my NBD tree for -rc2

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