On 04/10/2014 11:53 AM, Chunyan Liu wrote: > Add def_value_str (default value) to QemuOptDesc, to replace function of the > default value in QEMUOptionParameter. > > Improve qemu_opts_get_* functions: if find opt, return opt->str; otherwise, > if desc->def_value_str is set, return desc->def_value_str; otherwise, return > input defval. > > Improve qemu_opts_print: if option is set, print opt->str; otherwise, if > desc->def_value_str is set, also print it. It will replace > print_option_parameters. To avoid print info changes, change qemu_opts_print > from fprintf stderr to printf, and remove last printf.
This still feels like a bunch to be squashing into one patch. Two possibilities that would have made it nicer to review (either one could be done in isolation, or even doing both if you have a reason to respin): 1. Clearly document in the commit message that there are NO current callers of qemu_opts_print - it is dead code in this patch but later in the series will make use of it, so we are free to change it however we'd like to make it useful when it is no longer dead code 2. This is a lot of change in one patch. Splitting into two parts (repurpose qemu_opts_print, but without default value handling; then add default handling along with the change toe qemu_opts_print to print a default) splits the functionality of the two patches But as this series is already gone through so many revisions, and was small enough for me to look at again... > > Reviewed-by: Leandro Dorileo <l...@dorileo.org> > Reviewed-by: Eric Blake <ebl...@redhat.com> ...I'm fine with keeping my review. > Signed-off-by: Dong Xu Wang <wdon...@linux.vnet.ibm.com> > Signed-off-by: Chunyan Liu <cy...@suse.com> > --- > changes: > * Following Leandro's comment: > merge v24 0002-QemuOpts-add-def_value_str-to-QemuOptDesc.patch and > v24 0011-qemu_opts_print-change-fprintf-stderr-to-printf.patch into one. Normally, when you make non-trivial changes based on a previous review, it is wise to drop the Reviewed-by for anyone that you want to re-review those changes. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
