[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v25 06/31] QemuOpts: add qemu_opt_get_*_del func
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v25 06/31] QemuOpts: add qemu_opt_get_*_del functions for replace work |
Date: |
Mon, 21 Apr 2014 13:11:08 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.4.0 |
On 04/10/2014 11:54 AM, Chunyan Liu wrote:
> Add qemu_opt_get_del, qemu_opt_get_bool_del, qemu_opt_get_number_del and
> qemu_opt_get_size_del to replace the same handling of QEMUOptionParamter
s/Paramter/Parameter/
> (get and delete).
>
> Several drivers are coded to parse a known subset of options, then
> remove them from the list before handing all remaining options to a
> second driver for further option processing. get_*_del makes it easier
> to retrieve a known option (or its default) and remove it from the list
> all in one action.
>
> Share common helper function:
>
> For qemu_opt_get_bool/size/number, they and their get_*_del counterpart
> could share most of the code except whether or not deleting the opt from
> option list, so generate common helper functions.
>
> For qemu_opt_get and qemu_opt_get_del, keep code duplication, since
> 1. qemu_opt_get_del returns malloc'd memory while qemu_opt_get returns
> in-place memory
> 2. qemu_opt_get_del returns (char *), qemu_opt_get returns (const char *),
> and could not change to (char *), since in one case, it will return
> desc->def_value_str, which is (const char *).
>
> Signed-off-by: Dong Xu Wang <address@hidden>
> Signed-off-by: Chunyan Liu <address@hidden>
> ---
> include/qemu/option.h | 6 +++
> util/qemu-option.c | 116
> ++++++++++++++++++++++++++++++++++++++++++++------
> 2 files changed, 109 insertions(+), 13 deletions(-)
>
> +++ b/util/qemu-option.c
> @@ -575,6 +575,19 @@ static void qemu_opt_del(QemuOpt *opt)
> g_free(opt);
> }
>
> +/* qemu_opt_set allow many settings for the same option.
s/allow/allows/
Question for a possible future patch: who relies on qemu_opt_set
supporting multiple settings for the same option? Would it make the
code any cleaner to rework qemu_opt_set to track only one value for an
option, with the caller having the option of either flagging duplicates
as an error or silently replacing the old option value with the new?
But that doesn't belong in this patch, so it doesn't impact my review.
> + * This function is to delete all settings for an option.
s/is to delete/deletes/
> +/* Get a known option (or its default) and remove it from the list
> + * all in one action. Return a malloced string of the option vaule.
s/vaule/value/
As my findings on this patch are limited to typo fixes, it is trivial
enough that you can fix them and add:
Reviewed-by: Eric Blake <address@hidden>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH v25 02/31] QemuOpts: add def_value_str to QemuOptDesc, (continued)
- [Qemu-devel] [PATCH v25 05/31] QemuOpts: move qemu_opt_del ahead for later calling, Chunyan Liu, 2014/04/11
- [Qemu-devel] [PATCH v25 04/31] QemuOpts: change opt->name|str from (const char *) to (char *), Chunyan Liu, 2014/04/11
- [Qemu-devel] [PATCH v25 01/31] QemuOpts: move find_desc_by_name ahead for later calling, Chunyan Liu, 2014/04/11
- [Qemu-devel] [PATCH v25 03/31] qapi: output def_value_str when query command line options, Chunyan Liu, 2014/04/11
- [Qemu-devel] [PATCH v25 06/31] QemuOpts: add qemu_opt_get_*_del functions for replace work, Chunyan Liu, 2014/04/11
- Re: [Qemu-devel] [PATCH v25 06/31] QemuOpts: add qemu_opt_get_*_del functions for replace work,
Eric Blake <=
- [Qemu-devel] [PATCH v25 07/31] QemuOpts: add qemu_opts_print_help to replace print_option_help, Chunyan Liu, 2014/04/11
- [Qemu-devel] [PATCH v25 09/31] QemuOpts: add qemu_opts_append to replace append_option_parameters, Chunyan Liu, 2014/04/11
- [Qemu-devel] [PATCH v25 08/31] QemuOpts: add conversion between QEMUOptionParameter to QemuOpts, Chunyan Liu, 2014/04/11
- [Qemu-devel] [PATCH v25 11/31] change block layer to support both QemuOpts and QEMUOptionParamter, Chunyan Liu, 2014/04/11