Re: [RFC 0/3] 64bit block-layer part I

2020-04-23 Thread Kevin Wolf
Am 30.03.2020 um 16:18 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Hi all!
> 
> There is an idea to make NBD protocol extension to support 64bit
> write-zero/discard/block-status commands (commands, which doesn't
> transfer user data). It's needed to increase performance of zeroing
> large ranges (up to the whole image). Zeroing of the whole image is used
> as first step of mirror job, qemu-img convert, it should be also used at
> start of backup actually..
> 
> We need to support it in block-layer, so we want 64bit write_zeros.
> Currently driver handler now have int bytes parameter.
> 
> write_zeros path goes through normal pwritev, so we need 64bit write,
> and then we need 64bit read for symmetry, and better, let's make all io
> path work with 64bit bytes parameter.
> 
> Actually most of block-layer already have 64bit parameters: offset is
> sometimes int64_t and sometimes uint64_t. bytes parameter is one of
> int64_t, uint64_t, int, unsigned int...
> 
> I think we need one type for all of this, and this one type is int64_t.
> Signed int64_t is a bit better than uint64_t: you can use same variable
> to get some result (including error < 0) and than reuse it as an
> argument without any type conversion.
> 
> So, I propose, as a first step, convert all uint64_t parameters to
> int64_t.
> 
> Still, I don't have good idea of how to split this into more than 3
> patches, so, this is an RFC.

I think the split in three patches isn't too bad because it's not a
whole lot of code. But of course, it is little code that has lots of
implications which does make it hard to review. If we think that we
might bisect a bug in the series later, maybe it would be better to
split it into more patches.

write/write_zeroes has to be a single thing, I'm afraid. But I guess
read could be a separate patch, as could be copy_range. Not sure about
discard.

> What's next?
> 
> Converting write_zero and discard is not as simple: we can't just
> s/int/uint64_t/, as some functions may use some int variables for
> calculations and this will be broken by something larger than int.
> 
> So, I think the simplest way is to add .bdrv_co_pwritev_zeros64 and
> .bdrv_co_pdiscard64 and update drivers one-by-one. If at some point all
> drivers updated - drop unused 32bit functions, and then drop "64" suffix
> from API. If not - we'll live with both APIs.

We already have too many unfinished conversions in QEMU, let's not add
one more.

Fortunately, we already have a tool that could help us here: Things like
bs->bl.max_pwrite_zeroes. We could make BDRV_REQUEST_MAX_BYTES the
default value and only drivers that override it can get bigger requests.

> Another thing to do is updating default limiting of request (currently
> they are limited to INT_MAX).

As above, I wouldn't update the default, but rather enable drivers to
overload the default with a larger value. This will involve changing
some places where we use MIN() between INT_MAX and the driver's value.

> Then we may move some drivers to 64bit discard/write_zero: I think about
> qcow2, file-posix and nbd (as a proof-of-concept for already proposed
> NBD extension).

Makes sense to me.

Kevin




Re: [RFC 0/3] 64bit block-layer part I

2020-04-22 Thread Eric Blake

On 4/22/20 1:24 PM, Vladimir Sementsov-Ogievskiy wrote:


So, I think the simplest way is to add .bdrv_co_pwritev_zeros64 and
.bdrv_co_pdiscard64 and update drivers one-by-one. If at some point all
drivers updated - drop unused 32bit functions, and then drop "64" suffix
from API. If not - we'll live with both APIs.


Hmm. Or, maybe nothing dangerous if we convert it to 64bit, and add 
comment,

that function is not actually prepared for 64bit bytes and depends on
max_transfer/max_zeroes being <= INT_MAX ?

Or, maybe better, keep old functions as is and add 64bit wrappers, which
assert that bytes < INT_MAX, and call old function? This is clean, driver
maybe updated later on demand, and we don't need extra API. If no 
objections,

I'll try this way in the next version.


