Re: [Qemu-devel] [PATCH V17 0/9] replace QEMUOptionParameter with QemuOpts parser

[Eric Blake]

On 07/17/2013 03:29 AM, Dong Xu Wang wrote:
> These patches will replace QEMUOptionParameter with QemuOpts. Change logs
> please go to each patch's commit message.
> 
> Dong Xu Wang (9):
>   qemu-option: add def_value_str in QemuOptDesc struct and rewrite
>     qemu_opts_print
>   qemu-option: avoid duplication of default value in QemuOpts
>   qemu-option: create four QemuOptsList related functions
>   qemu-option: create some QemuOpts functons
>   block: use QemuOpts support in block layer
>   qapi: query-command-line-options outputs def_value_str
>   qemu-option: remove QEMUOptionParameter related functions and struct
>   qemu-option: make qemu_opts_del accept opts being NULL
>   qemu-option: use qemu_opts_del without judging NULL
> 

> 
> -- 

Ouch - by putting your additional comments after an 'end-of-message'
marker, you made it harder for me to reply.  (Sane mailers intentionally
drop text after a "-- " line when replying).

> V17 add qemu_opts_del's support when opts is NULL, and made changes
> based on Eric's comments. I tried to split PATCH 5 into small pieces,
> but I found it is very hard, bdrv_* functions are all using
> QEMUOptionParameter or QemuOpts, if I split them, I have to support two
> parsers in block.c and qemu-img.c, it will make code very hard to read,

Yes, if you split the patches, then there will be a window of time in
the series where you have to support BOTH parsers at the same time.  But
since the existing parser is already present, what's so hard about that?
 By having two different callbacks, one with the old signature and one
with the new, with exactly one of the callbacks being non-NULL, it
should be relatively easy to decide which of the two callbacks to use.

> so I leave them in one big patch, I am sorry it is realy hard to review.

I still think that the ease of review that would be added by splitting
the patch is worth making the effort.  Yes, it may be harder for you,
but if the end result is that more people are willing to review the
patch, then the community as a whole spends less collective time, and
the series will be accepted sooner.

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

signature.asc
Description: OpenPGP digital signature