[v1] NBD patches through 2021-09-27

[PULL 00/20] NBD patches through 2021-09-27

Posted by Eric Blake 4 years ago

The following changes since commit 9b03a1178204598055f23f24e438fdddb5935df9:

  Merge remote-tracking branch 'remotes/vivier2/tags/trivial-branch-for-6.2-pull-request' into staging (2021-09-27 11:08:36 +0100)

are available in the Git repository at:

  https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2021-09-27

for you to fetch changes up to 3cb015ad05c7c1e07e0deb356cd20e6cd765c0ea:

  nbd/server: Add --selinux-label option (2021-09-27 16:16:28 -0500)

----------------------------------------------------------------
nbd patches for 2021-09-27

- Richard W.M. Jones: Add --selinux-label option to qemu-nbd
- Vladimir Sementsov-Ogievskiy: Rework coroutines of qemu NBD client
  to improve reconnect support
- Eric Blake: Relax server in regards to NBD_OPT_LIST_META_CONTEXT
- Vladimir Sementsov-Ogievskiy: Plumb up 64-bit bulk-zeroing support
  in block layer, in preparation for future NBD spec extensions
- Nir Soffer: Default to writeback cache in qemu-nbd

----------------------------------------------------------------
Eric Blake (1):
      nbd/server: Allow LIST_META_CONTEXT without STRUCTURED_REPLY

Nir Soffer (1):
      qemu-nbd: Change default cache mode to writeback

Richard W.M. Jones (1):
      nbd/server: Add --selinux-label option

Vladimir Sementsov-Ogievskiy (17):
      block/io: bring request check to bdrv_co_(read,write)v_vmstate
      qcow2: check request on vmstate save/load path
      block: use int64_t instead of uint64_t in driver read handlers
      block: use int64_t instead of uint64_t in driver write handlers
      block: use int64_t instead of uint64_t in copy_range driver handlers
      block: make BlockLimits::max_pwrite_zeroes 64bit
      block: use int64_t instead of int in driver write_zeroes handlers
      block/io: allow 64bit write-zeroes requests
      block: make BlockLimits::max_pdiscard 64bit
      block: use int64_t instead of int in driver discard handlers
      block/io: allow 64bit discard requests
      nbd/client-connection: nbd_co_establish_connection(): fix non set errp
      block/nbd: nbd_channel_error() shutdown channel unconditionally
      block/nbd: move nbd_recv_coroutines_wake_all() up
      block/nbd: refactor nbd_recv_coroutines_wake_all()
      block/nbd: drop connection_co
      block/nbd: check that received handle is valid

 docs/tools/qemu-nbd.rst                       |   6 +-
 configure                                     |   8 +-
 meson.build                                   |  10 +-
 include/block/block_int.h                     |  66 ++--
 block/io.c                                    |  44 ++-
 block/blkdebug.c                              |  12 +-
 block/blklogwrites.c                          |  16 +-
 block/blkreplay.c                             |   8 +-
 block/blkverify.c                             |   8 +-
 block/bochs.c                                 |   4 +-
 block/cloop.c                                 |   4 +-
 block/commit.c                                |   2 +-
 block/copy-before-write.c                     |  15 +-
 block/copy-on-read.c                          |  19 +-
 block/crypto.c                                |   8 +-
 block/curl.c                                  |   3 +-
 block/dmg.c                                   |   4 +-
 block/file-posix.c                            |  35 +-
 block/file-win32.c                            |   8 +-
 block/filter-compress.c                       |  15 +-
 block/gluster.c                               |  13 +-
 block/iscsi.c                                 |  58 ++--
 block/mirror.c                                |   8 +-
 block/nbd.c                                   | 443 ++++++++------------------
 block/nfs.c                                   |  12 +-
 block/null.c                                  |  18 +-
 block/nvme.c                                  |  48 ++-
 block/preallocate.c                           |  14 +-
 block/qcow.c                                  |  16 +-
 block/qcow2-cluster.c                         |  14 +-
 block/qcow2.c                                 |  70 ++--
 block/qed.c                                   |   9 +-
 block/quorum.c                                |  11 +-
 block/raw-format.c                            |  36 +--
 block/rbd.c                                   |  20 +-
 block/throttle.c                              |  18 +-
 block/vdi.c                                   |   8 +-
 block/vmdk.c                                  |  14 +-
 block/vpc.c                                   |   8 +-
 block/vvfat.c                                 |   8 +-
 nbd/client-connection.c                       |   1 +
 nbd/client.c                                  |   2 -
 nbd/server.c                                  |   2 +-
 qemu-nbd.c                                    |  45 ++-
 tests/unit/test-bdrv-drain.c                  |  16 +-
 tests/unit/test-block-iothread.c              |  21 +-
 block/trace-events                            |  10 +-
 meson_options.txt                             |   3 +
 tests/docker/dockerfiles/centos8.docker       |   1 +
 tests/docker/dockerfiles/fedora.docker        |   1 +
 tests/docker/dockerfiles/opensuse-leap.docker |   1 +
 tests/docker/dockerfiles/ubuntu1804.docker    |   1 +
 tests/docker/dockerfiles/ubuntu2004.docker    |   1 +
 53 files changed, 648 insertions(+), 598 deletions(-)

