[Qemu-devel] [PULL v2 0/9] bitmap export over NBD

Eric Blake posted 9 patches 5 years, 10 months ago
Only 3 patches received!
docs/interop/nbd.txt |  38 +++++
qapi/block.json      |  23 +++
include/block/nbd.h  |  11 +-
blockdev-nbd.c       |  23 +++
nbd/common.c         |   2 +
nbd/server.c         | 385 ++++++++++++++++++++++++++++++++++++++++++++-------
MAINTAINERS          |   1 +
nbd/trace-events     |   1 +
tests/.gitignore     |  93 +------------
9 files changed, 433 insertions(+), 144 deletions(-)
create mode 100644 docs/interop/nbd.txt
[Qemu-devel] [PULL v2 0/9] bitmap export over NBD
Posted by Eric Blake 5 years, 10 months ago
The following changes since commit 46012db666990ff2eed1d3dc199ab8006439a93b:

  Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20180619' into staging (2018-06-20 09:51:30 +0100)

are available in the Git repository at:

  git://repo.or.cz/qemu/ericb.git tags/pull-nbd-2018-06-20-v2

for you to fetch changes up to bc37b06a5cde24fb24c2a2cc44dd86756034ba9d:

  nbd/server: introduce NBD_CMD_CACHE (2018-06-21 09:41:39 -0500)

Only sending the new patches (2, 9) and the changed patch (6, was 5/7
in v1)

----------------------------------------------------------------
nbd patches for 2018-06-20

Add experimental x-nbd-server-add-bitmap to expose a disabled
bitmap over NBD, in preparation for a pull model incremental
backup scheme. Also fix a corner case protocol issue with
NBD_CMD_BLOCK_STATUS, and add new NBD_CMD_CACHE.

- Eric Blake: tests: Simplify .gitignore
- Eric Blake: nbd/server: Reject 0-length block status request
- Vladimir Sementsov-Ogievskiy: 0/6 NBD export bitmaps
- Vladimir Sementsov-Ogievskiy: nbd/server: introduce NBD_CMD_CACHE

----------------------------------------------------------------
Eric Blake (2):
      tests: Simplify .gitignore
      nbd/server: Reject 0-length block status request

Vladimir Sementsov-Ogievskiy (7):
      nbd/server: fix trace
      nbd/server: refactor NBDExportMetaContexts
      nbd/server: add nbd_meta_empty_or_pattern helper
      nbd/server: implement dirty bitmap export
      qapi: new qmp command nbd-server-add-bitmap
      docs/interop: add nbd.txt
      nbd/server: introduce NBD_CMD_CACHE

 docs/interop/nbd.txt |  38 +++++
 qapi/block.json      |  23 +++
 include/block/nbd.h  |  11 +-
 blockdev-nbd.c       |  23 +++
 nbd/common.c         |   2 +
 nbd/server.c         | 385 ++++++++++++++++++++++++++++++++++++++++++++-------
 MAINTAINERS          |   1 +
 nbd/trace-events     |   1 +
 tests/.gitignore     |  93 +------------
 9 files changed, 433 insertions(+), 144 deletions(-)
 create mode 100644 docs/interop/nbd.txt

-- 
2.14.4