That approach sounds good; it matches how we added flags and iov 
support, as well as how we transitioned from sector interfaces over to 
byte interfaces: we added a new .bdrv_ callback for the drivers that 
could take advantage of the increased interface, without having to touch 
the older drivers using the old callbacks, then only later finally got 
rid of the old interfaces as we modernized other drivers.  There's still 
the issue of how much we convert in the initial series - even if adding 
a new wrapper makes it easier to patch only one driver at a time, it's 
not nice to leave the job half-done by not visiting all of the drivers 
eventually.


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




Re: [RFC 0/3] 64bit block-layer part I

2020-04-22 Thread Vladimir Sementsov-Ogievskiy

30.03.2020 17:18, Vladimir Sementsov-Ogievskiy wrote:

Hi all!

There is an idea to make NBD protocol extension to support 64bit
write-zero/discard/block-status commands (commands, which doesn't
transfer user data). It's needed to increase performance of zeroing
large ranges (up to the whole image). Zeroing of the whole image is used
as first step of mirror job, qemu-img convert, it should be also used at
start of backup actually..

We need to support it in block-layer, so we want 64bit write_zeros.
Currently driver handler now have int bytes parameter.

write_zeros path goes through normal pwritev, so we need 64bit write,
and then we need 64bit read for symmetry, and better, let's make all io
path work with 64bit bytes parameter.

Actually most of block-layer already have 64bit parameters: offset is
sometimes int64_t and sometimes uint64_t. bytes parameter is one of
int64_t, uint64_t, int, unsigned int...

I think we need one type for all of this, and this one type is int64_t.
Signed int64_t is a bit better than uint64_t: you can use same variable
to get some result (including error < 0) and than reuse it as an
argument without any type conversion.

So, I propose, as a first step, convert all uint64_t parameters to
int64_t.

Still, I don't have good idea of how to split this into more than 3
patches, so, this is an RFC.

What's next?

Converting write_zero and discard is not as simple: we can't just
s/int/uint64_t/, as some functions may use some int variables for
calculations and this will be broken by something larger than int.

So, I think the simplest way is to add .bdrv_co_pwritev_zeros64 and
.bdrv_co_pdiscard64 and update drivers one-by-one. If at some point all
drivers updated - drop unused 32bit functions, and then drop "64" suffix
from API. If not - we'll live with both APIs.


Hmm. Or, maybe nothing dangerous if we convert it to 64bit, and add comment,
that function is not actually prepared for 64bit bytes and depends on
max_transfer/max_zeroes being <= INT_MAX ?

Or, maybe better, keep old functions as is and add 64bit wrappers, which
assert that bytes < INT_MAX, and call old function? This is clean, driver
maybe updated later on demand, and we don't need extra API. If no objections,
I'll try this way in the next version.



Another thing to do is updating default limiting of request (currently
they are limited to INT_MAX).

Then we may move some drivers to 64bit discard/write_zero: I think about
qcow2, file-posix and nbd (as a proof-of-concept for already proposed
NBD extension).

Any ideas?

Vladimir Sementsov-Ogievskiy (3):
   block: use int64_t as bytes type in tracked requests
   block/io: convert generic io path to use int64_t parameters
   block: use int64_t instead of uint64_t in driver handlers

  include/block/block.h |  8 ++--
  include/block/block_int.h | 52 ++---
  block/backup-top.c|  5 +-
  block/blkdebug.c  |  4 +-
  block/blklogwrites.c  |  4 +-
  block/blkreplay.c |  4 +-
  block/blkverify.c |  6 +--
  block/bochs.c |  2 +-
  block/cloop.c |  2 +-
  block/commit.c|  2 +-
  block/copy-on-read.c  |  4 +-
  block/crypto.c|  4 +-
  block/curl.c  |  2 +-
  block/dmg.c   |  2 +-
  block/file-posix.c| 18 
  block/filter-compress.c   |  6 +--
  block/io.c| 97 ---
  block/iscsi.c | 12 ++---
  block/mirror.c|  4 +-
  block/nbd.c   |  8 ++--
  block/nfs.c   |  8 ++--
  block/null.c  |  8 ++--
  block/nvme.c  |  4 +-
  block/qcow.c  | 12 ++---
  block/qcow2.c | 18 
  block/quorum.c|  8 ++--
  block/raw-format.c| 28 +--
  block/rbd.c   |  4 +-
  block/throttle.c  |  4 +-
  block/vdi.c   |  4 +-
  block/vmdk.c  |  8 ++--
  block/vpc.c   |  4 +-
  block/vvfat.c |  6 +--
  tests/test-bdrv-drain.c   |  8 ++--
  block/trace-events| 14 +++---
  35 files changed, 192 insertions(+), 192 deletions(-)




