Re: [Qemu-devel] [PATCH 06/13] QemuOpts: Drop qemu_opt_set(), rename qemu_opt_set_err(), fix use

[Eric Blake]

On 02/16/2015 07:44 AM, Markus Armbruster wrote:
> qemu_opt_set() is a wrapper around qemu_opt_set() that reports the
> error with qerror_report_err().
> 
> Most of its users assume the function can't fail.  Make them use
> qemu_opt_set_err() with &error_abort, so that should the assumption
> ever break, it'll break noisily.
> 
> Just two users remain, in util/qemu-config.c.  Switch them to
> qemu_opt_set_err() as well, then rename qemu_opt_set_err() to
> qemu_opt_set().

Might be better to split this into fixing the two users in one patch,
then doing the rename in another (since the rename touches more than two
callers).  On the other hand, it would represent more churn with
changing existing qemu_opt_set() to qemu_opt_set_err(,&error_abort) just
to change back to qemu_opt_set later.

> 
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>  block.c                  |  5 ++--
>  block/qcow2.c            |  3 +-
>  block/vvfat.c            |  2 +-
>  blockdev.c               | 17 +++++------
>  hw/pci/pci-hotplug-old.c |  2 +-
>  hw/usb/dev-network.c     |  4 +--
>  hw/usb/dev-storage.c     |  6 ++--
>  hw/watchdog/watchdog.c   |  2 +-
>  include/qemu/option.h    |  5 ++--
>  net/net.c                |  2 +-
>  qdev-monitor.c           |  6 ++--
>  qemu-char.c              | 58 +++++++++++++++++++-------------------
>  qemu-img.c               |  6 ++--
>  tests/test-qemu-opts.c   |  6 ++--
>  util/qemu-config.c       |  9 ++++--
>  util/qemu-option.c       | 22 +++------------
>  util/qemu-sockets.c      | 34 +++++++++++-----------
>  vl.c                     | 73 
> ++++++++++++++++++++++++++----------------------
>  18 files changed, 131 insertions(+), 131 deletions(-)

Also, the patch is fairly obvious that it is mechanical, so even if you
don't split, I could live with:

Reviewed-by: Eric Blake <address@hidden>

> +++ b/util/qemu-config.c
> @@ -219,6 +219,7 @@ void qemu_add_opts(QemuOptsList *list)
>  
>  int qemu_set_option(const char *str)
>  {
> +    Error *local_err = NULL;
>      char group[64], id[64], arg[64];
>      QemuOptsList *list;
>      QemuOpts *opts;
> @@ -242,7 +243,9 @@ int qemu_set_option(const char *str)
>          return -1;
>      }
>  
> -    if (qemu_opt_set(opts, arg, str+offset+1) == -1) {
> +    qemu_opt_set(opts, arg, str+offset+1, &local_err);

Worth adding whitespace around the 2 '+' operators while touching this?

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

signature.asc
Description: OpenPGP digital signature