-- 
2.31.1

Re: [PULL 00/20] NBD patches through 2021-09-27

Posted by Peter Maydell 4 years ago

On Mon, 27 Sept 2021 at 23:02, Eric Blake <eblake@redhat.com> wrote:
>
> The following changes since commit 9b03a1178204598055f23f24e438fdddb5935df9:
>
>   Merge remote-tracking branch 'remotes/vivier2/tags/trivial-branch-for-6.2-pull-request' into staging (2021-09-27 11:08:36 +0100)
>
> are available in the Git repository at:
>
>   https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2021-09-27
>
> for you to fetch changes up to 3cb015ad05c7c1e07e0deb356cd20e6cd765c0ea:
>
>   nbd/server: Add --selinux-label option (2021-09-27 16:16:28 -0500)
>
> ----------------------------------------------------------------
> nbd patches for 2021-09-27
>
> - Richard W.M. Jones: Add --selinux-label option to qemu-nbd
> - Vladimir Sementsov-Ogievskiy: Rework coroutines of qemu NBD client
>   to improve reconnect support
> - Eric Blake: Relax server in regards to NBD_OPT_LIST_META_CONTEXT
> - Vladimir Sementsov-Ogievskiy: Plumb up 64-bit bulk-zeroing support
>   in block layer, in preparation for future NBD spec extensions
> - Nir Soffer: Default to writeback cache in qemu-nbd
>
> ----------------------------------------------------------------

This seems to break the gitlab cross-i386-system build,
which now fails to link qemu-nbd because it is trying
to link the x86-64 libselinux.so into a 32-bit binary:

https://gitlab.com/qemu-project/qemu/-/jobs/1630661323

cc -o qemu-nbd qemu-nbd.p/qemu-nbd.c.o -Wl,--as-needed
-Wl,--no-undefined -pie -Wl,--whole-archive libblockdev.fa libblock.fa
libcrypto.fa libauthz.fa libqom.fa libio.fa -Wl,--no-whole-archive
-Wl,--warn-common -Wl,-z,relro -Wl,-z,now -m32 -m32
-fstack-protector-strong -Wl,--start-group libqemuutil.a
libblockdev.fa libblock.fa libcrypto.fa libauthz.fa libqom.fa libio.fa
@block.syms /usr/lib/libgnutls.so /usr/lib64/libselinux.so -lutil
-L/usr/lib -lgio-2.0 -lgobject-2.0 -lglib-2.0 -L/usr/lib -lgio-2.0
-lgobject-2.0 -lglib-2.0 -lm -pthread -L/usr/lib -lgmodule-2.0
-lglib-2.0 /usr/lib/libpixman-1.so /usr/lib/libzstd.so
/usr/lib/libz.so -Wl,--end-group
/usr/bin/ld: /usr/lib64/libselinux.so: error adding symbols: file in
wrong format
collect2: error: ld returned 1 exit status