--
Best regards,
Vladimir



Re: [RFC 0/3] 64bit block-layer part I

2020-04-22 Thread Stefan Hajnoczi
On Mon, Mar 30, 2020 at 05:18:15PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> There is an idea to make NBD protocol extension to support 64bit
> write-zero/discard/block-status commands (commands, which doesn't
> transfer user data). It's needed to increase performance of zeroing
> large ranges (up to the whole image). Zeroing of the whole image is used
> as first step of mirror job, qemu-img convert, it should be also used at
> start of backup actually..
> 
> We need to support it in block-layer, so we want 64bit write_zeros.
> Currently driver handler now have int bytes parameter.
> 
> write_zeros path goes through normal pwritev, so we need 64bit write,
> and then we need 64bit read for symmetry, and better, let's make all io
> path work with 64bit bytes parameter.
> 
> Actually most of block-layer already have 64bit parameters: offset is
> sometimes int64_t and sometimes uint64_t. bytes parameter is one of
> int64_t, uint64_t, int, unsigned int...
> 
> I think we need one type for all of this, and this one type is int64_t.
> Signed int64_t is a bit better than uint64_t: you can use same variable
> to get some result (including error < 0) and than reuse it as an
> argument without any type conversion.
> 
> So, I propose, as a first step, convert all uint64_t parameters to
> int64_t.
> 
> Still, I don't have good idea of how to split this into more than 3
> patches, so, this is an RFC.
> 
> What's next?
> 
> Converting write_zero and discard is not as simple: we can't just
> s/int/uint64_t/, as some functions may use some int variables for
> calculations and this will be broken by something larger than int.
> 
> So, I think the simplest way is to add .bdrv_co_pwritev_zeros64 and
> .bdrv_co_pdiscard64 and update drivers one-by-one. If at some point all
> drivers updated - drop unused 32bit functions, and then drop "64" suffix
> from API. If not - we'll live with both APIs.
> 
> Another thing to do is updating default limiting of request (currently
> they are limited to INT_MAX).
> 
> Then we may move some drivers to 64bit discard/write_zero: I think about
> qcow2, file-posix and nbd (as a proof-of-concept for already proposed
> NBD extension).

Sounds good to me.  This is a good series to merge early in the QEMU 5.1
release cycle.  That way we'll have time to catch any issues.

Stefan


signature.asc
Description: PGP signature


Re: [RFC 0/3] 64bit block-layer part I

2020-04-22 Thread Eric Blake

On 4/22/20 9:29 AM, Vladimir Sementsov-Ogievskiy wrote:
Any thoughts here? I need to resend to update some more functions as 
patchew said.


Is it OK in general? Or should we instead convert everything to uint64_t ?


I definitely prefer int64_t as our base (off_t is signed as well, making 
63 bits an effective limit everywhere).




30.03.2020 17:18, Vladimir Sementsov-Ogievskiy wrote:

Hi all!

There is an idea to make NBD protocol extension to support 64bit
write-zero/discard/block-status commands (commands, which doesn't
transfer user data). It's needed to increase performance of zeroing
large ranges (up to the whole image). Zeroing of the whole image is used
as first step of mirror job, qemu-img convert, it should be also used at
start of backup actually..

We need to support it in block-layer, so we want 64bit write_zeros.
Currently driver handler now have int bytes parameter.

write_zeros path goes through normal pwritev, so we need 64bit write,
and then we need 64bit read for symmetry, and better, let's make all io
path work with 64bit bytes parameter.

