[Qemu-devel] [PATCH 00/18] nbd: BLOCK_STATUS

Vladimir Sementsov-Ogievskiy posted 18 patches 7 years, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170203154757.36140-1-vsementsov@virtuozzo.com
Test checkpatch passed
Test docker passed
Test s390x failed
block/dirty-bitmap.c                     |  70 ++++
block/nbd-client.c                       | 230 ++++++++++-
block/nbd-client.h                       |  13 +
block/nbd.c                              |  44 +-
blockdev.c                               |  57 +++
include/block/block_int.h                |   4 +
include/block/dirty-bitmap.h             |  10 +
include/block/nbd.h                      |  73 +++-
include/qemu/hbitmap.h                   |  16 +
nbd/client.c                             | 373 ++++++++++++++---
nbd/nbd-internal.h                       |  25 +-
nbd/server.c                             | 661 ++++++++++++++++++++++++++++++-
qapi/block-core.json                     |  46 ++-
qemu-nbd.c                               |   2 +-
tests/Makefile.include                   |   2 +-
tests/qemu-iotests/180                   | 133 +++++++
tests/qemu-iotests/180.out               |   5 +
tests/qemu-iotests/group                 |   1 +
tests/qemu-iotests/nbd-fault-injector.py |   4 +-
util/hbitmap.c                           |  37 ++
20 files changed, 1713 insertions(+), 93 deletions(-)
create mode 100755 tests/qemu-iotests/180
create mode 100644 tests/qemu-iotests/180.out
[Qemu-devel] [PATCH 00/18] nbd: BLOCK_STATUS
Posted by Vladimir Sementsov-Ogievskiy 7 years, 2 months ago
Hi all!

