[Qemu-devel] [PATCH for-4.0 v3 0/6] NBD server alignment improvement

Eric Blake posted 6 patches 5 years ago
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test checkpatch passed
Test asan passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190329042750.14704-1-eblake@redhat.com
Maintainers: Eric Blake <eblake@redhat.com>, Max Reitz <mreitz@redhat.com>, Kevin Wolf <kwolf@redhat.com>
include/sysemu/block-backend.h |  1 +
block/block-backend.c          |  7 ++++
block/nbd-client.c             | 48 +++++++++++++++++++++--
block/nbd.c                    | 19 ++++++++-
nbd/server.c                   | 13 ++++---
tests/qemu-iotests/209.out     |  4 +-
tests/qemu-iotests/223.out     | 18 ++++-----
tests/qemu-iotests/241         | 70 ++++++++++++++++++++++++++++++++++
tests/qemu-iotests/241.out     |  8 ++++
tests/qemu-iotests/group       |  1 +
10 files changed, 169 insertions(+), 20 deletions(-)
create mode 100755 tests/qemu-iotests/241
create mode 100644 tests/qemu-iotests/241.out
[Qemu-devel] [PATCH for-4.0 v3 0/6] NBD server alignment improvement
Posted by Eric Blake 5 years ago
I'm tired of nbdkit being able to trigger an assertion failure
in qemu-img convert.  Let's get this into 4.0-rc2.

Since v2:
- add more patches, including a respin of a separately-posted
nbd/client: Deal with unaligned size from server
- Don't make unaligned sector from server inaccessible; instead,
round our requests down and handle the tail locally
- fix another issue reported by Rich where 'qemu-img map' output
was less than stellar

Eric Blake (6):
  iotests: Add 241 to test NBD on unaligned images
  nbd/client: Lower min_block for block-status, unaligned size
  nbd/client: Report offsets in bdrv_block_status
  nbd/client: Support qemu-img convert from unaligned size
  block: Add bdrv_get_request_alignment()
  nbd/server: Advertise actual minimum block size

 include/sysemu/block-backend.h |  1 +
 block/block-backend.c          |  7 ++++
 block/nbd-client.c             | 48 +++++++++++++++++++++--
 block/nbd.c                    | 19 ++++++++-
 nbd/server.c                   | 13 ++++---
 tests/qemu-iotests/209.out     |  4 +-
 tests/qemu-iotests/223.out     | 18 ++++-----
 tests/qemu-iotests/241         | 70 ++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/241.out     |  8 ++++
 tests/qemu-iotests/group       |  1 +
 10 files changed, 169 insertions(+), 20 deletions(-)
 create mode 100755 tests/qemu-iotests/241
 create mode 100644 tests/qemu-iotests/241.out

-- 
2.20.1


Re: [Qemu-devel] [PATCH for-4.0 v3 0/6] NBD server alignment improvement
Posted by Richard W.M. Jones 5 years ago
I tested this version of qemu against the nbdkit test suite and it
passed.

It also fixes the problem with qemu-img convert:

  $ ./nbdkit -U - memory size=511 --run 'qemu-img convert $nbd /var/tmp/out'

There are a couple of issues though (I don't think you'll think they
are bugs).

Firstly it rounds up the size to 512 bytes.  eg. /var/tmp/out above is
512 bytes, and qemu-img info shows the virtual size as 512 bytes.

Secondly I still can't open the INT64_MAX image which as you will
recall has a 511 byte tail:

  $ ./nbdkit -U - memory size=$((2**63 - 1)) --run 'qemu-img info $nbd'
  qemu-img: Could not open 'nbd:unix:/tmp/nbdkitTimYSJ/socket': Could not read image for determining its format: File too large

... yet the 2^63-512 image can be opened (same as before):

  $ ./nbdkit -U - memory size=$((2**63 - 512)) --run 'qemu-img info $nbd'
  image: nbd+unix://?socket=/tmp/nbdkitHSAzNz/socket
  file format: raw
  virtual size: -8388607T (9223372036854775296 bytes)
  disk size: unavailable

I've just noticed that the virtual size negative (also in upstream
qemu).  That looks like a bug too ...

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