Actually most of block-layer already have 64bit parameters: offset is
sometimes int64_t and sometimes uint64_t. bytes parameter is one of
int64_t, uint64_t, int, unsigned int...

I think we need one type for all of this, and this one type is int64_t.
Signed int64_t is a bit better than uint64_t: you can use same variable
to get some result (including error < 0) and than reuse it as an
argument without any type conversion.

So, I propose, as a first step, convert all uint64_t parameters to
int64_t.


Makes sense.  I haven't looked at the series closely in part because it 
was 5.1 material while we were still focused on getting 5.0 out the 
door, but it is now raising in my review queue again.


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




Re: [RFC 0/3] 64bit block-layer part I

2020-04-22 Thread Vladimir Sementsov-Ogievskiy

Any thoughts here? I need to resend to update some more functions as patchew 
said.

Is it OK in general? Or should we instead convert everything to uint64_t ?

30.03.2020 17:18, Vladimir Sementsov-Ogievskiy wrote:

Hi all!

There is an idea to make NBD protocol extension to support 64bit
write-zero/discard/block-status commands (commands, which doesn't
transfer user data). It's needed to increase performance of zeroing
large ranges (up to the whole image). Zeroing of the whole image is used
as first step of mirror job, qemu-img convert, it should be also used at
start of backup actually..

We need to support it in block-layer, so we want 64bit write_zeros.
Currently driver handler now have int bytes parameter.

write_zeros path goes through normal pwritev, so we need 64bit write,
and then we need 64bit read for symmetry, and better, let's make all io
path work with 64bit bytes parameter.

Actually most of block-layer already have 64bit parameters: offset is
sometimes int64_t and sometimes uint64_t. bytes parameter is one of
int64_t, uint64_t, int, unsigned int...

I think we need one type for all of this, and this one type is int64_t.
Signed int64_t is a bit better than uint64_t: you can use same variable
to get some result (including error < 0) and than reuse it as an
argument without any type conversion.

So, I propose, as a first step, convert all uint64_t parameters to
int64_t.

Still, I don't have good idea of how to split this into more than 3
patches, so, this is an RFC.

What's next?

Converting write_zero and discard is not as simple: we can't just
s/int/uint64_t/, as some functions may use some int variables for
calculations and this will be broken by something larger than int.

So, I think the simplest way is to add .bdrv_co_pwritev_zeros64 and
.bdrv_co_pdiscard64 and update drivers one-by-one. If at some point all
drivers updated - drop unused 32bit functions, and then drop "64" suffix
from API. If not - we'll live with both APIs.

Another thing to do is updating default limiting of request (currently
they are limited to INT_MAX).

Then we may move some drivers to 64bit discard/write_zero: I think about
qcow2, file-posix and nbd (as a proof-of-concept for already proposed
NBD extension).

Any ideas?

Vladimir Sementsov-Ogievskiy (3):
   block: use int64_t as bytes type in tracked requests
   block/io: convert generic io path to use int64_t parameters
   block: use int64_t instead of uint64_t in driver handlers

  include/block/block.h |  8 ++--
  include/block/block_int.h | 52 ++---
  block/backup-top.c|  5 +-
  block/blkdebug.c  |  4 +-
  block/blklogwrites.c  |  4 +-
  block/blkreplay.c |  4 +-
  block/blkverify.c |  6 +--
  block/bochs.c |  2 +-
  block/cloop.c |  2 +-
  block/commit.c|  2 +-
  block/copy-on-read.c  |  4 +-
  block/crypto.c|  4 +-
  block/curl.c  |  2 +-
  block/dmg.c   |  2 +-
  block/file-posix.c| 18 
  block/filter-compress.c   |  6 +--
  block/io.c| 97 ---
  block/iscsi.c | 12 ++---
  block/mirror.c|  4 +-
  block/nbd.c   |  8 ++--
  block/nfs.c   |  8 ++--
  block/null.c  |  8 ++--
  block/nvme.c  |  4 +-
  block/qcow.c  | 12 ++---
  block/qcow2.c | 18 
  block/quorum.c|  8 ++--
  block/raw-format.c| 28 +--
  block/rbd.c   |  4 +-
  block/throttle.c  |  4 +-
  block/vdi.c   |  4 +-
  block/vmdk.c  |  8 ++--
  block/vpc.c   |  4 +-
  block/vvfat.c |  6 +--
  tests/test-bdrv-drain.c   |  8 ++--
  block/trace-events| 14 +++---
  35 files changed, 192 insertions(+), 192 deletions(-)