We really need exporting dirty bitmaps feature as well as remote
get_block_status for nbd devices. So, here is minimalistic and restricted
realization of 'structured reply' and 'block status' nbd protocol extension
(as second is developed over the first the combined spec may be found here:
https://github.com/NetworkBlockDevice/nbd/blob/extension-blockstatus/doc/proto.md)

What is done:

server:
 - can send not fragmented structured replies for CMD_READ (this was done only
   because the spec doesn't allow structure reply feature without maintaining
   structured read)
 - can export dirty bitmap through BLOCK_STATUS. Only one bitmap can be exported,
   negotiation query should be 'qemu-dirty-bitmap:<bitmap name>'
 - cab export block status through BLOCK_STATUS. Client can negotiate only one
   entity for exporting through BLOCK_STATUS - bitmap _or_ block status.
   negotiation query should be 'base:allocation', as defined in the spec.
   server sends only one extent on each BLOCK_STATUS query.

client:
 - can receive not fragmented structured replies for CMD_READ
 - can get load dirty bitmap through nbd. Be careful: bitmap for export is
   is selected during nbd negotiation - actually in open(). So, name argument for
   qmp block-dirty-bitmap-load is just a _new_ name for loaded bitmap.
   (any way, for us block-dirty-bitmap-load is just a way to test the feature,
    really we need only server part)
 - get_block_status works now through nbd CMD_BLOCK_STATUS, if base:allocation is
   negotiated for export.

It should be minimal but fully compatible realization of the spec.

web: https://src.openvz.org/users/vsementsov/repos/qemu/browse?at=nbd-block-status-v1
git: https://src.openvz.org/scm/~vsementsov/qemu.git (tag nbd-block-status-v1)

Vladimir Sementsov-Ogievskiy (18):
  nbd: rename NBD_REPLY_MAGIC to NBD_SIMPLE_REPLY_MAGIC
  nbd-server: refactor simple reply sending
  nbd: Minimal structured read for server
  nbd/client: refactor nbd_receive_starttls
  nbd/client: fix drop_sync
  nbd/client: refactor drop_sync
  nbd: Minimal structured read for client
  hbitmap: add next_zero function
  block/dirty-bitmap: add bdrv_dirty_bitmap_next()
  block/dirty-bitmap: add bdrv_load_dirty_bitmap
  nbd: BLOCK_STATUS for bitmap export: server part
  nbd: BLOCK_STATUS for bitmap export: client part
  nbd: add nbd_dirty_bitmap_load
  qmp: add x-debug-block-dirty-bitmap-sha256
  qmp: add block-dirty-bitmap-load
  iotests: add test for nbd dirty bitmap export
  nbd: BLOCK_STATUS for standard get_block_status function: server part
  nbd: BLOCK_STATUS for standard get_block_status function: client part

 block/dirty-bitmap.c                     |  70 ++++
 block/nbd-client.c                       | 230 ++++++++++-
 block/nbd-client.h                       |  13 +
 block/nbd.c                              |  44 +-
 blockdev.c                               |  57 +++
 include/block/block_int.h                |   4 +
 include/block/dirty-bitmap.h             |  10 +
 include/block/nbd.h                      |  73 +++-
 include/qemu/hbitmap.h                   |  16 +
 nbd/client.c                             | 373 ++++++++++++++---
 nbd/nbd-internal.h                       |  25 +-
 nbd/server.c                             | 661 ++++++++++++++++++++++++++++++-
 qapi/block-core.json                     |  46 ++-
 qemu-nbd.c                               |   2 +-
 tests/Makefile.include                   |   2 +-
 tests/qemu-iotests/180                   | 133 +++++++
 tests/qemu-iotests/180.out               |   5 +
 tests/qemu-iotests/group                 |   1 +
 tests/qemu-iotests/nbd-fault-injector.py |   4 +-
 util/hbitmap.c                           |  37 ++
 20 files changed, 1713 insertions(+), 93 deletions(-)
 create mode 100755 tests/qemu-iotests/180
 create mode 100644 tests/qemu-iotests/180.out

-- 
2.11.0


Re: [Qemu-devel] [PATCH 00/18] nbd: BLOCK_STATUS
Posted by Paolo Bonzini 7 years, 2 months ago

On 03/02/2017 16:47, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> We really need exporting dirty bitmaps feature as well as remote
> get_block_status for nbd devices. So, here is minimalistic and restricted
> realization of 'structured reply' and 'block status' nbd protocol extension
> (as second is developed over the first the combined spec may be found here:
> https://github.com/NetworkBlockDevice/nbd/blob/extension-blockstatus/doc/proto.md)
> 
> What is done:
> 
> server:
>  - can send not fragmented structured replies for CMD_READ (this was done only
>    because the spec doesn't allow structure reply feature without maintaining
>    structured read)
>  - can export dirty bitmap through BLOCK_STATUS. Only one bitmap can be exported,
>    negotiation query should be 'qemu-dirty-bitmap:<bitmap name>'
>  - cab export block status through BLOCK_STATUS. Client can negotiate only one
>    entity for exporting through BLOCK_STATUS - bitmap _or_ block status.
>    negotiation query should be 'base:allocation', as defined in the spec.
>    server sends only one extent on each BLOCK_STATUS query.
> 
> client:
>  - can receive not fragmented structured replies for CMD_READ
>  - can get load dirty bitmap through nbd. Be careful: bitmap for export is
>    is selected during nbd negotiation - actually in open(). So, name argument for
>    qmp block-dirty-bitmap-load is just a _new_ name for loaded bitmap.
>    (any way, for us block-dirty-bitmap-load is just a way to test the feature,
>     really we need only server part)
>  - get_block_status works now through nbd CMD_BLOCK_STATUS, if base:allocation is
>    negotiated for export.
> 
> It should be minimal but fully compatible realization of the spec.

This will require v2, but it seems pretty close already.

Paolo

> web: https://src.openvz.org/users/vsementsov/repos/qemu/browse?at=nbd-block-status-v1
> git: https://src.openvz.org/scm/~vsementsov/qemu.git (tag nbd-block-status-v1)
> 
> Vladimir Sementsov-Ogievskiy (18):
>   nbd: rename NBD_REPLY_MAGIC to NBD_SIMPLE_REPLY_MAGIC
>   nbd-server: refactor simple reply sending
>   nbd: Minimal structured read for server
>   nbd/client: refactor nbd_receive_starttls
>   nbd/client: fix drop_sync
>   nbd/client: refactor drop_sync
>   nbd: Minimal structured read for client
>   hbitmap: add next_zero function
>   block/dirty-bitmap: add bdrv_dirty_bitmap_next()
>   block/dirty-bitmap: add bdrv_load_dirty_bitmap
>   nbd: BLOCK_STATUS for bitmap export: server part
>   nbd: BLOCK_STATUS for bitmap export: client part
>   nbd: add nbd_dirty_bitmap_load
>   qmp: add x-debug-block-dirty-bitmap-sha256
>   qmp: add block-dirty-bitmap-load
>   iotests: add test for nbd dirty bitmap export
>   nbd: BLOCK_STATUS for standard get_block_status function: server part
>   nbd: BLOCK_STATUS for standard get_block_status function: client part
> 
>  block/dirty-bitmap.c                     |  70 ++++
>  block/nbd-client.c                       | 230 ++++++++++-
>  block/nbd-client.h                       |  13 +
>  block/nbd.c                              |  44 +-
>  blockdev.c                               |  57 +++
>  include/block/block_int.h                |   4 +
>  include/block/dirty-bitmap.h             |  10 +
>  include/block/nbd.h                      |  73 +++-
>  include/qemu/hbitmap.h                   |  16 +
>  nbd/client.c                             | 373 ++++++++++++++---
>  nbd/nbd-internal.h                       |  25 +-
>  nbd/server.c                             | 661 ++++++++++++++++++++++++++++++-
>  qapi/block-core.json                     |  46 ++-
>  qemu-nbd.c                               |   2 +-
>  tests/Makefile.include                   |   2 +-
>  tests/qemu-iotests/180                   | 133 +++++++
>  tests/qemu-iotests/180.out               |   5 +
>  tests/qemu-iotests/group                 |   1 +
>  tests/qemu-iotests/nbd-fault-injector.py |   4 +-
>  util/hbitmap.c                           |  37 ++
>  20 files changed, 1713 insertions(+), 93 deletions(-)
>  create mode 100755 tests/qemu-iotests/180
>  create mode 100644 tests/qemu-iotests/180.out
> 

Re: [Qemu-devel] [PATCH 00/18] nbd: BLOCK_STATUS
Posted by Eric Blake 6 years, 9 months ago
On 02/15/2017 11:05 AM, Paolo Bonzini wrote:

[now several months later...]

> 
> 
> On 03/02/2017 16:47, Vladimir Sementsov-Ogievskiy wrote:
>> Hi all!
>>
>> We really need exporting dirty bitmaps feature as well as remote
>> get_block_status for nbd devices. So, here is minimalistic and restricted
>> realization of 'structured reply' and 'block status' nbd protocol extension
>> (as second is developed over the first the combined spec may be found here:
>> https://github.com/NetworkBlockDevice/nbd/blob/extension-blockstatus/doc/proto.md)
>>

>>
>> It should be minimal but fully compatible realization of the spec.
> 
> This will require v2, but it seems pretty close already.

This series now needs a major rebase on top of the cleanups already
present for qemu 2.10.  Realistically, I'm aiming to get my series [1]
for NBD_OPT_GO into 2.10 (we have less than a week before soft freeze,
so any reviews offered on that series will be appreciated), then the
work on adding structured read and BLOCK_STATUS will probably be big
enough that it will have to be 2.11 material, but getting it posted
sooner rather than later will make it easier to fine-tune all the details.

[1] https://lists.gnu.org/archive/html/qemu-devel/2017-07/msg01971.html

And remember that in open source projects, review can often be the
bottleneck - it does not scale to have reviews done solely by the people
listed in MAINTAINERS.  My personal rule of thumb is that I try to
review 2 patches for every 1 that I submit, to make sure I'm not
contributing to a black hole of unreviewed patch backlog (although no
one will ever turn down a patch from a contributor unable to follow that
advice).  Other benefits of offering reviews: you quickly become more
familiar with the code base, and you earn some name-recognition in the
community (where it is more likely that your own patches get reviewed
quickly because someone recognizes that you are a regular contributor).
It's okay to state on a review that you are not familiar/comfortable
with the code, and want a second reviewer to double-check your work.

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