-- PMM

Re: [PULL 00/20] NBD patches through 2021-09-27

Posted by Paolo Bonzini 4 years ago

On 29/09/21 10:59, Peter Maydell wrote:
> This seems to break the gitlab cross-i386-system build,
> which now fails to link qemu-nbd because it is trying
> to link the x86-64 libselinux.so into a 32-bit binary:
> 
> https://gitlab.com/qemu-project/qemu/-/jobs/1630661323
> 
> cc -o qemu-nbd qemu-nbd.p/qemu-nbd.c.o -Wl,--as-needed
> -Wl,--no-undefined -pie -Wl,--whole-archive libblockdev.fa libblock.fa
> libcrypto.fa libauthz.fa libqom.fa libio.fa -Wl,--no-whole-archive
> -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -m32 -m32
> -fstack-protector-strong -Wl,--start-group libqemuutil.a
> libblockdev.fa libblock.fa libcrypto.fa libauthz.fa libqom.fa libio.fa
> @block.syms /usr/lib/libgnutls.so /usr/lib64/libselinux.so -lutil
> -L/usr/lib -lgio-2.0 -lgobject-2.0 -lglib-2.0 -L/usr/lib -lgio-2.0
> -lgobject-2.0 -lglib-2.0 -lm -pthread -L/usr/lib -lgmodule-2.0
> -lglib-2.0 /usr/lib/libpixman-1.so /usr/lib/libzstd.so
> /usr/lib/libz.so -Wl,--end-group
> /usr/bin/ld: /usr/lib64/libselinux.so: error adding symbols: file in
> wrong format
> collect2: error: ld returned 1 exit status

Missing libselinux-devel.i686 in 
tests/docker/dockerfiles/fedora-i386-cross.docker, I think?

Paolo

Re: [PULL 00/20] NBD patches through 2021-09-27

Posted by Richard Henderson 4 years ago

On Wed, 29 Sep 2021, 08:42 Paolo Bonzini, <pbonzini@redhat.com> wrote:

> On 29/09/21 10:59, Peter Maydell wrote:
> > This seems to break the gitlab cross-i386-system build,
> > which now fails to link qemu-nbd because it is trying
> > to link the x86-64 libselinux.so into a 32-bit binary:
> >
> > https://gitlab.com/qemu-project/qemu/-/jobs/1630661323
> >
> > cc -o qemu-nbd qemu-nbd.p/qemu-nbd.c.o -Wl,--as-needed
> > -Wl,--no-undefined -pie -Wl,--whole-archive libblockdev.fa libblock.fa
> > libcrypto.fa libauthz.fa libqom.fa libio.fa -Wl,--no-whole-archive
> > -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -m32 -m32
> > -fstack-protector-strong -Wl,--start-group libqemuutil.a
> > libblockdev.fa libblock.fa libcrypto.fa libauthz.fa libqom.fa libio.fa
> > @block.syms /usr/lib/libgnutls.so /usr/lib64/libselinux.so -lutil
> > -L/usr/lib -lgio-2.0 -lgobject-2.0 -lglib-2.0 -L/usr/lib -lgio-2.0
> > -lgobject-2.0 -lglib-2.0 -lm -pthread -L/usr/lib -lgmodule-2.0
> > -lglib-2.0 /usr/lib/libpixman-1.so /usr/lib/libzstd.so
> > /usr/lib/libz.so -Wl,--end-group
> > /usr/bin/ld: /usr/lib64/libselinux.so: error adding symbols: file in
> > wrong format
> > collect2: error: ld returned 1 exit status
>
> Missing libselinux-devel.i686 in
> tests/docker/dockerfiles/fedora-i386-cross.docker, I think?
>

