On 04/17/2015 12:01 PM, Max Reitz wrote:
On 27.03.2015 20:20, John Snow wrote:
In general, since transactions may reference QMP function helpers,
it would be nice for them to sit beneath them.
This will avoid the need for forward declaring any QMP interfaces,
which would be aggravating to update in so many places.
Signed-off-by: John Snow <js...@redhat.com>
---
blockdev.c | 2581
++++++++++++++++++++++++++++++------------------------------
1 file changed, 1292 insertions(+), 1289 deletions(-)
I'm not so sure about this patch. I mean... 2581 changes? :-/
If you (or someone else) can convince me that forward declarations are
really worse than this (is it more aggravating than having a patch with
this diffcount?), well...
But even then, I'd strongly vote for removing dropping all functional
changes beside the code movement from this patch. Because I'm seeing this:
2096c2096,2097
< error_set(errp, QERR_INVALID_PARAMETER_VALUE, "granularity",
"power of 2");
---
> error_set(errp, QERR_INVALID_PARAMETER_VALUE,
> "granularity", "power of 2");
2517c2518,2519
< /* New and old BlockDriverState structs for atomic group operations */
---
>
/******************************************************************************/
> /* Transactions */
3439a3442,3443
>
>
/******************************************************************************/
Probably sensible changes, but if you really, really, really want to go
for this code movement, please apply them in an own patch before this
one so that this one really is nothing but code movement.
Max
OK. I fixed the line length because checkpatch yelled at me.
I can add a micropatch that adds my organizational flair and fixes line
lengths in a teensy patch that precedes this one.
Now: is the code movement necessary? well... Maybe, maybe not. I'd
actually like to just break out transactions into their own file
entirely, but that has its own challenges (like our heavy usage of
internal block goodies) ...
My only real justification is in the cover letter: forward declarations
of QMP functions is a burden.
Then again, so is basically erasing transaction development history...
:(