--
Best regards,
Vladimir



Re: [RFC 0/3] 64bit block-layer part I

2020-03-30 Thread no-reply
Patchew URL: 
https://patchew.org/QEMU/20200330141818.31294-1-vsement...@virtuozzo.com/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC  block/nbd.o
  CC  block/sheepdog.o
  CC  block/accounting.o
/tmp/qemu-test/src/block/file-win32.c:643:27: error: initialization of 
'BlockAIOCB * (*)(BlockDriverState *, int64_t,  int64_t,  QEMUIOVector *, int,  
void (*)(void *, int), void *)' {aka 'struct BlockAIOCB * (*)(struct 
BlockDriverState *, long long int,  long long int,  struct QEMUIOVector *, int, 
 void (*)(void *, int), void *)'} from incompatible pointer type 'BlockAIOCB * 
(*)(BlockDriverState *, uint64_t,  uint64_t,  QEMUIOVector *, int,  void 
(*)(void *, int), void *)' {aka 'struct BlockAIOCB * (*)(struct 
BlockDriverState *, long long unsigned int,  long long unsigned int,  struct 
QEMUIOVector *, int,  void (*)(void *, int), void *)'} 
[-Werror=incompatible-pointer-types]
 .bdrv_aio_preadv= raw_aio_preadv,
   ^~
/tmp/qemu-test/src/block/file-win32.c:643:27: note: (near initialization for 
'bdrv_file.bdrv_aio_preadv')
/tmp/qemu-test/src/block/file-win32.c:644:27: error: initialization of 
'BlockAIOCB * (*)(BlockDriverState *, int64_t,  int64_t,  QEMUIOVector *, int,  
void (*)(void *, int), void *)' {aka 'struct BlockAIOCB * (*)(struct 
BlockDriverState *, long long int,  long long int,  struct QEMUIOVector *, int, 
 void (*)(void *, int), void *)'} from incompatible pointer type 'BlockAIOCB * 
(*)(BlockDriverState *, uint64_t,  uint64_t,  QEMUIOVector *, int,  void 
(*)(void *, int), void *)' {aka 'struct BlockAIOCB * (*)(struct 
BlockDriverState *, long long unsigned int,  long long unsigned int,  struct 
QEMUIOVector *, int,  void (*)(void *, int), void *)'} 
[-Werror=incompatible-pointer-types]
 .bdrv_aio_pwritev   = raw_aio_pwritev,
   ^~~
/tmp/qemu-test/src/block/file-win32.c:644:27: note: (near initialization for 
'bdrv_file.bdrv_aio_pwritev')
/tmp/qemu-test/src/block/file-win32.c:812:27: error: initialization of 
'BlockAIOCB * (*)(BlockDriverState *, int64_t,  int64_t,  QEMUIOVector *, int,  
void (*)(void *, int), void *)' {aka 'struct BlockAIOCB * (*)(struct 
BlockDriverState *, long long int,  long long int,  struct QEMUIOVector *, int, 
 void (*)(void *, int), void *)'} from incompatible pointer type 'BlockAIOCB * 
(*)(BlockDriverState *, uint64_t,  uint64_t,  QEMUIOVector *, int,  void 
(*)(void *, int), void *)' {aka 'struct BlockAIOCB * (*)(struct 
BlockDriverState *, long long unsigned int,  long long unsigned int,  struct 
QEMUIOVector *, int,  void (*)(void *, int), void *)'} 
[-Werror=incompatible-pointer-types]
 .bdrv_aio_preadv= raw_aio_preadv,
   ^~