Re: [Qemu-devel] [PATCH for-4.0 v3 0/6] NBD server alignment improvement
Posted by Eric Blake 5 years ago
On 3/29/19 11:40 AM, Richard W.M. Jones wrote:
> 
> I tested this version of qemu against the nbdkit test suite and it
> passed.
> 
> It also fixes the problem with qemu-img convert:
> 
>   $ ./nbdkit -U - memory size=511 --run 'qemu-img convert $nbd /var/tmp/out'
> 
> There are a couple of issues though (I don't think you'll think they
> are bugs).
> 
> Firstly it rounds up the size to 512 bytes.  eg. /var/tmp/out above is
> 512 bytes, and qemu-img info shows the virtual size as 512 bytes.

Well, that's the long-standing problem that qemu rounds up to sector
boundaries, instead of being byte-accurate. Not going to get fixed for
4.0, but I've got my on it for 4.1.

> 
> Secondly I still can't open the INT64_MAX image which as you will
> recall has a 511 byte tail:
> 
>   $ ./nbdkit -U - memory size=$((2**63 - 1)) --run 'qemu-img info $nbd'
>   qemu-img: Could not open 'nbd:unix:/tmp/nbdkitTimYSJ/socket': Could not read image for determining its format: File too large

Correct, fallout of the point above (rounding up overflows).

> 
> ... yet the 2^63-512 image can be opened (same as before):
> 
>   $ ./nbdkit -U - memory size=$((2**63 - 512)) --run 'qemu-img info $nbd'
>   image: nbd+unix://?socket=/tmp/nbdkitHSAzNz/socket
>   file format: raw
>   virtual size: -8388607T (9223372036854775296 bytes)
>   disk size: unavailable
> 
> I've just noticed that the virtual size negative (also in upstream
> qemu).  That looks like a bug too ...

Yes, it does. I'll see if I can find a quick fix for that formatting
routine.

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

Re: [Qemu-devel] [PATCH for-4.0 v3 0/6] NBD server alignment improvement
Posted by Eric Blake 5 years ago
On 3/29/19 3:30 PM, Eric Blake wrote:

>> ... yet the 2^63-512 image can be opened (same as before):
>>
>>   $ ./nbdkit -U - memory size=$((2**63 - 512)) --run 'qemu-img info $nbd'
>>   image: nbd+unix://?socket=/tmp/nbdkitHSAzNz/socket
>>   file format: raw
>>   virtual size: -8388607T (9223372036854775296 bytes)
>>   disk size: unavailable
>>
>> I've just noticed that the virtual size negative (also in upstream
>> qemu).  That looks like a bug too ...
> 
> Yes, it does. I'll see if I can find a quick fix for that formatting
> routine.
> 

qemu-img is calling block/qapi.c:get_human_readable_size(), which stops
at T, and has indeed has some horrible overflow problems, such as:
                         ((size + (base >> 1)) / base),
without regards to whether int64_t size is near maximum before the addition.

Other places in the code base use util/cutils.c:size_to_str(), which
handles all the way up to E, and appears to be a bit saner.

Why we have to separate integer-to-human converters, where one is rather
buggy, is beyond me.

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

Re: [Qemu-devel] [PATCH for-4.0 v3 0/6] NBD server alignment improvement
Posted by Eric Blake 5 years ago
On 3/28/19 11:27 PM, Eric Blake wrote:
> I'm tired of nbdkit being able to trigger an assertion failure
> in qemu-img convert.  Let's get this into 4.0-rc2.
> 
> Since v2:
> - add more patches, including a respin of a separately-posted
> nbd/client: Deal with unaligned size from server
> - Don't make unaligned sector from server inaccessible; instead,
> round our requests down and handle the tail locally
> - fix another issue reported by Rich where 'qemu-img map' output
> was less than stellar
> 
> Eric Blake (6):
>   iotests: Add 241 to test NBD on unaligned images
>   nbd/client: Lower min_block for block-status, unaligned size
>   nbd/client: Report offsets in bdrv_block_status
>   nbd/client: Support qemu-img convert from unaligned size
>   block: Add bdrv_get_request_alignment()
>   nbd/server: Advertise actual minimum block size

I've queued this series on my NBD tree, and will send a pull request
shortly.

I have a couple more patches that I started work on that fix the final
few alignment bugs, but as they have not been on the list yet, they may
be too risky for 4.0; we'll see what happens based on how fast I can
polish them.

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