Re: [Qemu-devel] [PULL v2 0/9] bitmap export over NBD
Posted by Peter Maydell 5 years, 10 months ago
On 21 June 2018 at 15:57, Eric Blake <eblake@redhat.com> wrote:
> The following changes since commit 46012db666990ff2eed1d3dc199ab8006439a93b:
>
>   Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20180619' into staging (2018-06-20 09:51:30 +0100)
>
> are available in the Git repository at:
>
>   git://repo.or.cz/qemu/ericb.git tags/pull-nbd-2018-06-20-v2
>
> for you to fetch changes up to bc37b06a5cde24fb24c2a2cc44dd86756034ba9d:
>
>   nbd/server: introduce NBD_CMD_CACHE (2018-06-21 09:41:39 -0500)
>
> Only sending the new patches (2, 9) and the changed patch (6, was 5/7
> in v1)
>
> ----------------------------------------------------------------
> nbd patches for 2018-06-20
>
> Add experimental x-nbd-server-add-bitmap to expose a disabled
> bitmap over NBD, in preparation for a pull model incremental
> backup scheme. Also fix a corner case protocol issue with
> NBD_CMD_BLOCK_STATUS, and add new NBD_CMD_CACHE.
>
> - Eric Blake: tests: Simplify .gitignore
> - Eric Blake: nbd/server: Reject 0-length block status request
> - Vladimir Sementsov-Ogievskiy: 0/6 NBD export bitmaps
> - Vladimir Sementsov-Ogievskiy: nbd/server: introduce NBD_CMD_CACHE
>
Applied, thanks.

-- PMM

Re: [Qemu-devel] [PULL v2 0/9] bitmap export over NBD
Posted by Peter Maydell 5 years, 10 months ago
On 22 June 2018 at 10:57, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 21 June 2018 at 15:57, Eric Blake <eblake@redhat.com> wrote:
>> ----------------------------------------------------------------
>> nbd patches for 2018-06-20
>>
>> Add experimental x-nbd-server-add-bitmap to expose a disabled
>> bitmap over NBD, in preparation for a pull model incremental
>> backup scheme. Also fix a corner case protocol issue with
>> NBD_CMD_BLOCK_STATUS, and add new NBD_CMD_CACHE.
>>
>> - Eric Blake: tests: Simplify .gitignore
>> - Eric Blake: nbd/server: Reject 0-length block status request
>> - Vladimir Sementsov-Ogievskiy: 0/6 NBD export bitmaps
>> - Vladimir Sementsov-Ogievskiy: nbd/server: introduce NBD_CMD_CACHE
>>
> Applied, thanks.

...patchew seems to be unhappy, though, eg
https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg06559.html
has an unrelated patchset failing with:

/tmp/qemu-test/src/nbd/server.c: In function 'nbd_trip':
/tmp/qemu-test/src/nbd/server.c:1980:19: error: 'end' may be used uninitialized
in this function [-Werror=maybe-uninitialized]
     *length = end - offset;
               ~~~~^~~~~~~~
/tmp/qemu-test/src/nbd/server.c:1940:30: note: 'end' was declared here
     uint64_t begin = offset, end;
                              ^~~
cc1: all warnings being treated as errors

thanks
-- PMM

Re: [Qemu-devel] [PULL v2 0/9] bitmap export over NBD
Posted by Eric Blake 5 years, 10 months ago
On 06/22/2018 06:43 AM, Peter Maydell wrote:
> On 22 June 2018 at 10:57, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 21 June 2018 at 15:57, Eric Blake <eblake@redhat.com> wrote:
>>> ----------------------------------------------------------------
>>> nbd patches for 2018-06-20
>>>
>>> Add experimental x-nbd-server-add-bitmap to expose a disabled
>>> bitmap over NBD, in preparation for a pull model incremental
>>> backup scheme. Also fix a corner case protocol issue with
>>> NBD_CMD_BLOCK_STATUS, and add new NBD_CMD_CACHE.
>>>
>>> - Eric Blake: tests: Simplify .gitignore
>>> - Eric Blake: nbd/server: Reject 0-length block status request
>>> - Vladimir Sementsov-Ogievskiy: 0/6 NBD export bitmaps
>>> - Vladimir Sementsov-Ogievskiy: nbd/server: introduce NBD_CMD_CACHE
>>>
>> Applied, thanks.
> 
> ...patchew seems to be unhappy, though, eg
> https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg06559.html
> has an unrelated patchset failing with:
> 
> /tmp/qemu-test/src/nbd/server.c: In function 'nbd_trip':
> /tmp/qemu-test/src/nbd/server.c:1980:19: error: 'end' may be used uninitialized
> in this function [-Werror=maybe-uninitialized]
>       *length = end - offset;
>                 ~~~~^~~~~~~~