But additionally, incorrect package probing, I think.

r~

>

Re: [PULL 00/20] NBD patches through 2021-09-27

Posted by Paolo Bonzini 4 years ago

On 29/09/21 15:58, Richard Henderson wrote:
> 
>      > /usr/bin/ld: /usr/lib64/libselinux.so: error adding symbols: file in
>      > wrong format
>      > collect2: error: ld returned 1 exit status
> 
>     Missing libselinux-devel.i686 in
>     tests/docker/dockerfiles/fedora-i386-cross.docker, I think?
> 
> But additionally, incorrect package probing, I think.

Probably Meson deciding to look at --print-search-dirs and crossing 
fingers.  But -m32 and other multilib flags should be added to 
config-meson.cross rather than QEMU_CFLAGS.

Paolo

Re: [PULL 00/20] NBD patches through 2021-09-27

Posted by Eric Blake 4 years ago

On Wed, Sep 29, 2021 at 05:03:08PM +0200, Paolo Bonzini wrote:
> On 29/09/21 15:58, Richard Henderson wrote:
> > 
> >      > /usr/bin/ld: /usr/lib64/libselinux.so: error adding symbols: file in
> >      > wrong format
> >      > collect2: error: ld returned 1 exit status
> > 
> >     Missing libselinux-devel.i686 in
> >     tests/docker/dockerfiles/fedora-i386-cross.docker, I think?
> > 
> > But additionally, incorrect package probing, I think.
> 
> Probably Meson deciding to look at --print-search-dirs and crossing fingers.
> But -m32 and other multilib flags should be added to config-meson.cross
> rather than QEMU_CFLAGS.

Rich, Dan, this is caused by 'nbd/server: Add --selinux-label option'
(20/20 in this pull request); can you investigate?

In the short term, I'm leaning towards withdrawing that patch from the
pull request, and getting everything else upstream; we can revisit a
fixed version of that patch for my next pull request.

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

Re: [PULL 00/20] NBD patches through 2021-09-27

Posted by Richard W.M. Jones 4 years ago

On Wed, Sep 29, 2021 at 01:29:21PM -0500, Eric Blake wrote:
> On Wed, Sep 29, 2021 at 05:03:08PM +0200, Paolo Bonzini wrote:
> > On 29/09/21 15:58, Richard Henderson wrote:
> > > 
> > >      > /usr/bin/ld: /usr/lib64/libselinux.so: error adding symbols: file in
> > >      > wrong format
> > >      > collect2: error: ld returned 1 exit status
> > > 
> > >     Missing libselinux-devel.i686 in
> > >     tests/docker/dockerfiles/fedora-i386-cross.docker, I think?
> > > 
> > > But additionally, incorrect package probing, I think.
> > 
> > Probably Meson deciding to look at --print-search-dirs and crossing fingers.
> > But -m32 and other multilib flags should be added to config-meson.cross
> > rather than QEMU_CFLAGS.
> 
> Rich, Dan, this is caused by 'nbd/server: Add --selinux-label option'
> (20/20 in this pull request); can you investigate?
> 
> In the short term, I'm leaning towards withdrawing that patch from the
> pull request, and getting everything else upstream; we can revisit a
> fixed version of that patch for my next pull request.

Yes I'll have to look at this later, so withdraw it from the PR for now.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

Re: [PULL 00/20] NBD patches through 2021-09-27

Posted by Richard W.M. Jones 4 years ago

On Wed, Sep 29, 2021 at 01:29:21PM -0500, Eric Blake wrote:
> On Wed, Sep 29, 2021 at 05:03:08PM +0200, Paolo Bonzini wrote:
> > On 29/09/21 15:58, Richard Henderson wrote:
> > > 
> > >      > /usr/bin/ld: /usr/lib64/libselinux.so: error adding symbols: file in
> > >      > wrong format
> > >      > collect2: error: ld returned 1 exit status
> > > 
> > >     Missing libselinux-devel.i686 in
> > >     tests/docker/dockerfiles/fedora-i386-cross.docker, I think?