/tmp/qemu-test/src/block/file-win32.c:812:27: note: (near initialization for 
'bdrv_host_device.bdrv_aio_preadv')
/tmp/qemu-test/src/block/file-win32.c:813:27: error: initialization of 
'BlockAIOCB * (*)(BlockDriverState *, int64_t,  int64_t,  QEMUIOVector *, int,  
void (*)(void *, int), void *)' {aka 'struct BlockAIOCB * (*)(struct 
BlockDriverState *, long long int,  long long int,  struct QEMUIOVector *, int, 
 void (*)(void *, int), void *)'} from incompatible pointer type 'BlockAIOCB * 
(*)(BlockDriverState *, uint64_t,  uint64_t,  QEMUIOVector *, int,  void 
(*)(void *, int), void *)' {aka 'struct BlockAIOCB * (*)(struct 
BlockDriverState *, long long unsigned int,  long long unsigned int,  struct 
QEMUIOVector *, int,  void (*)(void *, int), void *)'} 
[-Werror=incompatible-pointer-types]
 .bdrv_aio_pwritev   = raw_aio_pwritev,
   ^~~
/tmp/qemu-test/src/block/file-win32.c:813:27: note: (near initialization for 
'bdrv_host_device.bdrv_aio_pwritev')
cc1: all warnings being treated as errors
make: *** [/tmp/qemu-test/src/rules.mak:69: block/file-win32.o] Error 1
make: *** Waiting for unfinished jobs
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 664, in 
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', 
'--label', 'com.qemu.instance.uuid=46f0957c6a3a47e5971905cb30141048', '-u', 
'1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', 
'-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 
'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', 
'/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', 
'/var/tmp/patchew-tester-tmp-0szyhgs2/src/docker-src.2020-03-30-13.48.43.9903:/var/tmp/qemu:z,ro',
 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit 
status 2.

Re: [RFC 0/3] 64bit block-layer part I

2020-03-30 Thread no-reply
Patchew URL: 
https://patchew.org/QEMU/20200330141818.31294-1-vsement...@virtuozzo.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC  tests/test-int128.o
  CC  tests/rcutorture.o
  CC  tests/test-rcu-list.o
/tmp/qemu-test/src/tests/test-block-iothread.c:68:5: error: initialization from 
incompatible pointer type [-Werror]
 .bdrv_co_preadv = bdrv_test_co_prwv,
 ^
/tmp/qemu-test/src/tests/test-block-iothread.c:68:5: error: (near 
initialization for 'bdrv_test.bdrv_co_preadv') [-Werror]
/tmp/qemu-test/src/tests/test-block-iothread.c:69:5: error: initialization from 
incompatible pointer type [-Werror]
 .bdrv_co_pwritev= bdrv_test_co_prwv,
 ^
/tmp/qemu-test/src/tests/test-block-iothread.c:69:5: error: (near 
initialization for 'bdrv_test.bdrv_co_pwritev') [-Werror]
cc1: all warnings being treated as errors
make: *** [tests/test-block-iothread.o] Error 1
make: *** Waiting for unfinished jobs
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 664, in 
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', 
'--label', 'com.qemu.instance.uuid=ac3e5859c1b54c68988c07cda02958cd', '-u', 
'1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', 
'-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 
'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', 
'/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', 
'/var/tmp/patchew-tester-tmp-4cxd8d3k/src/docker-src.2020-03-30-13.44.40.29863:/var/tmp/qemu:z,ro',
 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit 
status 2.
filter=--filter=label=com.qemu.instance.uuid=ac3e5859c1b54c68988c07cda02958cd
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-4cxd8d3k/src'
make: *** [docker-run-test-quick@centos7] Error 2

real3m30.965s
user0m8.371s


The full log is available at
http://patchew.org/logs/20200330141818.31294-1-vsement...@virtuozzo.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [RFC 0/3] 64bit block-layer part I

2020-03-30 Thread no-reply
Patchew URL: 
https://patchew.org/QEMU/20200330141818.31294-1-vsement...@virtuozzo.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