Uurgh. It's a false positive (the compiler is complaining that the 
variable is uninitialized, which can only happen if the while loop is 
not executed; but the preconditions guarantee the loop executes at least 
once).  The assert() that I added was enough to silence gcc 7.3.1 on my 
Fedora 27 system, but docker-mingw@fedora is using 
gcc-8.1.1-1.fc28.x86_64. This should silence things (another way to 
silence would be rewriting while{} into do{}while).  I'll submit this as 
a formal patch if I can reproduce the problem/fix on docker.

diff --git i/nbd/server.c w/nbd/server.c
index 94137fbfe8f..2379f82d5d4 100644
--- i/nbd/server.c
+++ w/nbd/server.c
@@ -1968,7 +1968,7 @@ static unsigned int 
bitmap_to_extents(BdrvDirtyBitmap *bitmap, uint64_t offset,
                                        unsigned int nb_extents,
                                        bool dont_fragment)
  {
-    uint64_t begin = offset, end;
+    uint64_t begin = offset, end = offset;
      uint64_t overall_end = offset + *length;
      unsigned int i = 0;
      BdrvDirtyBitmapIter *it;
@@ -2008,6 +2008,7 @@ static unsigned int 
bitmap_to_extents(BdrvDirtyBitmap *bitmap, uint64_t offset,

      bdrv_dirty_bitmap_unlock(bitmap);

+    assert(offset > end);
      *length = end - offset;
      return i;
  }


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

Re: [Qemu-devel] [PULL v2 0/9] bitmap export over NBD
Posted by Peter Maydell 5 years, 10 months ago
On 22 June 2018 at 13:12, Eric Blake <eblake@redhat.com> wrote:
> Uurgh. It's a false positive (the compiler is complaining that the variable
> is uninitialized, which can only happen if the while loop is not executed;
> but the preconditions guarantee the loop executes at least once).  The
> assert() that I added was enough to silence gcc 7.3.1 on my Fedora 27
> system, but docker-mingw@fedora is using gcc-8.1.1-1.fc28.x86_64. This
> should silence things (another way to silence would be rewriting while{}
> into do{}while).  I'll submit this as a formal patch if I can reproduce the
> problem/fix on docker.

Huh. I had thought this was an old-gcc problem, not a new-gcc
one. Might be worth submitting to the gcc folks as a regression...

thanks
-- PMM

Re: [Qemu-devel] [PULL v2 0/9] bitmap export over NBD
Posted by Eric Blake 5 years, 10 months ago
On 06/22/2018 07:13 AM, Peter Maydell wrote:
> On 22 June 2018 at 13:12, Eric Blake <eblake@redhat.com> wrote:
>> Uurgh. It's a false positive (the compiler is complaining that the variable
>> is uninitialized, which can only happen if the while loop is not executed;
>> but the preconditions guarantee the loop executes at least once).  The
>> assert() that I added was enough to silence gcc 7.3.1 on my Fedora 27
>> system, but docker-mingw@fedora is using gcc-8.1.1-1.fc28.x86_64. This
>> should silence things (another way to silence would be rewriting while{}
>> into do{}while).  I'll submit this as a formal patch if I can reproduce the
>> problem/fix on docker.
> 
> Huh. I had thought this was an old-gcc problem, not a new-gcc
> one. Might be worth submitting to the gcc folks as a regression...

Hmm, I couldn't quickly reproduce it with a direct gcc on Fedora 28; and 
I don't know docker well enough to know if the cross-gcc used in 
docker-mingw@fedora is a different version than the native gcc-8.1.1 
listed in the installed packages list (the patchew output doesn't list 
the version of x86_64-w64-mingw32-gcc). Thus, I can't tell if it is a 
recent gcc regression or merely an old-gcc issue.  But I can confirm 
that 'make docker-test-mingw@fedora' reproduced the problem without my 
cleanup patch, and that my cleanup patch now in master made that happy 
again.

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