This part is easy to fix.

> > > But additionally, incorrect package probing, I think.
> > 
> > Probably Meson deciding to look at --print-search-dirs and crossing fingers.
> > But -m32 and other multilib flags should be added to config-meson.cross
> > rather than QEMU_CFLAGS.

However this part I've no idea.  The docker file uses:

  ENV QEMU_CONFIGURE_OPTS --extra-cflags=-m32 --disable-vhost-user

I emulated this locally using:

  PKG_CONFIG_LIBDIR=/usr/lib/pkgconfig ../configure --extra-cflags=-m32 --disable-vhost-user

but config-meson.cross does not have -m32 anywhere.  Nevertheless it
seemed to build a 32 bit qemu with selinux fine:

$ file ./qemu-system-x86_64
./qemu-system-x86_64: ELF 32-bit LSB pie executable, Intel 80386, version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux.so.2, BuildID[sha1]=1d070416c7d211f8bfa018557265c25e79a913bb, for GNU/Linux 3.2.0, with debug_info, not stripped
$ ldd ./qemu-system-x86_64 | grep selin
  libselinux.so.1 => /lib/libselinux.so.1 (0xf52b0000)

I will post v3 soon, but how can I test patches under your CI system?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

Re: [PULL 00/20] NBD patches through 2021-09-27

Posted by Richard Henderson 4 years ago

On 9/30/21 4:45 AM, Richard W.M. Jones wrote:
>    PKG_CONFIG_LIBDIR=/usr/lib/pkgconfig ../configure --extra-cflags=-m32 --disable-vhost-user

Not --extra-cflags, use --cpu=i386.


r~

Re: [PULL 00/20] NBD patches through 2021-09-27

Posted by Richard W.M. Jones 4 years ago

On Thu, Sep 30, 2021 at 10:27:45AM -0400, Richard Henderson wrote:
> On 9/30/21 4:45 AM, Richard W.M. Jones wrote:
> >   PKG_CONFIG_LIBDIR=/usr/lib/pkgconfig ../configure --extra-cflags=-m32 --disable-vhost-user
> 
> Not --extra-cflags, use --cpu=i386.

That also builds the 32 bit binary successfully.  config-meson.cross
doesn't contain -m32, if that matters.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

Re: [PULL 00/20] NBD patches through 2021-09-27

Posted by Daniel P. Berrangé 4 years ago

On Wed, Sep 29, 2021 at 01:29:21PM -0500, Eric Blake wrote:
> On Wed, Sep 29, 2021 at 05:03:08PM +0200, Paolo Bonzini wrote:
> > On 29/09/21 15:58, Richard Henderson wrote:
> > > 
> > >      > /usr/bin/ld: /usr/lib64/libselinux.so: error adding symbols: file in
> > >      > wrong format
> > >      > collect2: error: ld returned 1 exit status
> > > 
> > >     Missing libselinux-devel.i686 in
> > >     tests/docker/dockerfiles/fedora-i386-cross.docker, I think?
> > > 
> > > But additionally, incorrect package probing, I think.
> > 
> > Probably Meson deciding to look at --print-search-dirs and crossing fingers.
> > But -m32 and other multilib flags should be added to config-meson.cross
> > rather than QEMU_CFLAGS.
> 
> Rich, Dan, this is caused by 'nbd/server: Add --selinux-label option'
> (20/20 in this pull request); can you investigate?

I think this is a bug in the fedora-i386-cross.docker file

It sets

  ENV PKG_CONFIG_PATH /usr/lib/pkgconfig

which *adds* /usr/lib/pkgconfig to the pkg-config search path, in
additional to all existing search paths. IOW, it'll look in this
32-bit directory, and then carry on looking in all the 64-bit
directories.

For cross compiling it is neccessary to set PKG_CONFIG_LIBDIR
instead, which completely replaces the default paths.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|