[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 06/13] QemuOpts: Drop qemu_opt_set(), rename qem
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 06/13] QemuOpts: Drop qemu_opt_set(), rename qemu_opt_set_err(), fix use |
Date: |
Tue, 17 Feb 2015 09:21:44 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Eric Blake <address@hidden> writes:
> 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.
Yeah, that's why I chose to do it in one go.
>> 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?
Yes, if I need to respin or send a pull request.
Thanks!
- [Qemu-devel] [PATCH 08/13] QemuOpts: Propagate errors through opts_parse(), (continued)
- [Qemu-devel] [PATCH 08/13] QemuOpts: Propagate errors through opts_parse(), Markus Armbruster, 2015/02/16
- [Qemu-devel] [PATCH 12/13] pc: Use qemu_opt_set() instead of qemu_opts_parse(), Markus Armbruster, 2015/02/16
- [Qemu-devel] [PATCH 05/13] block: Suppress unhelpful extra errors in bdrv_img_create(), Markus Armbruster, 2015/02/16
- [Qemu-devel] [PATCH 07/13] QemuOpts: Propagate errors through opts_do_parse(), Markus Armbruster, 2015/02/16
- [Qemu-devel] [PATCH 06/13] QemuOpts: Drop qemu_opt_set(), rename qemu_opt_set_err(), fix use, Markus Armbruster, 2015/02/16
- [Qemu-devel] [PATCH 03/13] QemuOpts: Convert qemu_opts_set() to Error, fix its use, Markus Armbruster, 2015/02/16
- Re: [Qemu-devel] [PATCH 00/13] QemuOpts: Convert various setters to Error, Markus Armbruster, 2015/02/17