[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 00/16] qapi: Allow blockdev-add for NBD
From: |
Daniel P. Berrange |
Subject: |
Re: [Qemu-devel] [PATCH v2 00/16] qapi: Allow blockdev-add for NBD |
Date: |
Tue, 1 Mar 2016 10:00:35 +0000 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Tue, Mar 01, 2016 at 12:37:14AM +0100, Max Reitz wrote:
> On 01.03.2016 00:24, Eric Blake wrote:
> > On 02/29/2016 04:19 PM, 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.
> >>
> >> Patches 1 and 2 add one less and one more complicated QDict function,
> >> respectively, which I needed in later NBD patches: Patch 1 for handling
> >> legacy options (move "host" to "address.data.host" etc.) and patch 2
> >> because I'd like to use the input visitor for transforming the NBD
> >> options into a SocketAddress. Unfortunately, the block layer uses
> >> flattened QDicts everywhere, so we'll have to unflatten (erect?) them
> >> before we can use that visitor.
> >
> > Dan had a patch proposal that called the operation "crumple"; I need to
> > review both proposals and see which one I like.
> > https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg04618.html
>
> Well, here I go again, not looking at patches on the list...
>
> Looking at the design, I like his idea of having an escape sequence;
> also, his qdict_crumple() can return boths lists and dicts where my
> qdict_unflatten() only returns dicts (then again, this is what
> qdict_flatten() always works on). And his patch doesn't suffer from as
> much indentation as mine does.
The escape sequence is critical to support for my use case, because
sadly some object properties have '.' in their name :-(
> What I like more about my patch, however, is that I'm reusing
> qdict_array_split() and qdict_array_entries(). That is mostly because my
> function modifies the given QDict, where Dan's does not.
The reason for that is that the use context in which I need to call
qdict_crumple() has a const QDict, so modifying the original QDict
was not an option.
Second, for error handling, if there is a problem we can't resolve
half way through the unflattening process, then if you're modifying
the original QDict you end up with a QDict that is a hybrid between
the flat & unflat forms. I think it is pretty bad practice for API
design / behaviour to leave inputs in such a state on error. ie if
the code isn't capable of rolling back to the original state it
should not be modifying the input arg.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
- Re: [Qemu-devel] [PATCH v2 00/16] qapi: Allow blockdev-add for NBD,
Daniel P. Berrange <=