Re: [Qemu-block] [PATCH v3 for-2.7 00/14] qapi: Allow blockdev-add for NBD

2016-06-14 Thread Eric Blake
On 05/03/2016 09:23 AM, Kevin Wolf wrote:
> Am 06.04.2016 um 20:28 hat Max Reitz geschrieben:
>> Turns out NBD is not so simple to do if you do it right. Anyway, this
>> series adds blockdev-add support for NBD clients.
> 
> What the series does seems to make sense to me, though things would be a
> bit nicer (especially on the command line) if SocketAddress had been a
> flat union from the beginning.  It's too late to change now, and I guess
> the ugliness isn't worth duplicating it.

I'm sorely tempted to make SocketAddress a flat union anyways (or even
an alternate that can be flat or nested at will), if only for
convenience.  But that's a pipe dream - there's probably not enough time
before 2.7 to actually achieve it.

> 
> I'll leave the in-detail review to our NBD folks.
> 
> Kevin
> 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-block] [PATCH v3 for-2.7 00/14] qapi: Allow blockdev-add for NBD

2016-05-03 Thread Kevin Wolf
Am 06.04.2016 um 20:28 hat Max Reitz geschrieben:
> Turns out NBD is not so simple to do if you do it right. Anyway, this
> series adds blockdev-add support for NBD clients.

What the series does seems to make sense to me, though things would be a
bit nicer (especially on the command line) if SocketAddress had been a
flat union from the beginning.  It's too late to change now, and I guess
the ugliness isn't worth duplicating it.

I'll leave the in-detail review to our NBD folks.

Kevin



Re: [Qemu-block] [PATCH v3 for-2.7 00/14] qapi: Allow blockdev-add for NBD

2016-05-03 Thread Max Reitz
On 06.04.2016 20:28, Max Reitz wrote:
> Turns out NBD is not so simple to do if you do it right. Anyway, this
> series adds blockdev-add support for NBD clients.
> 
> Patch 1 adds a QDict function which I needed in later NBD patches for
> handling legacy options (move "host" to "address.data.host" etc.).
> 
> Patches 2, 3, 4, and 5 are minor patches with no functional relation to
> this series, other than later patches will touch the code they touch,
> too.
> 
> Patches 6 and 7 prepare the code for the addition of a new option
> prefix, which is "address.".
> 
> Patch 8 makes the NBD client accept a SocketAddress under the "address"
> option (or rather, a flattened SocketAddress QDict with its keys
> prefixed by "address."). The old options "host", "port", and "path" are
> supported as legacy options and translated to the respective
> SocketAddress representation.
> 
> Patch 9 drops usage of "host", "port", and "path" outside of
> nbd_has_filename_options_conflict(),
> nbd_process_legacy_socket_options(), and nbd_refresh_filename(), making
> those options nothing but legacy.
> 
> Patch 10, the goal of this series, is again not very complicated.
> 
> Patches 11, 12, and 13 are required for the iotest added in patch 16. It
> will invoke qemu-nbd, so patch 13 is required. Besides qemu-nbd, it will
> launch an NBD server VM concurrently to the client VM, which is why
> patch 14 is required. And finally, it will test whether we can add an
> NBD BDS by passing it a file descriptor, which patch 15 is needed for
> (so we use the socket_scm_helper to pass sockets to qemu).
> 
> Patch 14 then adds the iotest for NBD's blockdev-add interface.
> 
> 
> *** This series requires Daniel's qdict_crumple() function (from his
> "Provide a QOM-based authorization API" series). ***
> 
> 
> v2:
> - Dropped patches 2 and 3; use Daniel's qdict_crumple() instead.
> - Patch 7: Not sure why the diff differs, seems functionally like the
>   same patch to me. Maybe git changed something about its default diff
>   algorithm.
> - Patch 8: Use qdict_crumple() instead of qdict_unflatten()
> - Patch 10: Rebase conflicts


Ping



signature.asc
Description: OpenPGP digital signature


[Qemu-block] [PATCH v3 for-2.7 00/14] qapi: Allow blockdev-add for NBD

2016-04-06 Thread Max Reitz
Turns out NBD is not so simple to do if you do it right. Anyway, this
series adds blockdev-add support for NBD clients.

Patch 1 adds a QDict function which I needed in later NBD patches for
handling legacy options (move "host" to "address.data.host" etc.).