clang -iquote /tmp/qemu-test/build/tests -iquote tests -iquote 
/tmp/qemu-test/src/tcg/i386 -isystem /tmp/qemu-test/src/linux-headers -isystem 
/tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote 
/tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -iquote 
/tmp/qemu-test/src/disas/libvixl -I/usr/include/pixman-1   -Werror 
-fsanitize=undefined -fsanitize=address  -pthread -I/usr/include/glib-2.0 
-I/usr/lib64/glib-2.0/include  -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE 
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
-Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes 
-fno-strict-aliasing -fno-common -fwrapv -std=gnu99  -Wno-string-plus-int 
-Wno-typedef-redefinition -Wno-initializer-overrides -Wexpansion-to-defined 
-Wendif-labels -Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body 
-Wnested-externs -Wformat-security -Wformat-y2k -Winit-self 
-Wignored-qualifiers -Wold-style-definition -Wtype-limits 
-fstack-protector-strong   -I/usr/include/p11-kit-1   -DLEGACY_RDMA_REG_MR 
-DSTRUCT_IOVEC_DEFINED  -I/usr/include/libpng16  -I/usr/include/spice-1 
-I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 
-I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread 
-I/usr/include/libmount -I/usr/include/blkid -I/usr/include/uuid 
-I/usr/include/pixman-1   -I/tmp/qemu-test/src/tests 
-I/tmp/qemu-test/src/tests/qtest -MMD -MP -MT tests/test-crypto-hash.o -MF 
tests/test-crypto-hash.d -g   -c -o tests/test-crypto-hash.o 
/tmp/qemu-test/src/tests/test-crypto-hash.c
clang -iquote /tmp/qemu-test/build/tests -iquote tests -iquote 
/tmp/qemu-test/src/tcg/i386 -isystem /tmp/qemu-test/src/linux-headers -isystem 
/tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote 
/tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -iquote 
/tmp/qemu-test/src/disas/libvixl -I/usr/include/pixman-1   -Werror 
-fsanitize=undefined -fsanitize=address  -pthread -I/usr/include/glib-2.0 
-I/usr/lib64/glib-2.0/include  -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE 
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
-Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes 
-fno-strict-aliasing -fno-common -fwrapv -std=gnu99  -Wno-string-plus-int 
-Wno-typedef-redefinition -Wno-initializer-overrides -Wexpansion-to-defined 
-Wendif-labels -Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body 
-Wnested-externs -Wformat-security -Wformat-y2k -Winit-self 
-Wignored-qualifiers -Wold-style-definition -Wtype-limits 
-fstack-protector-strong   -I/usr/include/p11-kit-1   -DLEGACY_RDMA_REG_MR 
-DSTRUCT_IOVEC_DEFINED  -I/usr/include/libpng16  -I/usr/include/spice-1 
-I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 
-I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread 
-I/usr/include/libmount -I/usr/include/blkid -I/usr/include/uuid 
-I/usr/include/pixman-1   -I/tmp/qemu-test/src/tests 
-I/tmp/qemu-test/src/tests/qtest -MMD -MP -MT tests/test-crypto-hmac.o -MF 
tests/test-crypto-hmac.d -g   -c -o tests/test-crypto-hmac.o 
/tmp/qemu-test/src/tests/test-crypto-hmac.c
clang -iquote /tmp/qemu-test/build/tests -iquote tests -iquote 
/tmp/qemu-test/src/tcg/i386 -isystem /tmp/qemu-test/src/linux-headers -isystem 
/tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote 
/tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -iquote 
/tmp/qemu-test/src/disas/libvixl -I/usr/include/pixman-1   -Werror 
-fsanitize=undefined -fsanitize=address  -pthread -I/usr/include/glib-2.0 
-I/usr/lib64/glib-2.0/include  -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE 
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
-Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes 
-fno-strict-aliasing -fno-common -fwrapv -std=gnu99  -Wno-string-plus-int 
-Wno-typedef-redefinition -Wno-initializer-overrides -Wexpansion-to-defined 
-Wendif-labels -Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body 
-Wnested-externs -Wformat-security -Wformat-y2k -Winit-self 
-Wignored-qualifiers -Wold-style-definition -Wtype-limits 
-fstack-protector-strong   -I/usr/include/p11-kit-1   -DLEGACY_RDMA_REG_MR 
-DSTRUCT_IOVEC_DEFINED  -I/usr/include/libpng16  -I/usr/include/spice-1 
-I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 
-I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread 
-I/usr/include/libmount 

