Re: [Qemu-block] [RFC PATCH 00/19] block: Configuration fixes and rbd authentication
On Thu, Jun 07, 2018 at 05:33:03PM -0400, Jeff Cody wrote:
> On Thu, Jun 07, 2018 at 08:25:40AM +0200, Markus Armbruster wrote:
> > This series is RFC because:
> >
> > * It clashes with parts of Max's "[PATCH 00/13] block: Try to create
> > well typed json:{} filenames". I missed that one until too late,
> > sorry.
> >
> > - I stole "[PATCH 06/13] block: Add block-specific QDict header",
> > and tacked on fixups I'd like to have.
> >
> > - My qobject_input_visitor_new_flat_confused() addresses the same
> > general problem as Max's qdict_stringify_for_keyval(): the block
> > layer's confused use of QObject types. My solution fixes a number
> > of bugs in -blockdev, blockdev-add and -drive. If Max's solution
> > adds further value, we need to merge the two somehow.
> >
> > - I changed qdict_flatten() to fix -blockdev and blockdev-add for
> > empty objects and arrays. Max fixed it to not mess up shallow
> > clones. We need to merge the two.
> >
> > * Rbd testing is incomplete. Jeff Cody volunteered to test on his
> > rig. Results should be in soon.
> >
>
> Here are some results from auth testing of various combinations; I haven't
> completed all the combinations in my matrix yet, but what I have completed
> looks like what I would expect.
>
> These were all tested with blockdev-add QAPI commands against this patch
> series.
>
> I'll be away on PTO tomorrow (Friday), so I'll conclude testing on Monday.
>
> Warning, long lines below, so don't read it on a vt220 (apologies in
> advance if you do...):
>
Below is the rest of the matrix filled out. Everything looks OK to me, the
ones that were a bit different than I expected were when the server was
'none', and we passed an bad key-secret. But that isn't a qemu/qapi issue,
and not really an issue at all (just different from what I expected).
Completed tests:
Server| Client-Side (qemu host)
+--
|
|
ServerAuth | userkey-secret/etc/ceph/keyring
auth-client-required | Result
+--
cephx, none | --- --- --- ---
{"return": {}}
cephx, none | --- --- valid ---
{"return": {}}
cephx, none | --- --- invalid ---
{"error": {"class": "GenericError", "desc": "error connecting: Invalid
argument"}}
cephx, none | --- valid --- ---
{"return": {}}
cephx, none | --- invalid --- ---
{"error": {"class": "GenericError", "desc": "error connecting: Invalid
argument"}}
cephx, none | --- invalid valid ---
{"error": {"class": "GenericError", "desc": "error connecting: Invalid
argument"}}
cephx, none | --- valid invalid ---
{"return": {}}
cephx, none | admin --- valid ---
{"return": {}}
cephx, none | admin --- invalid ---
{"error": {"class": "GenericError", "desc": "error connecting: Invalid
argument"}}
cephx, none | invalid --- valid ---
{"error": {"class": "GenericError", "desc": "error connecting: Invalid
argument"}}
cephx, none | invalid --- invalid ---
{"error": {"class": "GenericError", "desc": "error connecting: Invalid
argument"}}
cephx, none | --- --- --- none
{"return": {}}
cephx, none | --- --- valid none
{"return": {}}
cephx, none | --- --- invalid none
{"return": {}}
cephx, none | --- valid --- none
{"return": {}}
cephx, none | --- invalid --- none
{"return": {}}
cephx, none | --- invalid valid none
{"return": {}}
cephx, none | --- valid invalid none
{"return": {}}
cephx, none | admin --- valid none
{"return": {}}
cephx, none | admin --- invalid none
{"return": {}}
cephx, none | invalid --- valid none
{"return": {}}
cephx, none | invalid ---
Re: [Qemu-block] [RFC PATCH 00/19] block: Configuration fixes and rbd authentication
On 2018-06-11 16:52, Max Reitz wrote:
> On 2018-06-07 08:25, Markus Armbruster wrote:
>> This series is RFC because:
>>
>> * It clashes with parts of Max's "[PATCH 00/13] block: Try to create
>> well typed json:{} filenames". I missed that one until too late,
>> sorry.
>>
>> - I stole "[PATCH 06/13] block: Add block-specific QDict header",
>> and tacked on fixups I'd like to have.
>>
>> - My qobject_input_visitor_new_flat_confused() addresses the same
>> general problem as Max's qdict_stringify_for_keyval(): the block
>> layer's confused use of QObject types. My solution fixes a number
>> of bugs in -blockdev, blockdev-add and -drive. If Max's solution
>> adds further value, we need to merge the two somehow.
>
> So that looks trivial to change in my series (just merge the
> qdict_stringify_for_keyval() + qdict_crumple() into one
> qdict_crumple_for_keyval_qiv()).
Err, that's static, so actually "qdict_stringify_for_keyval() +
qdict_crumple() + qobject_input_visitor_new_keyval()" ->
"qobject_input_visitor_new_flat_confused()".
>
>> - I changed qdict_flatten() to fix -blockdev and blockdev-add for
>> empty objects and arrays. Max fixed it to not mess up shallow
>> clones. We need to merge the two.
>
> That seems possible. I suppose I'll just need to figure out the bugs
> after your series and fix them again. ;-)
I can now report that indeed all worked out fine, so I'll just base v2
of my series on yours.
Max
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-block] [RFC PATCH 00/19] block: Configuration fixes and rbd authentication
On 2018-06-07 08:25, Markus Armbruster wrote:
> This series is RFC because:
>
> * It clashes with parts of Max's "[PATCH 00/13] block: Try to create
> well typed json:{} filenames". I missed that one until too late,
> sorry.
>
> - I stole "[PATCH 06/13] block: Add block-specific QDict header",
> and tacked on fixups I'd like to have.
>
> - My qobject_input_visitor_new_flat_confused() addresses the same
> general problem as Max's qdict_stringify_for_keyval(): the block
> layer's confused use of QObject types. My solution fixes a number
> of bugs in -blockdev, blockdev-add and -drive. If Max's solution
> adds further value, we need to merge the two somehow.
So that looks trivial to change in my series (just merge the
qdict_stringify_for_keyval() + qdict_crumple() into one
qdict_crumple_for_keyval_qiv()).
> - I changed qdict_flatten() to fix -blockdev and blockdev-add for
> empty objects and arrays. Max fixed it to not mess up shallow
> clones. We need to merge the two.
That seems possible. I suppose I'll just need to figure out the bugs
after your series and fix them again. ;-)
Max
> * Rbd testing is incomplete. Jeff Cody volunteered to test on his
> rig. Results should be in soon.
>
> Perhaps the series should be split in two: PATCH 01-17 are
> configuration fixes, PATCH 18-19 are rbd authentication work. I may
> still do that for the non-RFC patch submission.
>
> Markus Armbruster (18):
> rbd: Drop deprecated -drive parameter "filename"
> iscsi: Drop deprecated -drive parameter "filename"
> fixup block: Add block-specific QDict header
> qobject: Move block-specific qdict code to block-qdict.c
> block: Fix -blockdev for certain non-string scalars
> block: Fix -drive for certain non-string scalars
> block: Clean up a misuse of qobject_to() in .bdrv_co_create_opts()
> block: Factor out qobject_input_visitor_new_flat_confused()
> block: Make remaining uses of qobject input visitor more robust
> block-qdict: Simplify qdict_flatten_qdict()
> block-qdict: Tweak qdict_flatten_qdict(), qdict_flatten_qlist()
> block-qdict: Clean up qdict_crumple() a bit
> block-qdict: Simplify qdict_is_list() some
> check-block-qdict: Rename qdict_flatten()'s variables for clarity
> check-block-qdict: Cover flattening of empty lists and dictionaries
> block: Fix -blockdev / blockdev-add for empty objects and arrays
> rbd: New parameter auth-client-required
> rbd: New parameter key-secret
>
> Max Reitz (1):
> block: Add block-specific QDict header
>
> MAINTAINERS | 2 +
> block.c | 1 +
> block/crypto.c| 6 +-
> block/gluster.c | 1 +
> block/iscsi.c | 24 +-
> block/nbd.c | 16 +-
> block/nfs.c | 8 +-
> block/parallels.c | 11 +-
> block/qcow.c | 11 +-
> block/qcow2.c | 11 +-
> block/qed.c | 11 +-
> block/quorum.c| 1 +
> block/rbd.c | 85 +++--
> block/sheepdog.c | 23 +-
> block/snapshot.c | 1 +
> block/ssh.c | 16 +-
> block/vdi.c | 4 +-
> block/vhdx.c | 11 +-
> block/vpc.c | 11 +-
> block/vvfat.c | 1 +
> block/vxhs.c | 1 +
> blockdev.c| 1 +
> include/block/qdict.h | 34 ++
> include/qapi/qmp/qdict.h | 17 -
> qapi/block-core.json | 15 +
> qobject/Makefile.objs | 1 +
> qobject/block-qdict.c | 722 ++
> qobject/qdict.c | 628 -
> tests/Makefile.include| 4 +
> tests/check-block-qdict.c | 690
> tests/check-qdict.c | 641 -
> tests/check-qobject.c | 1 +
> tests/test-replication.c | 1 +
> util/qemu-config.c| 1 +
> 34 files changed, 1573 insertions(+), 1439 deletions(-)
> create mode 100644 include/block/qdict.h
> create mode 100644 qobject/block-qdict.c
> create mode 100644 tests/check-block-qdict.c
>
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-block] [RFC PATCH 00/19] block: Configuration fixes and rbd authentication
On Thu, Jun 07, 2018 at 08:25:40AM +0200, Markus Armbruster wrote:
> This series is RFC because:
>
> * It clashes with parts of Max's "[PATCH 00/13] block: Try to create
> well typed json:{} filenames". I missed that one until too late,
> sorry.
>
> - I stole "[PATCH 06/13] block: Add block-specific QDict header",
> and tacked on fixups I'd like to have.
>
> - My qobject_input_visitor_new_flat_confused() addresses the same
> general problem as Max's qdict_stringify_for_keyval(): the block
> layer's confused use of QObject types. My solution fixes a number
> of bugs in -blockdev, blockdev-add and -drive. If Max's solution
> adds further value, we need to merge the two somehow.
>
> - I changed qdict_flatten() to fix -blockdev and blockdev-add for
> empty objects and arrays. Max fixed it to not mess up shallow
> clones. We need to merge the two.
>
> * Rbd testing is incomplete. Jeff Cody volunteered to test on his
> rig. Results should be in soon.
>
Here are some results from auth testing of various combinations; I haven't
completed all the combinations in my matrix yet, but what I have completed
looks like what I would expect.
These were all tested with blockdev-add QAPI commands against this patch
series.
I'll be away on PTO tomorrow (Friday), so I'll conclude testing on Monday.
Warning, long lines below, so don't read it on a vt220 (apologies in
advance if you do...):
Server| Client-Side (qemu host)
+--
|
|
ServerAuth | userkey-secret/etc/ceph/keyring
auth-client-required | Result
+--
cephx, none | --- --- --- ---
{"return": {}}
cephx, none | --- --- valid ---
{"return": {}}
cephx, none | --- --- invalid ---
{"error": {"class": "GenericError", "desc": "error connecting: Invalid
argument"}}
cephx, none | --- valid --- ---
{"return": {}}
cephx, none | --- invalid --- ---
{"error": {"class": "GenericError", "desc": "error connecting: Invalid
argument"}}
cephx, none | --- invalid valid ---
{"error": {"class": "GenericError", "desc": "error connecting: Invalid
argument"}}
cephx, none | --- valid invalid ---
{"return": {}}
cephx, none | admin --- valid ---
cephx, none | admin --- invalid ---
cephx, none | invalid --- valid ---
cephx, none | invalid --- invalid ---
cephx, none | --- --- --- none
{"return": {}}
cephx, none | --- --- valid none
{"return": {}}
cephx, none | --- --- invalid none
{"return": {}}
cephx, none | --- valid --- none
{"return": {}}
cephx, none | --- invalid --- none
{"return": {}}
cephx, none | --- invalid valid none
{"return": {}}
cephx, none | --- valid invalid none
{"return": {}}
cephx, none | admin --- valid none
{"return": {}}
cephx, none | admin --- invalid none
{"return": {}}
cephx, none | invalid --- valid none
{"return": {}}
cephx, none | invalid --- invalid none
{"return": {}}
|
cephx, none | --- --- --- cephx
{"error": {"class": "GenericError", "desc": "error connecting: No such
file or directory"}}
cephx, none | --- --- valid cephx
{"return": {}}
cephx, none | --- --- invalid cephx
{"error": {"class": "GenericError", "desc": "error connecting: Invalid
argument"}}
cephx, none | --- valid --- cephx
{"return": {}}
cephx, none | --- invalid ---
[Qemu-block] [RFC PATCH 00/19] block: Configuration fixes and rbd authentication
This series is RFC because:
* It clashes with parts of Max's "[PATCH 00/13] block: Try to create
well typed json:{} filenames". I missed that one until too late,
sorry.
- I stole "[PATCH 06/13] block: Add block-specific QDict header",
and tacked on fixups I'd like to have.
- My qobject_input_visitor_new_flat_confused() addresses the same
general problem as Max's qdict_stringify_for_keyval(): the block
layer's confused use of QObject types. My solution fixes a number
of bugs in -blockdev, blockdev-add and -drive. If Max's solution
adds further value, we need to merge the two somehow.
- I changed qdict_flatten() to fix -blockdev and blockdev-add for
empty objects and arrays. Max fixed it to not mess up shallow
clones. We need to merge the two.
* Rbd testing is incomplete. Jeff Cody volunteered to test on his
rig. Results should be in soon.
Perhaps the series should be split in two: PATCH 01-17 are
configuration fixes, PATCH 18-19 are rbd authentication work. I may
still do that for the non-RFC patch submission.
Markus Armbruster (18):
rbd: Drop deprecated -drive parameter "filename"
iscsi: Drop deprecated -drive parameter "filename"
fixup block: Add block-specific QDict header
qobject: Move block-specific qdict code to block-qdict.c
block: Fix -blockdev for certain non-string scalars
block: Fix -drive for certain non-string scalars
block: Clean up a misuse of qobject_to() in .bdrv_co_create_opts()
block: Factor out qobject_input_visitor_new_flat_confused()
block: Make remaining uses of qobject input visitor more robust
block-qdict: Simplify qdict_flatten_qdict()
block-qdict: Tweak qdict_flatten_qdict(), qdict_flatten_qlist()
block-qdict: Clean up qdict_crumple() a bit
block-qdict: Simplify qdict_is_list() some
check-block-qdict: Rename qdict_flatten()'s variables for clarity
check-block-qdict: Cover flattening of empty lists and dictionaries
block: Fix -blockdev / blockdev-add for empty objects and arrays
rbd: New parameter auth-client-required
rbd: New parameter key-secret
Max Reitz (1):
block: Add block-specific QDict header
MAINTAINERS | 2 +
block.c | 1 +
block/crypto.c| 6 +-
block/gluster.c | 1 +
block/iscsi.c | 24 +-
block/nbd.c | 16 +-
block/nfs.c | 8 +-
block/parallels.c | 11 +-
block/qcow.c | 11 +-
block/qcow2.c | 11 +-
block/qed.c | 11 +-
block/quorum.c| 1 +
block/rbd.c | 85 +++--
block/sheepdog.c | 23 +-
block/snapshot.c | 1 +
block/ssh.c | 16 +-
block/vdi.c | 4 +-
block/vhdx.c | 11 +-
block/vpc.c | 11 +-
block/vvfat.c | 1 +
block/vxhs.c | 1 +
blockdev.c| 1 +
include/block/qdict.h | 34 ++
include/qapi/qmp/qdict.h | 17 -
qapi/block-core.json | 15 +
qobject/Makefile.objs | 1 +
qobject/block-qdict.c | 722 ++
qobject/qdict.c | 628 -
tests/Makefile.include| 4 +
tests/check-block-qdict.c | 690
tests/check-qdict.c | 641 -
tests/check-qobject.c | 1 +
tests/test-replication.c | 1 +
util/qemu-config.c| 1 +
34 files changed, 1573 insertions(+), 1439 deletions(-)
create mode 100644 include/block/qdict.h
create mode 100644 qobject/block-qdict.c
create mode 100644 tests/check-block-qdict.c
--
2.17.1
