Re: [Qemu-devel] [PATCH v20 04/26] add some QemuOpts functions for replace work

[Chunyan Liu <cyliu@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 1913 bytes --]

2014-02-13 7:31 GMT+08:00 Eric Blake <eblake@redhat.com>:

> On 02/11/2014 11:33 PM, Chunyan Liu wrote:
> > Add some qemu_opt functions to replace the same functionality of
> > QEMUOptionParameter handling.
> >
> > Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
> > Signed-off-by: Chunyan Liu <cyliu@suse.com>
> > ---
> >  include/qemu/option.h |    9 +++
> >  util/qemu-option.c    |  134
> +++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 143 insertions(+), 0 deletions(-)
> >
>
> > +static void qemu_opt_del(QemuOpt *opt);
> > +
> > +const char *qemu_opt_get_del(QemuOpts *opts, const char *name)
> > +{
> > +    const char *str = qemu_opt_get(opts, name);
> > +    QemuOpt *opt = qemu_opt_find(opts, name);
> > +    if (opt) {
> > +        qemu_opt_del(opt);
>
> Is it any more efficient to reduce the number of times you crawl through
> the list?  For example, instead of crawling the list with qemu_opt_get,
> why not get teh string directly from the result of qemu_opt_find?  Also,
> you crawl again for qemu_opt_del.  Maybe it's unavoidable, and certainly
> still O(n), so not the end of the world, but worth thinking about.
>

In previous Dong Xu's patch, it's commented that there is too much code
duplication in qemu_opt_get and qemu_opt_get_del, could using qemu_opt_get
directly. Well, I'll improve that. Add "remove" flag to _qemu_opt_get to
handle
delete or not, and then wrapper _qemu_opt_get to generate qemu_opt_get and
qemu_opt_get_del.

> +void qemu_opts_print_help(QemuOptsList *list)
> > +{
> > +    int i;
> > +    printf("Supported options:\n");
>
> Why printf?  Might there be callers that want to print to somewhere
> other than stdout?
>

Just followed print_option_help. Existing print_option_help uses printf.
Could improve of course.

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

[-- Attachment #2: Type: text/html, Size: 3142 bytes --]