[RFC 0/3] 64bit block-layer part I

2020-03-30 Thread Vladimir Sementsov-Ogievskiy
Hi all!

There is an idea to make NBD protocol extension to support 64bit
write-zero/discard/block-status commands (commands, which doesn't
transfer user data). It's needed to increase performance of zeroing
large ranges (up to the whole image). Zeroing of the whole image is used
as first step of mirror job, qemu-img convert, it should be also used at
start of backup actually..

We need to support it in block-layer, so we want 64bit write_zeros.
Currently driver handler now have int bytes parameter.

write_zeros path goes through normal pwritev, so we need 64bit write,
and then we need 64bit read for symmetry, and better, let's make all io
path work with 64bit bytes parameter.

Actually most of block-layer already have 64bit parameters: offset is
sometimes int64_t and sometimes uint64_t. bytes parameter is one of
int64_t, uint64_t, int, unsigned int...

I think we need one type for all of this, and this one type is int64_t.
Signed int64_t is a bit better than uint64_t: you can use same variable
to get some result (including error < 0) and than reuse it as an
argument without any type conversion.

So, I propose, as a first step, convert all uint64_t parameters to
int64_t.

Still, I don't have good idea of how to split this into more than 3
patches, so, this is an RFC.

What's next?

Converting write_zero and discard is not as simple: we can't just
s/int/uint64_t/, as some functions may use some int variables for
calculations and this will be broken by something larger than int.

So, I think the simplest way is to add .bdrv_co_pwritev_zeros64 and
.bdrv_co_pdiscard64 and update drivers one-by-one. If at some point all
drivers updated - drop unused 32bit functions, and then drop "64" suffix
from API. If not - we'll live with both APIs.

Another thing to do is updating default limiting of request (currently
they are limited to INT_MAX).

Then we may move some drivers to 64bit discard/write_zero: I think about
qcow2, file-posix and nbd (as a proof-of-concept for already proposed
NBD extension).

Any ideas?

Vladimir Sementsov-Ogievskiy (3):
  block: use int64_t as bytes type in tracked requests
  block/io: convert generic io path to use int64_t parameters
  block: use int64_t instead of uint64_t in driver handlers

 include/block/block.h |  8 ++--
 include/block/block_int.h | 52 ++---
 block/backup-top.c|  5 +-
 block/blkdebug.c  |  4 +-
 block/blklogwrites.c  |  4 +-
 block/blkreplay.c |  4 +-
 block/blkverify.c |  6 +--
 block/bochs.c |  2 +-
 block/cloop.c |  2 +-
 block/commit.c|  2 +-
 block/copy-on-read.c  |  4 +-
 block/crypto.c|  4 +-
 block/curl.c  |  2 +-
 block/dmg.c   |  2 +-
 block/file-posix.c| 18 
 block/filter-compress.c   |  6 +--
 block/io.c| 97 ---
 block/iscsi.c | 12 ++---
 block/mirror.c|  4 +-
 block/nbd.c   |  8 ++--
 block/nfs.c   |  8 ++--
 block/null.c  |  8 ++--
 block/nvme.c  |  4 +-
 block/qcow.c  | 12 ++---
 block/qcow2.c | 18 
 block/quorum.c|  8 ++--
 block/raw-format.c| 28 +--
 block/rbd.c   |  4 +-
 block/throttle.c  |  4 +-
 block/vdi.c   |  4 +-
 block/vmdk.c  |  8 ++--
 block/vpc.c   |  4 +-
 block/vvfat.c |  6 +--
 tests/test-bdrv-drain.c   |  8 ++--
 block/trace-events| 14 +++---
 35 files changed, 192 insertions(+), 192 deletions(-)

-- 
2.21.0