Patches 2, 3, 4, and 5 are minor patches with no functional relation to
this series, other than later patches will touch the code they touch,
too.

Patches 6 and 7 prepare the code for the addition of a new option
prefix, which is "address.".

Patch 8 makes the NBD client accept a SocketAddress under the "address"
option (or rather, a flattened SocketAddress QDict with its keys
prefixed by "address."). The old options "host", "port", and "path" are
supported as legacy options and translated to the respective
SocketAddress representation.

Patch 9 drops usage of "host", "port", and "path" outside of
nbd_has_filename_options_conflict(),
nbd_process_legacy_socket_options(), and nbd_refresh_filename(), making
those options nothing but legacy.

Patch 10, the goal of this series, is again not very complicated.

Patches 11, 12, and 13 are required for the iotest added in patch 16. It
will invoke qemu-nbd, so patch 13 is required. Besides qemu-nbd, it will
launch an NBD server VM concurrently to the client VM, which is why
patch 14 is required. And finally, it will test whether we can add an
NBD BDS by passing it a file descriptor, which patch 15 is needed for
(so we use the socket_scm_helper to pass sockets to qemu).

Patch 14 then adds the iotest for NBD's blockdev-add interface.


*** This series requires Daniel's qdict_crumple() function (from his
"Provide a QOM-based authorization API" series). ***


v2:
- Dropped patches 2 and 3; use Daniel's qdict_crumple() instead.
- Patch 7: Not sure why the diff differs, seems functionally like the
  same patch to me. Maybe git changed something about its default diff
  algorithm.
- Patch 8: Use qdict_crumple() instead of qdict_unflatten()
- Patch 10: Rebase conflicts


git-backport-diff against v2:

Key:
[] : patches are identical
[] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/14:[] [--] 'qdict: Add qdict_change_key()'
002/14:[] [--] 'block/nbd: Drop trailing "." in error messages'
003/14:[] [--] 'block/nbd: Reject port parameter without host'
004/14:[] [--] 'block/nbd: Default port in nbd_refresh_filename()'
005/14:[] [--] 'block/nbd: Use qdict_put()'
006/14:[] [--] 'block/nbd: Add nbd_has_filename_options_conflict()'
007/14:[0008] [FC] 'block/nbd: "address" in nbd_refresh_filename()'
008/14:[0024] [FC] 'block/nbd: Accept SocketAddress'
009/14:[] [--] 'block/nbd: Use SocketAddress options'
010/14:[0004] [FC] 'qapi: Allow blockdev-add for NBD'
011/14:[] [-C] 'iotests.py: Add qemu_nbd function'
012/14:[] [--] 'iotests.py: Allow concurrent qemu instances'
013/14:[] [--] 'socket_scm_helper: Accept fd directly'
014/14:[] [-C] 'iotests: Add test for NBD's blockdev-add interface'


Max Reitz (14):
  qdict: Add qdict_change_key()
  block/nbd: Drop trailing "." in error messages
  block/nbd: Reject port parameter without host
  block/nbd: Default port in nbd_refresh_filename()
  block/nbd: Use qdict_put()
  block/nbd: Add nbd_has_filename_options_conflict()
  block/nbd: "address" in nbd_refresh_filename()
  block/nbd: Accept SocketAddress
  block/nbd: Use SocketAddress options
  qapi: Allow blockdev-add for NBD
  iotests.py: Add qemu_nbd function
  iotests.py: Allow concurrent qemu instances
  socket_scm_helper: Accept fd directly
  iotests: Add test for NBD's blockdev-add interface

 block/nbd.c| 235 ++---
 include/qapi/qmp/qdict.h   |   1 +
 qapi/block-core.json   |  23 +++-
 qobject/qdict.c|  23 
 tests/qemu-iotests/051.out |   4 +-
 tests/qemu-iotests/051.pc.out  |   4 +-
 tests/qemu-iotests/147 | 194 +++
 tests/qemu-iotests/147.out |   5 +
 tests/qemu-iotests/group   |   1 +
 tests/qemu-iotests/iotests.py  |  22 ++-
 tests/qemu-iotests/socket_scm_helper.c |  29 ++--
 11 files changed, 443 insertions(+), 98 deletions(-)
 create mode 100755 tests/qemu-iotests/147
 create mode 100644 tests/qemu-iotests/147.out

-- 
2.8.0