* [Qemu-devel] [PULL 00/13] QemuOpts: Convert various setters to Error
@ 2015-02-26 14:07 Markus Armbruster
2015-02-26 14:07 ` [Qemu-devel] [PULL 01/13] QemuOpts: Convert qemu_opt_set_bool() to Error, fix its use Markus Armbruster
` (13 more replies)
0 siblings, 14 replies; 15+ messages in thread
From: Markus Armbruster @ 2015-02-26 14:07 UTC (permalink / raw)
To: qemu-devel
Bonus: improve qemu-img error reporting slightly.
Changes since v1:
* Straightforward rebase
* Whitespace fix in PATCH 6 [Eric]
* Comment polished in PATCH 9 [Eric]
Because they're trivial, I'm requesting a pull without another round
of review.
The following changes since commit c5c6d7f81a6950d8e32a3b5a0bafd37bfa5a8e88:
Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2015-02-18' into staging (2015-02-26 07:01:08 +0000)
are available in the git repository at:
git://repo.or.cz/qemu/armbru.git tags/pull-error-2015-02-26
for you to fetch changes up to b3adf5acb57dee14a74e57ab4f16cd1a83e5a7d2:
qtest: Use qemu_opt_set() instead of qemu_opts_parse() (2015-02-26 14:52:13 +0100)
----------------------------------------------------------------
QemuOpts: Convert various setters to Error
----------------------------------------------------------------
Markus Armbruster (13):
QemuOpts: Convert qemu_opt_set_bool() to Error, fix its use
QemuOpts: Convert qemu_opt_set_number() to Error, fix its use
QemuOpts: Convert qemu_opts_set() to Error, fix its use
qemu-img: Suppress unhelpful extra errors in convert, resize
block: Suppress unhelpful extra errors in bdrv_img_create()
QemuOpts: Drop qemu_opt_set(), rename qemu_opt_set_err(), fix use
QemuOpts: Propagate errors through opts_do_parse()
QemuOpts: Propagate errors through opts_parse()
qemu-img: Suppress unhelpful extra errors in convert, amend
block: Simplify setting numeric options
qemu-sockets: Simplify setting numeric and boolean options
pc: Use qemu_opt_set() instead of qemu_opts_parse()
qtest: Use qemu_opt_set() instead of qemu_opts_parse()
block.c | 17 +++++---
block/nbd.c | 3 +-
block/qcow2.c | 5 ++-
block/vvfat.c | 5 ++-
blockdev.c | 25 ++++++------
hw/i386/pc.c | 11 ++---
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 | 18 +++++----
net/net.c | 6 +--
qdev-monitor.c | 6 +--
qemu-char.c | 68 +++++++++++++++++--------------
qemu-img.c | 44 ++++++++++++++------
qtest.c | 11 ++---
tests/test-qemu-opts.c | 38 +++++++++---------
util/qemu-config.c | 9 ++++-
util/qemu-option.c | 100 +++++++++++++++++++++++-----------------------
util/qemu-sockets.c | 42 +++++++++----------
vl.c | 102 ++++++++++++++++++++++++++---------------------
21 files changed, 284 insertions(+), 240 deletions(-)
--
1.9.3
^ permalink raw reply [flat|nested] 15+ messages in thread* [Qemu-devel] [PULL 01/13] QemuOpts: Convert qemu_opt_set_bool() to Error, fix its use 2015-02-26 14:07 [Qemu-devel] [PULL 00/13] QemuOpts: Convert various setters to Error Markus Armbruster @ 2015-02-26 14:07 ` Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 02/13] QemuOpts: Convert qemu_opt_set_number() " Markus Armbruster ` (12 subsequent siblings) 13 siblings, 0 replies; 15+ messages in thread From: Markus Armbruster @ 2015-02-26 14:07 UTC (permalink / raw) To: qemu-devel Return the Error object instead of reporting it with qerror_report_err(). Change callers that assume the function can't fail to pass &error_abort, so that should the assumption ever break, it'll break noisily. Turns out all callers outside its unit test assume that. We could drop the Error ** argument, but that would make the interface less regular, so don't. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- blockdev.c | 6 +++--- include/qemu/option.h | 3 ++- tests/test-qemu-opts.c | 10 +++++----- util/qemu-option.c | 9 ++++----- util/qemu-sockets.c | 4 ++-- vl.c | 5 +++-- 6 files changed, 19 insertions(+), 18 deletions(-) diff --git a/blockdev.c b/blockdev.c index ae73539..79c33ab 100644 --- a/blockdev.c +++ b/blockdev.c @@ -737,15 +737,15 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) /* Specific options take precedence */ if (!qemu_opt_get(all_opts, "cache.writeback")) { qemu_opt_set_bool(all_opts, "cache.writeback", - !!(flags & BDRV_O_CACHE_WB)); + !!(flags & BDRV_O_CACHE_WB), &error_abort); } if (!qemu_opt_get(all_opts, "cache.direct")) { qemu_opt_set_bool(all_opts, "cache.direct", - !!(flags & BDRV_O_NOCACHE)); + !!(flags & BDRV_O_NOCACHE), &error_abort); } if (!qemu_opt_get(all_opts, "cache.no-flush")) { qemu_opt_set_bool(all_opts, "cache.no-flush", - !!(flags & BDRV_O_NO_FLUSH)); + !!(flags & BDRV_O_NO_FLUSH), &error_abort); } qemu_opt_unset(all_opts, "cache"); } diff --git a/include/qemu/option.h b/include/qemu/option.h index 58c0157..3206874 100644 --- a/include/qemu/option.h +++ b/include/qemu/option.h @@ -97,7 +97,8 @@ int qemu_opt_unset(QemuOpts *opts, const char *name); int qemu_opt_set(QemuOpts *opts, const char *name, const char *value); void qemu_opt_set_err(QemuOpts *opts, const char *name, const char *value, Error **errp); -int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val); +void qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val, + Error **errp); int qemu_opt_set_number(QemuOpts *opts, const char *name, int64_t val); typedef int (*qemu_opt_loopfunc)(const char *name, const char *value, void *opaque); int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque, diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c index ca08ac5..6b0ae33 100644 --- a/tests/test-qemu-opts.c +++ b/tests/test-qemu-opts.c @@ -169,10 +169,10 @@ static void test_qemu_opt_get(void) static void test_qemu_opt_get_bool(void) { + Error *err = NULL; QemuOptsList *list; QemuOpts *opts; bool opt; - int ret; list = qemu_find_opts("opts_list_02"); g_assert(list != NULL); @@ -192,16 +192,16 @@ static void test_qemu_opt_get_bool(void) opt = qemu_opt_get_bool(opts, "bool1", false); g_assert(opt == false); - ret = qemu_opt_set_bool(opts, "bool1", true); - g_assert(ret == 0); + qemu_opt_set_bool(opts, "bool1", true, &err); + g_assert(!err); /* now we have set bool1, should know about it */ opt = qemu_opt_get_bool(opts, "bool1", false); g_assert(opt == true); /* having reset the value, opt should be the reset one not defval */ - ret = qemu_opt_set_bool(opts, "bool1", false); - g_assert(ret == 0); + qemu_opt_set_bool(opts, "bool1", false, &err); + g_assert(!err); opt = qemu_opt_get_bool(opts, "bool1", true); g_assert(opt == false); diff --git a/util/qemu-option.c b/util/qemu-option.c index d3ab65d..4de8326 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -568,7 +568,8 @@ void qemu_opt_set_err(QemuOpts *opts, const char *name, const char *value, opt_set(opts, name, value, false, errp); } -int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val) +void qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val, + Error **errp) { QemuOpt *opt; const QemuOptDesc *desc = opts->list->desc; @@ -576,9 +577,9 @@ int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val) opt = g_malloc0(sizeof(*opt)); opt->desc = find_desc_by_name(desc, name); if (!opt->desc && !opts_accepts_any(opts)) { - qerror_report(QERR_INVALID_PARAMETER, name); + error_set(errp, QERR_INVALID_PARAMETER, name); g_free(opt); - return -1; + return; } opt->name = g_strdup(name); @@ -586,8 +587,6 @@ int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val) opt->value.boolean = !!val; opt->str = g_strdup(val ? "on" : "off"); QTAILQ_INSERT_TAIL(&opts->head, opt, next); - - return 0; } int qemu_opt_set_number(QemuOpts *opts, const char *name, int64_t val) diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c index 61fc3c1..7205dad 100644 --- a/util/qemu-sockets.c +++ b/util/qemu-sockets.c @@ -580,8 +580,8 @@ static void inet_addr_to_opts(QemuOpts *opts, const InetSocketAddress *addr) bool ipv6 = addr->ipv6 || !addr->has_ipv6; if (!ipv4 || !ipv6) { - qemu_opt_set_bool(opts, "ipv4", ipv4); - qemu_opt_set_bool(opts, "ipv6", ipv6); + qemu_opt_set_bool(opts, "ipv4", ipv4, &error_abort); + qemu_opt_set_bool(opts, "ipv6", ipv6, &error_abort); } if (addr->has_to) { char to[20]; diff --git a/vl.c b/vl.c index e1ffd0a..27e738a 100644 --- a/vl.c +++ b/vl.c @@ -2222,7 +2222,7 @@ static void monitor_parse(const char *optarg, const char *mode, bool pretty) } qemu_opt_set(opts, "mode", mode); qemu_opt_set(opts, "chardev", label); - qemu_opt_set_bool(opts, "pretty", pretty); + qemu_opt_set_bool(opts, "pretty", pretty, &error_abort); if (def) qemu_opt_set(opts, "default", "on"); monitor_device_index++; @@ -3287,7 +3287,8 @@ int main(int argc, char **argv, char **envp) } qemu_opt_set_bool(fsdev, "readonly", - qemu_opt_get_bool(opts, "readonly", 0)); + qemu_opt_get_bool(opts, "readonly", 0), + &error_abort); device = qemu_opts_create(qemu_find_opts("device"), NULL, 0, &error_abort); qemu_opt_set(device, "driver", "virtio-9p-pci"); -- 1.9.3 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PULL 02/13] QemuOpts: Convert qemu_opt_set_number() to Error, fix its use 2015-02-26 14:07 [Qemu-devel] [PULL 00/13] QemuOpts: Convert various setters to Error Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 01/13] QemuOpts: Convert qemu_opt_set_bool() to Error, fix its use Markus Armbruster @ 2015-02-26 14:07 ` Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 03/13] QemuOpts: Convert qemu_opts_set() " Markus Armbruster ` (11 subsequent siblings) 13 siblings, 0 replies; 15+ messages in thread From: Markus Armbruster @ 2015-02-26 14:07 UTC (permalink / raw) To: qemu-devel Return the Error object instead of reporting it with qerror_report_err(). Change callers that assume the function can't fail to pass &error_abort, so that should the assumption ever break, it'll break noisily. Turns out all callers outside its unit test assume that. We could drop the Error ** argument, but that would make the interface less regular, so don't. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- block.c | 6 +++--- block/nbd.c | 3 ++- block/qcow2.c | 2 +- block/vvfat.c | 3 ++- include/qemu/option.h | 3 ++- qemu-img.c | 3 ++- tests/test-qemu-opts.c | 16 ++++++++-------- util/qemu-option.c | 9 ++++----- vl.c | 2 +- 9 files changed, 25 insertions(+), 22 deletions(-) diff --git a/block.c b/block.c index 9b707e3..f59c4cf 100644 --- a/block.c +++ b/block.c @@ -1364,7 +1364,7 @@ int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp) opts = qemu_opts_create(bdrv_qcow2.create_opts, NULL, 0, &error_abort); - qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_size); + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_size, &error_abort); ret = bdrv_create(&bdrv_qcow2, tmp_filename, opts, &local_err); qemu_opts_del(opts); if (ret < 0) { @@ -5649,7 +5649,7 @@ void bdrv_img_create(const char *filename, const char *fmt, /* Create parameter list with default values */ opts = qemu_opts_create(create_opts, NULL, 0, &error_abort); - qemu_opt_set_number(opts, BLOCK_OPT_SIZE, img_size); + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, img_size, &error_abort); /* Parse -o options */ if (options) { @@ -5731,7 +5731,7 @@ void bdrv_img_create(const char *filename, const char *fmt, goto out; } - qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size); + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size, &error_abort); bdrv_unref(bs); } else { diff --git a/block/nbd.c b/block/nbd.c index 2f3b9ad..697c021 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -215,7 +215,8 @@ static void nbd_config(BDRVNBDState *s, QDict *options, char **export, } if (!qemu_opt_get(s->socket_opts, "port")) { - qemu_opt_set_number(s->socket_opts, "port", NBD_DEFAULT_PORT); + qemu_opt_set_number(s->socket_opts, "port", NBD_DEFAULT_PORT, + &error_abort); } *export = g_strdup(qdict_get_try_str(options, "export")); diff --git a/block/qcow2.c b/block/qcow2.c index 2ed8d95..7720874 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1858,7 +1858,7 @@ static int qcow2_create2(const char *filename, int64_t total_size, meta_size += nreftablee * sizeof(uint64_t); qemu_opt_set_number(opts, BLOCK_OPT_SIZE, - aligned_total_size + meta_size); + aligned_total_size + meta_size, &error_abort); qemu_opt_set(opts, BLOCK_OPT_PREALLOC, PreallocMode_lookup[prealloc]); } diff --git a/block/vvfat.c b/block/vvfat.c index a1a44f0..5e32d77 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -2924,7 +2924,8 @@ static int enable_write_target(BDRVVVFATState *s, Error **errp) } opts = qemu_opts_create(bdrv_qcow->create_opts, NULL, 0, &error_abort); - qemu_opt_set_number(opts, BLOCK_OPT_SIZE, s->sector_count * 512); + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, s->sector_count * 512, + &error_abort); qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, "fat:"); ret = bdrv_create(bdrv_qcow, s->qcow_filename, opts, errp); diff --git a/include/qemu/option.h b/include/qemu/option.h index 3206874..7422cc2 100644 --- a/include/qemu/option.h +++ b/include/qemu/option.h @@ -99,7 +99,8 @@ void qemu_opt_set_err(QemuOpts *opts, const char *name, const char *value, Error **errp); void qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val, Error **errp); -int qemu_opt_set_number(QemuOpts *opts, const char *name, int64_t val); +void qemu_opt_set_number(QemuOpts *opts, const char *name, int64_t val, + Error **errp); typedef int (*qemu_opt_loopfunc)(const char *name, const char *value, void *opaque); int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque, int abort_on_failure); diff --git a/qemu-img.c b/qemu-img.c index fcdfb67..752eecb 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1550,7 +1550,8 @@ static int img_convert(int argc, char **argv) goto out; } - qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_sectors * 512); + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_sectors * 512, + &error_abort); ret = add_old_style_options(out_fmt, opts, out_baseimg, NULL); if (ret < 0) { goto out; diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c index 6b0ae33..3fea96a 100644 --- a/tests/test-qemu-opts.c +++ b/tests/test-qemu-opts.c @@ -215,10 +215,10 @@ static void test_qemu_opt_get_bool(void) static void test_qemu_opt_get_number(void) { + Error *err = NULL; QemuOptsList *list; QemuOpts *opts; uint64_t opt; - int ret; list = qemu_find_opts("opts_list_01"); g_assert(list != NULL); @@ -238,16 +238,16 @@ static void test_qemu_opt_get_number(void) opt = qemu_opt_get_number(opts, "number1", 5); g_assert(opt == 5); - ret = qemu_opt_set_number(opts, "number1", 10); - g_assert(ret == 0); + qemu_opt_set_number(opts, "number1", 10, &err); + g_assert(!err); /* now we have set number1, should know about it */ opt = qemu_opt_get_number(opts, "number1", 5); g_assert(opt == 10); /* having reset it, the returned should be the reset one not defval */ - ret = qemu_opt_set_number(opts, "number1", 15); - g_assert(ret == 0); + qemu_opt_set_number(opts, "number1", 15, &err); + g_assert(!err); opt = qemu_opt_get_number(opts, "number1", 5); g_assert(opt == 15); @@ -349,10 +349,10 @@ static void test_qemu_opt_unset(void) static void test_qemu_opts_reset(void) { + Error *err = NULL; QemuOptsList *list; QemuOpts *opts; uint64_t opt; - int ret; list = qemu_find_opts("opts_list_01"); g_assert(list != NULL); @@ -372,8 +372,8 @@ static void test_qemu_opts_reset(void) opt = qemu_opt_get_number(opts, "number1", 5); g_assert(opt == 5); - ret = qemu_opt_set_number(opts, "number1", 10); - g_assert(ret == 0); + qemu_opt_set_number(opts, "number1", 10, &err); + g_assert(!err); /* now we have set number1, should know about it */ opt = qemu_opt_get_number(opts, "number1", 5); diff --git a/util/qemu-option.c b/util/qemu-option.c index 4de8326..c873b6c 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -589,7 +589,8 @@ void qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val, QTAILQ_INSERT_TAIL(&opts->head, opt, next); } -int qemu_opt_set_number(QemuOpts *opts, const char *name, int64_t val) +void qemu_opt_set_number(QemuOpts *opts, const char *name, int64_t val, + Error **errp) { QemuOpt *opt; const QemuOptDesc *desc = opts->list->desc; @@ -597,9 +598,9 @@ int qemu_opt_set_number(QemuOpts *opts, const char *name, int64_t val) opt = g_malloc0(sizeof(*opt)); opt->desc = find_desc_by_name(desc, name); if (!opt->desc && !opts_accepts_any(opts)) { - qerror_report(QERR_INVALID_PARAMETER, name); + error_set(errp, QERR_INVALID_PARAMETER, name); g_free(opt); - return -1; + return; } opt->name = g_strdup(name); @@ -607,8 +608,6 @@ int qemu_opt_set_number(QemuOpts *opts, const char *name, int64_t val) opt->value.uint = val; opt->str = g_strdup_printf("%" PRId64, val); QTAILQ_INSERT_TAIL(&opts->head, opt, next); - - return 0; } int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque, diff --git a/vl.c b/vl.c index 27e738a..8ea8b08 100644 --- a/vl.c +++ b/vl.c @@ -2684,7 +2684,7 @@ static void set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size) } /* store value for the future use */ - qemu_opt_set_number(opts, "size", ram_size); + qemu_opt_set_number(opts, "size", ram_size, &error_abort); *maxram_size = ram_size; maxmem_str = qemu_opt_get(opts, "maxmem"); -- 1.9.3 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PULL 03/13] QemuOpts: Convert qemu_opts_set() to Error, fix its use 2015-02-26 14:07 [Qemu-devel] [PULL 00/13] QemuOpts: Convert various setters to Error Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 01/13] QemuOpts: Convert qemu_opt_set_bool() to Error, fix its use Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 02/13] QemuOpts: Convert qemu_opt_set_number() " Markus Armbruster @ 2015-02-26 14:07 ` Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 04/13] qemu-img: Suppress unhelpful extra errors in convert, resize Markus Armbruster ` (10 subsequent siblings) 13 siblings, 0 replies; 15+ messages in thread From: Markus Armbruster @ 2015-02-26 14:07 UTC (permalink / raw) To: qemu-devel Return the Error object instead of reporting it with qerror_report_err(). Change callers that assume the function can't fail to pass &error_abort, so that should the assumption ever break, it'll break noisily. Turns out all callers outside its unit test assume that. We could drop the Error ** argument, but that would make the interface less regular, so don't. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- include/qemu/option.h | 4 ++-- net/net.c | 4 ++-- tests/test-qemu-opts.c | 6 +++--- util/qemu-option.c | 11 +++++------ vl.c | 15 ++++++++++----- 5 files changed, 22 insertions(+), 18 deletions(-) diff --git a/include/qemu/option.h b/include/qemu/option.h index 7422cc2..7d6addc 100644 --- a/include/qemu/option.h +++ b/include/qemu/option.h @@ -110,8 +110,8 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id, int fail_if_exists, Error **errp); void qemu_opts_reset(QemuOptsList *list); void qemu_opts_loc_restore(QemuOpts *opts); -int qemu_opts_set(QemuOptsList *list, const char *id, - const char *name, const char *value); +void qemu_opts_set(QemuOptsList *list, const char *id, + const char *name, const char *value, Error **errp); const char *qemu_opts_id(QemuOpts *opts); void qemu_opts_set_id(QemuOpts *opts, char *id); void qemu_opts_del(QemuOpts *opts); diff --git a/net/net.c b/net/net.c index 5146361..2af07e6 100644 --- a/net/net.c +++ b/net/net.c @@ -1296,9 +1296,9 @@ int net_init_clients(void) if (default_net) { /* if no clients, we use a default config */ - qemu_opts_set(net, NULL, "type", "nic"); + qemu_opts_set(net, NULL, "type", "nic", &error_abort); #ifdef CONFIG_SLIRP - qemu_opts_set(net, NULL, "type", "user"); + qemu_opts_set(net, NULL, "type", "user", &error_abort); #endif } diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c index 3fea96a..11cd1bd 100644 --- a/tests/test-qemu-opts.c +++ b/tests/test-qemu-opts.c @@ -388,9 +388,9 @@ static void test_qemu_opts_reset(void) static void test_qemu_opts_set(void) { + Error *err = NULL; QemuOptsList *list; QemuOpts *opts; - int ret; const char *opt; list = qemu_find_opts("opts_list_01"); @@ -403,8 +403,8 @@ static void test_qemu_opts_set(void) g_assert(opts == NULL); /* implicitly create opts and set str3 value */ - ret = qemu_opts_set(list, NULL, "str3", "value"); - g_assert(ret == 0); + qemu_opts_set(list, NULL, "str3", "value", &err); + g_assert(!err); g_assert(!QTAILQ_EMPTY(&list->head)); /* get the just created opts */ diff --git a/util/qemu-option.c b/util/qemu-option.c index c873b6c..56711ca 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -690,19 +690,18 @@ void qemu_opts_loc_restore(QemuOpts *opts) loc_restore(&opts->loc); } -int qemu_opts_set(QemuOptsList *list, const char *id, - const char *name, const char *value) +void qemu_opts_set(QemuOptsList *list, const char *id, + const char *name, const char *value, Error **errp) { QemuOpts *opts; Error *local_err = NULL; opts = qemu_opts_create(list, id, 1, &local_err); if (local_err) { - qerror_report_err(local_err); - error_free(local_err); - return -1; + error_propagate(errp, local_err); + return; } - return qemu_opt_set(opts, name, value); + qemu_opt_set_err(opts, name, value, errp); } const char *qemu_opts_id(QemuOpts *opts) diff --git a/vl.c b/vl.c index 8ea8b08..80e2be0 100644 --- a/vl.c +++ b/vl.c @@ -3023,16 +3023,20 @@ int main(int argc, char **argv, char **envp) } break; case QEMU_OPTION_kernel: - qemu_opts_set(qemu_find_opts("machine"), 0, "kernel", optarg); + qemu_opts_set(qemu_find_opts("machine"), 0, "kernel", optarg, + &error_abort); break; case QEMU_OPTION_initrd: - qemu_opts_set(qemu_find_opts("machine"), 0, "initrd", optarg); + qemu_opts_set(qemu_find_opts("machine"), 0, "initrd", optarg, + &error_abort); break; case QEMU_OPTION_append: - qemu_opts_set(qemu_find_opts("machine"), 0, "append", optarg); + qemu_opts_set(qemu_find_opts("machine"), 0, "append", optarg, + &error_abort); break; case QEMU_OPTION_dtb: - qemu_opts_set(qemu_find_opts("machine"), 0, "dtb", optarg); + qemu_opts_set(qemu_find_opts("machine"), 0, "dtb", optarg, + &error_abort); break; case QEMU_OPTION_cdrom: drive_add(IF_DEFAULT, 2, optarg, CDROM_OPTS); @@ -3136,7 +3140,8 @@ int main(int argc, char **argv, char **envp) } break; case QEMU_OPTION_bios: - qemu_opts_set(qemu_find_opts("machine"), 0, "firmware", optarg); + qemu_opts_set(qemu_find_opts("machine"), 0, "firmware", optarg, + &error_abort); break; case QEMU_OPTION_singlestep: singlestep = 1; -- 1.9.3 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PULL 04/13] qemu-img: Suppress unhelpful extra errors in convert, resize 2015-02-26 14:07 [Qemu-devel] [PULL 00/13] QemuOpts: Convert various setters to Error Markus Armbruster ` (2 preceding siblings ...) 2015-02-26 14:07 ` [Qemu-devel] [PULL 03/13] QemuOpts: Convert qemu_opts_set() " Markus Armbruster @ 2015-02-26 14:07 ` Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 05/13] block: Suppress unhelpful extra errors in bdrv_img_create() Markus Armbruster ` (9 subsequent siblings) 13 siblings, 0 replies; 15+ messages in thread From: Markus Armbruster @ 2015-02-26 14:07 UTC (permalink / raw) To: qemu-devel add_old_style_options() for img_convert() and img_resize() use qemu_opt_set(), which reports errors with qerror_report_err(). Its error messages aren't helpful here, the caller reports one that actually makes sense. Reproducer: $ qemu-img convert -B raw in.img out.img qemu-img: Invalid parameter 'backing_file' qemu-img: Backing file not supported for file format 'raw' Switch to qemu_opt_set_err() to get rid of the unwanted messages. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- qemu-img.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index 752eecb..702a051 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -332,17 +332,23 @@ static int add_old_style_options(const char *fmt, QemuOpts *opts, const char *base_filename, const char *base_fmt) { + Error *err = NULL; + if (base_filename) { - if (qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, base_filename)) { + qemu_opt_set_err(opts, BLOCK_OPT_BACKING_FILE, base_filename, &err); + if (err) { error_report("Backing file not supported for file format '%s'", fmt); + error_free(err); return -1; } } if (base_fmt) { - if (qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, base_fmt)) { + qemu_opt_set_err(opts, BLOCK_OPT_BACKING_FMT, base_fmt, &err); + if (err) { error_report("Backing file format not supported for file " "format '%s'", fmt); + error_free(err); return -1; } } @@ -2750,6 +2756,7 @@ out: static int img_resize(int argc, char **argv) { + Error *err = NULL; int c, ret, relative; const char *filename, *fmt, *size; int64_t n, total_size; @@ -2821,8 +2828,9 @@ static int img_resize(int argc, char **argv) /* Parse size */ param = qemu_opts_create(&resize_options, NULL, 0, &error_abort); - if (qemu_opt_set(param, BLOCK_OPT_SIZE, size)) { - /* Error message already printed when size parsing fails */ + qemu_opt_set_err(param, BLOCK_OPT_SIZE, size, &err); + if (err) { + error_report_err(err); ret = -1; qemu_opts_del(param); goto out; -- 1.9.3 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PULL 05/13] block: Suppress unhelpful extra errors in bdrv_img_create() 2015-02-26 14:07 [Qemu-devel] [PULL 00/13] QemuOpts: Convert various setters to Error Markus Armbruster ` (3 preceding siblings ...) 2015-02-26 14:07 ` [Qemu-devel] [PULL 04/13] qemu-img: Suppress unhelpful extra errors in convert, resize Markus Armbruster @ 2015-02-26 14:07 ` Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 06/13] QemuOpts: Drop qemu_opt_set(), rename qemu_opt_set_err(), fix use Markus Armbruster ` (8 subsequent siblings) 13 siblings, 0 replies; 15+ messages in thread From: Markus Armbruster @ 2015-02-26 14:07 UTC (permalink / raw) To: qemu-devel bdrv_img_create() uses qemu_opt_set(), which reports errors with qerror_report_err(). Its error messages aren't helpful here, the caller reports one that actually makes sense. I don't know how to trigger the error conditions, though. Switch to qemu_opt_set_err() to get rid of the unwanted messages. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- block.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/block.c b/block.c index f59c4cf..f5ae5cd 100644 --- a/block.c +++ b/block.c @@ -5660,7 +5660,9 @@ void bdrv_img_create(const char *filename, const char *fmt, } if (base_filename) { - if (qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, base_filename)) { + qemu_opt_set_err(opts, BLOCK_OPT_BACKING_FILE, base_filename, + &local_err); + if (local_err) { error_setg(errp, "Backing file not supported for file format '%s'", fmt); goto out; @@ -5668,7 +5670,8 @@ void bdrv_img_create(const char *filename, const char *fmt, } if (base_fmt) { - if (qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, base_fmt)) { + qemu_opt_set_err(opts, BLOCK_OPT_BACKING_FMT, base_fmt, &local_err); + if (local_err) { error_setg(errp, "Backing file format not supported for file " "format '%s'", fmt); goto out; -- 1.9.3 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PULL 06/13] QemuOpts: Drop qemu_opt_set(), rename qemu_opt_set_err(), fix use 2015-02-26 14:07 [Qemu-devel] [PULL 00/13] QemuOpts: Convert various setters to Error Markus Armbruster ` (4 preceding siblings ...) 2015-02-26 14:07 ` [Qemu-devel] [PULL 05/13] block: Suppress unhelpful extra errors in bdrv_img_create() Markus Armbruster @ 2015-02-26 14:07 ` Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 07/13] QemuOpts: Propagate errors through opts_do_parse() Markus Armbruster ` (7 subsequent siblings) 13 siblings, 0 replies; 15+ messages in thread From: Markus Armbruster @ 2015-02-26 14:07 UTC (permalink / raw) To: qemu-devel 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(). Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- 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(-) diff --git a/block.c b/block.c index f5ae5cd..ebbb515 100644 --- a/block.c +++ b/block.c @@ -5660,8 +5660,7 @@ void bdrv_img_create(const char *filename, const char *fmt, } if (base_filename) { - qemu_opt_set_err(opts, BLOCK_OPT_BACKING_FILE, base_filename, - &local_err); + qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, base_filename, &local_err); if (local_err) { error_setg(errp, "Backing file not supported for file format '%s'", fmt); @@ -5670,7 +5669,7 @@ void bdrv_img_create(const char *filename, const char *fmt, } if (base_fmt) { - qemu_opt_set_err(opts, BLOCK_OPT_BACKING_FMT, base_fmt, &local_err); + qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, base_fmt, &local_err); if (local_err) { error_setg(errp, "Backing file format not supported for file " "format '%s'", fmt); diff --git a/block/qcow2.c b/block/qcow2.c index 7720874..50e0a94 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1859,7 +1859,8 @@ static int qcow2_create2(const char *filename, int64_t total_size, qemu_opt_set_number(opts, BLOCK_OPT_SIZE, aligned_total_size + meta_size, &error_abort); - qemu_opt_set(opts, BLOCK_OPT_PREALLOC, PreallocMode_lookup[prealloc]); + qemu_opt_set(opts, BLOCK_OPT_PREALLOC, PreallocMode_lookup[prealloc], + &error_abort); } ret = bdrv_create_file(filename, opts, &local_err); diff --git a/block/vvfat.c b/block/vvfat.c index 5e32d77..9be632f 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -2926,7 +2926,7 @@ static int enable_write_target(BDRVVVFATState *s, Error **errp) opts = qemu_opts_create(bdrv_qcow->create_opts, NULL, 0, &error_abort); qemu_opt_set_number(opts, BLOCK_OPT_SIZE, s->sector_count * 512, &error_abort); - qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, "fat:"); + qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, "fat:", &error_abort); ret = bdrv_create(bdrv_qcow, s->qcow_filename, opts, errp); qemu_opts_del(opts); diff --git a/blockdev.c b/blockdev.c index 79c33ab..5aa23b5 100644 --- a/blockdev.c +++ b/blockdev.c @@ -187,14 +187,14 @@ QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file, return NULL; } if (type != IF_DEFAULT) { - qemu_opt_set(opts, "if", if_name[type]); + qemu_opt_set(opts, "if", if_name[type], &error_abort); } if (index >= 0) { snprintf(buf, sizeof(buf), "%d", index); - qemu_opt_set(opts, "index", buf); + qemu_opt_set(opts, "index", buf, &error_abort); } if (file) - qemu_opt_set(opts, "file", file); + qemu_opt_set(opts, "file", file, &error_abort); return opts; } @@ -584,7 +584,7 @@ static void qemu_opt_rename(QemuOpts *opts, const char *from, const char *to, /* rename all items in opts */ while ((value = qemu_opt_get(opts, from))) { - qemu_opt_set(opts, to, value); + qemu_opt_set(opts, to, value, &error_abort); qemu_opt_unset(opts, from); } } @@ -935,13 +935,14 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) devopts = qemu_opts_create(qemu_find_opts("device"), NULL, 0, &error_abort); if (arch_type == QEMU_ARCH_S390X) { - qemu_opt_set(devopts, "driver", "virtio-blk-s390"); + qemu_opt_set(devopts, "driver", "virtio-blk-s390", &error_abort); } else { - qemu_opt_set(devopts, "driver", "virtio-blk-pci"); + qemu_opt_set(devopts, "driver", "virtio-blk-pci", &error_abort); } - qemu_opt_set(devopts, "drive", qdict_get_str(bs_opts, "id")); + qemu_opt_set(devopts, "drive", qdict_get_str(bs_opts, "id"), + &error_abort); if (devaddr) { - qemu_opt_set(devopts, "addr", devaddr); + qemu_opt_set(devopts, "addr", devaddr, &error_abort); } } diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c index beea6d2..501a918 100644 --- a/hw/pci/pci-hotplug-old.c +++ b/hw/pci/pci-hotplug-old.c @@ -87,7 +87,7 @@ static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon, return NULL; } - qemu_opt_set(opts, "type", "nic"); + qemu_opt_set(opts, "type", "nic", &error_abort); ret = net_client_init(opts, 0, &local_err); if (local_err) { diff --git a/hw/usb/dev-network.c b/hw/usb/dev-network.c index b27b1f4..1866991 100644 --- a/hw/usb/dev-network.c +++ b/hw/usb/dev-network.c @@ -1394,8 +1394,8 @@ static USBDevice *usb_net_init(USBBus *bus, const char *cmdline) if (!opts) { return NULL; } - qemu_opt_set(opts, "type", "nic"); - qemu_opt_set(opts, "model", "usb"); + qemu_opt_set(opts, "type", "nic", &error_abort); + qemu_opt_set(opts, "model", "usb", &error_abort); idx = net_client_init(opts, 0, &local_err); if (local_err) { diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c index af2e1b9..65d9aa6 100644 --- a/hw/usb/dev-storage.c +++ b/hw/usb/dev-storage.c @@ -683,7 +683,7 @@ static USBDevice *usb_msd_init(USBBus *bus, const char *filename) if (strstart(filename, "format=", &p2)) { int len = MIN(p1 - p2, sizeof(fmt)); pstrcpy(fmt, len, p2); - qemu_opt_set(opts, "format", fmt); + qemu_opt_set(opts, "format", fmt, &error_abort); } else if (*filename != ':') { error_report("unrecognized USB mass-storage option %s", filename); return NULL; @@ -694,8 +694,8 @@ static USBDevice *usb_msd_init(USBBus *bus, const char *filename) error_report("block device specification needed"); return NULL; } - qemu_opt_set(opts, "file", filename); - qemu_opt_set(opts, "if", "none"); + qemu_opt_set(opts, "file", filename, &error_abort); + qemu_opt_set(opts, "if", "none", &error_abort); /* create host drive */ dinfo = drive_new(opts, 0); diff --git a/hw/watchdog/watchdog.c b/hw/watchdog/watchdog.c index c307f9b..54440c9 100644 --- a/hw/watchdog/watchdog.c +++ b/hw/watchdog/watchdog.c @@ -68,7 +68,7 @@ int select_watchdog(const char *p) /* add the device */ opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0, &error_abort); - qemu_opt_set(opts, "driver", p); + qemu_opt_set(opts, "driver", p, &error_abort); return 0; } } diff --git a/include/qemu/option.h b/include/qemu/option.h index 7d6addc..a41ee92 100644 --- a/include/qemu/option.h +++ b/include/qemu/option.h @@ -94,9 +94,8 @@ uint64_t qemu_opt_get_number_del(QemuOpts *opts, const char *name, uint64_t qemu_opt_get_size_del(QemuOpts *opts, const char *name, uint64_t defval); int qemu_opt_unset(QemuOpts *opts, const char *name); -int qemu_opt_set(QemuOpts *opts, const char *name, const char *value); -void qemu_opt_set_err(QemuOpts *opts, const char *name, const char *value, - Error **errp); +void qemu_opt_set(QemuOpts *opts, const char *name, const char *value, + Error **errp); void qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val, Error **errp); void qemu_opt_set_number(QemuOpts *opts, const char *name, int64_t val, diff --git a/net/net.c b/net/net.c index 2af07e6..11fb769 100644 --- a/net/net.c +++ b/net/net.c @@ -970,7 +970,7 @@ void hmp_host_net_add(Monitor *mon, const QDict *qdict) return; } - qemu_opt_set(opts, "type", device); + qemu_opt_set(opts, "type", device, &error_abort); net_client_init(opts, 0, &local_err); if (local_err) { diff --git a/qdev-monitor.c b/qdev-monitor.c index 0c25b82..5d30ac5 100644 --- a/qdev-monitor.c +++ b/qdev-monitor.c @@ -772,8 +772,8 @@ int qemu_global_option(const char *str) } opts = qemu_opts_create(&qemu_global_opts, NULL, 0, &error_abort); - qemu_opt_set(opts, "driver", driver); - qemu_opt_set(opts, "property", property); - qemu_opt_set(opts, "value", str+offset+1); + qemu_opt_set(opts, "driver", driver, &error_abort); + qemu_opt_set(opts, "property", property, &error_abort); + qemu_opt_set(opts, "value", str + offset + 1, &error_abort); return 0; } diff --git a/qemu-char.c b/qemu-char.c index 8159462..3ed46ee 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -3312,14 +3312,14 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename) if (strstart(filename, "mon:", &p)) { filename = p; - qemu_opt_set(opts, "mux", "on"); + qemu_opt_set(opts, "mux", "on", &error_abort); if (strcmp(filename, "stdio") == 0) { /* Monitor is muxed to stdio: do not exit on Ctrl+C by default * but pass it to the guest. Handle this only for compat syntax, * for -chardev syntax we have special option for this. * This is what -nographic did, redirecting+muxing serial+monitor * to stdio causing Ctrl+C to be passed to guest. */ - qemu_opt_set(opts, "signal", "off"); + qemu_opt_set(opts, "signal", "off", &error_abort); } } @@ -3329,20 +3329,20 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename) strcmp(filename, "braille") == 0 || strcmp(filename, "testdev") == 0 || strcmp(filename, "stdio") == 0) { - qemu_opt_set(opts, "backend", filename); + qemu_opt_set(opts, "backend", filename, &error_abort); return opts; } if (strstart(filename, "vc", &p)) { - qemu_opt_set(opts, "backend", "vc"); + qemu_opt_set(opts, "backend", "vc", &error_abort); if (*p == ':') { if (sscanf(p+1, "%7[0-9]x%7[0-9]", width, height) == 2) { /* pixels */ - qemu_opt_set(opts, "width", width); - qemu_opt_set(opts, "height", height); + qemu_opt_set(opts, "width", width, &error_abort); + qemu_opt_set(opts, "height", height, &error_abort); } else if (sscanf(p+1, "%7[0-9]Cx%7[0-9]C", width, height) == 2) { /* chars */ - qemu_opt_set(opts, "cols", width); - qemu_opt_set(opts, "rows", height); + qemu_opt_set(opts, "cols", width, &error_abort); + qemu_opt_set(opts, "rows", height, &error_abort); } else { goto fail; } @@ -3350,22 +3350,22 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename) return opts; } if (strcmp(filename, "con:") == 0) { - qemu_opt_set(opts, "backend", "console"); + qemu_opt_set(opts, "backend", "console", &error_abort); return opts; } if (strstart(filename, "COM", NULL)) { - qemu_opt_set(opts, "backend", "serial"); - qemu_opt_set(opts, "path", filename); + qemu_opt_set(opts, "backend", "serial", &error_abort); + qemu_opt_set(opts, "path", filename, &error_abort); return opts; } if (strstart(filename, "file:", &p)) { - qemu_opt_set(opts, "backend", "file"); - qemu_opt_set(opts, "path", p); + qemu_opt_set(opts, "backend", "file", &error_abort); + qemu_opt_set(opts, "path", p, &error_abort); return opts; } if (strstart(filename, "pipe:", &p)) { - qemu_opt_set(opts, "backend", "pipe"); - qemu_opt_set(opts, "path", p); + qemu_opt_set(opts, "backend", "pipe", &error_abort); + qemu_opt_set(opts, "path", p, &error_abort); return opts; } if (strstart(filename, "tcp:", &p) || @@ -3375,27 +3375,27 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename) if (sscanf(p, ":%32[^,]%n", port, &pos) < 1) goto fail; } - qemu_opt_set(opts, "backend", "socket"); - qemu_opt_set(opts, "host", host); - qemu_opt_set(opts, "port", port); + qemu_opt_set(opts, "backend", "socket", &error_abort); + qemu_opt_set(opts, "host", host, &error_abort); + qemu_opt_set(opts, "port", port, &error_abort); if (p[pos] == ',') { if (qemu_opts_do_parse(opts, p+pos+1, NULL) != 0) goto fail; } if (strstart(filename, "telnet:", &p)) - qemu_opt_set(opts, "telnet", "on"); + qemu_opt_set(opts, "telnet", "on", &error_abort); return opts; } if (strstart(filename, "udp:", &p)) { - qemu_opt_set(opts, "backend", "udp"); + qemu_opt_set(opts, "backend", "udp", &error_abort); if (sscanf(p, "%64[^:]:%32[^@,]%n", host, port, &pos) < 2) { host[0] = 0; if (sscanf(p, ":%32[^@,]%n", port, &pos) < 1) { goto fail; } } - qemu_opt_set(opts, "host", host); - qemu_opt_set(opts, "port", port); + qemu_opt_set(opts, "host", host, &error_abort); + qemu_opt_set(opts, "port", port, &error_abort); if (p[pos] == '@') { p += pos + 1; if (sscanf(p, "%64[^:]:%32[^,]%n", host, port, &pos) < 2) { @@ -3404,26 +3404,26 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename) goto fail; } } - qemu_opt_set(opts, "localaddr", host); - qemu_opt_set(opts, "localport", port); + qemu_opt_set(opts, "localaddr", host, &error_abort); + qemu_opt_set(opts, "localport", port, &error_abort); } return opts; } if (strstart(filename, "unix:", &p)) { - qemu_opt_set(opts, "backend", "socket"); + qemu_opt_set(opts, "backend", "socket", &error_abort); if (qemu_opts_do_parse(opts, p, "path") != 0) goto fail; return opts; } if (strstart(filename, "/dev/parport", NULL) || strstart(filename, "/dev/ppi", NULL)) { - qemu_opt_set(opts, "backend", "parport"); - qemu_opt_set(opts, "path", filename); + qemu_opt_set(opts, "backend", "parport", &error_abort); + qemu_opt_set(opts, "path", filename, &error_abort); return opts; } if (strstart(filename, "/dev/", NULL)) { - qemu_opt_set(opts, "backend", "tty"); - qemu_opt_set(opts, "path", filename); + qemu_opt_set(opts, "backend", "tty", &error_abort); + qemu_opt_set(opts, "path", filename, &error_abort); return opts; } diff --git a/qemu-img.c b/qemu-img.c index 702a051..54e34bd 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -335,7 +335,7 @@ static int add_old_style_options(const char *fmt, QemuOpts *opts, Error *err = NULL; if (base_filename) { - qemu_opt_set_err(opts, BLOCK_OPT_BACKING_FILE, base_filename, &err); + qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, base_filename, &err); if (err) { error_report("Backing file not supported for file format '%s'", fmt); @@ -344,7 +344,7 @@ static int add_old_style_options(const char *fmt, QemuOpts *opts, } } if (base_fmt) { - qemu_opt_set_err(opts, BLOCK_OPT_BACKING_FMT, base_fmt, &err); + qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, base_fmt, &err); if (err) { error_report("Backing file format not supported for file " "format '%s'", fmt); @@ -2828,7 +2828,7 @@ static int img_resize(int argc, char **argv) /* Parse size */ param = qemu_opts_create(&resize_options, NULL, 0, &error_abort); - qemu_opt_set_err(param, BLOCK_OPT_SIZE, size, &err); + qemu_opt_set(param, BLOCK_OPT_SIZE, size, &err); if (err) { error_report_err(err); ret = -1; diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c index 11cd1bd..da56492 100644 --- a/tests/test-qemu-opts.c +++ b/tests/test-qemu-opts.c @@ -148,13 +148,13 @@ static void test_qemu_opt_get(void) opt = qemu_opt_get(opts, "str2"); g_assert(opt == NULL); - qemu_opt_set(opts, "str2", "value"); + qemu_opt_set(opts, "str2", "value", &error_abort); /* now we have set str2, should know about it */ opt = qemu_opt_get(opts, "str2"); g_assert_cmpstr(opt, ==, "value"); - qemu_opt_set(opts, "str2", "value2"); + qemu_opt_set(opts, "str2", "value2", &error_abort); /* having reset the value, the returned should be the reset one */ opt = qemu_opt_get(opts, "str2"); @@ -331,7 +331,7 @@ static void test_qemu_opt_unset(void) g_assert_cmpstr(value, ==, "value"); /* reset it to value2 */ - qemu_opt_set(opts, "key", "value2"); + qemu_opt_set(opts, "key", "value2", &error_abort); value = qemu_opt_get(opts, "key"); g_assert_cmpstr(value, ==, "value2"); diff --git a/util/qemu-config.c b/util/qemu-config.c index b13efe2..f3463df 100644 --- a/util/qemu-config.c +++ 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); + if (local_err) { + error_report_err(local_err); return -1; } return 0; @@ -335,7 +338,9 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname) error_report("no group defined"); goto out; } - if (qemu_opt_set(opts, arg, value) != 0) { + qemu_opt_set(opts, arg, value, &local_err); + if (local_err) { + error_report_err(local_err); goto out; } continue; diff --git a/util/qemu-option.c b/util/qemu-option.c index 56711ca..928b86f 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -548,22 +548,8 @@ static void opt_set(QemuOpts *opts, const char *name, const char *value, } } -int qemu_opt_set(QemuOpts *opts, const char *name, const char *value) -{ - Error *local_err = NULL; - - opt_set(opts, name, value, false, &local_err); - if (local_err) { - qerror_report_err(local_err); - error_free(local_err); - return -1; - } - - return 0; -} - -void qemu_opt_set_err(QemuOpts *opts, const char *name, const char *value, - Error **errp) +void qemu_opt_set(QemuOpts *opts, const char *name, const char *value, + Error **errp) { opt_set(opts, name, value, false, errp); } @@ -701,7 +687,7 @@ void qemu_opts_set(QemuOptsList *list, const char *id, error_propagate(errp, local_err); return; } - qemu_opt_set_err(opts, name, value, errp); + qemu_opt_set(opts, name, value, errp); } const char *qemu_opts_id(QemuOpts *opts) @@ -921,7 +907,7 @@ static void qemu_opts_from_qdict_1(const char *key, QObject *obj, void *opaque) return; } - qemu_opt_set_err(state->opts, key, value, state->errp); + qemu_opt_set(state->opts, key, value, state->errp); } /* diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c index 7205dad..e908523 100644 --- a/util/qemu-sockets.c +++ b/util/qemu-sockets.c @@ -200,10 +200,12 @@ listen: return -1; } snprintf(uport, sizeof(uport), "%d", inet_getport(e) - port_offset); - qemu_opt_set(opts, "host", uaddr); - qemu_opt_set(opts, "port", uport); - qemu_opt_set(opts, "ipv6", (e->ai_family == PF_INET6) ? "on" : "off"); - qemu_opt_set(opts, "ipv4", (e->ai_family != PF_INET6) ? "on" : "off"); + qemu_opt_set(opts, "host", uaddr, &error_abort); + qemu_opt_set(opts, "port", uport, &error_abort); + qemu_opt_set(opts, "ipv6", (e->ai_family == PF_INET6) ? "on" : "off", + &error_abort); + qemu_opt_set(opts, "ipv4", (e->ai_family != PF_INET6) ? "on" : "off", + &error_abort); freeaddrinfo(res); return slisten; } @@ -586,10 +588,10 @@ static void inet_addr_to_opts(QemuOpts *opts, const InetSocketAddress *addr) if (addr->has_to) { char to[20]; snprintf(to, sizeof(to), "%d", addr->to); - qemu_opt_set(opts, "to", to); + qemu_opt_set(opts, "to", to, &error_abort); } - qemu_opt_set(opts, "host", addr->host); - qemu_opt_set(opts, "port", addr->port); + qemu_opt_set(opts, "host", addr->host, &error_abort); + qemu_opt_set(opts, "port", addr->port, &error_abort); } int inet_listen(const char *str, char *ostr, int olen, @@ -726,7 +728,7 @@ int unix_listen_opts(QemuOpts *opts, Error **errp) goto err; } close(fd); - qemu_opt_set(opts, "path", un.sun_path); + qemu_opt_set(opts, "path", un.sun_path, &error_abort); } unlink(un.sun_path); @@ -838,11 +840,11 @@ int unix_listen(const char *str, char *ostr, int olen, Error **errp) if (len) { path = g_malloc(len+1); snprintf(path, len+1, "%.*s", len, str); - qemu_opt_set(opts, "path", path); + qemu_opt_set(opts, "path", path, &error_abort); g_free(path); } } else { - qemu_opt_set(opts, "path", str); + qemu_opt_set(opts, "path", str, &error_abort); } sock = unix_listen_opts(opts, errp); @@ -859,7 +861,7 @@ int unix_connect(const char *path, Error **errp) int sock; opts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort); - qemu_opt_set(opts, "path", path); + qemu_opt_set(opts, "path", path, &error_abort); sock = unix_connect_opts(opts, errp, NULL, NULL); qemu_opts_del(opts); return sock; @@ -876,7 +878,7 @@ int unix_nonblocking_connect(const char *path, g_assert(callback != NULL); opts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort); - qemu_opt_set(opts, "path", path); + qemu_opt_set(opts, "path", path, &error_abort); sock = unix_connect_opts(opts, errp, callback, opaque); qemu_opts_del(opts); return sock; @@ -933,7 +935,7 @@ int socket_connect(SocketAddress *addr, Error **errp, break; case SOCKET_ADDRESS_KIND_UNIX: - qemu_opt_set(opts, "path", addr->q_unix->path); + qemu_opt_set(opts, "path", addr->q_unix->path, &error_abort); fd = unix_connect_opts(opts, errp, callback, opaque); break; @@ -965,7 +967,7 @@ int socket_listen(SocketAddress *addr, Error **errp) break; case SOCKET_ADDRESS_KIND_UNIX: - qemu_opt_set(opts, "path", addr->q_unix->path); + qemu_opt_set(opts, "path", addr->q_unix->path, &error_abort); fd = unix_listen_opts(opts, errp); break; @@ -990,8 +992,8 @@ int socket_dgram(SocketAddress *remote, SocketAddress *local, Error **errp) case SOCKET_ADDRESS_KIND_INET: inet_addr_to_opts(opts, remote->inet); if (local) { - qemu_opt_set(opts, "localaddr", local->inet->host); - qemu_opt_set(opts, "localport", local->inet->port); + qemu_opt_set(opts, "localaddr", local->inet->host, &error_abort); + qemu_opt_set(opts, "localport", local->inet->port, &error_abort); } fd = inet_dgram_opts(opts, errp); break; diff --git a/vl.c b/vl.c index 80e2be0..66b0ad8 100644 --- a/vl.c +++ b/vl.c @@ -1096,7 +1096,7 @@ static int drive_init_func(QemuOpts *opts, void *opaque) static int drive_enable_snapshot(QemuOpts *opts, void *opaque) { if (qemu_opt_get(opts, "snapshot") == NULL) { - qemu_opt_set(opts, "snapshot", "on"); + qemu_opt_set(opts, "snapshot", "on", &error_abort); } return 0; } @@ -2074,7 +2074,7 @@ static int balloon_parse(const char *arg) opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0, &error_abort); } - qemu_opt_set(opts, "driver", "virtio-balloon"); + qemu_opt_set(opts, "driver", "virtio-balloon", &error_abort); return 0; } @@ -2220,11 +2220,11 @@ static void monitor_parse(const char *optarg, const char *mode, bool pretty) error_report_err(local_err); exit(1); } - qemu_opt_set(opts, "mode", mode); - qemu_opt_set(opts, "chardev", label); + qemu_opt_set(opts, "mode", mode, &error_abort); + qemu_opt_set(opts, "chardev", label, &error_abort); qemu_opt_set_bool(opts, "pretty", pretty, &error_abort); if (def) - qemu_opt_set(opts, "default", "on"); + qemu_opt_set(opts, "default", "on", &error_abort); monitor_device_index++; } @@ -2336,13 +2336,13 @@ static int virtcon_parse(const char *devname) bus_opts = qemu_opts_create(device, NULL, 0, &error_abort); if (arch_type == QEMU_ARCH_S390X) { - qemu_opt_set(bus_opts, "driver", "virtio-serial-s390"); + qemu_opt_set(bus_opts, "driver", "virtio-serial-s390", &error_abort); } else { - qemu_opt_set(bus_opts, "driver", "virtio-serial-pci"); + qemu_opt_set(bus_opts, "driver", "virtio-serial-pci", &error_abort); } dev_opts = qemu_opts_create(device, NULL, 0, &error_abort); - qemu_opt_set(dev_opts, "driver", "virtconsole"); + qemu_opt_set(dev_opts, "driver", "virtconsole", &error_abort); snprintf(label, sizeof(label), "virtcon%d", index); virtcon_hds[index] = qemu_chr_new(label, devname, NULL); @@ -2351,7 +2351,7 @@ static int virtcon_parse(const char *devname) " to character backend '%s'\n", devname); return -1; } - qemu_opt_set(dev_opts, "chardev", label); + qemu_opt_set(dev_opts, "chardev", label, &error_abort); index++; return 0; @@ -2375,7 +2375,7 @@ static int sclp_parse(const char *devname) assert(arch_type == QEMU_ARCH_S390X); dev_opts = qemu_opts_create(device, NULL, 0, NULL); - qemu_opt_set(dev_opts, "driver", "sclpconsole"); + qemu_opt_set(dev_opts, "driver", "sclpconsole", &error_abort); snprintf(label, sizeof(label), "sclpcon%d", index); sclp_hds[index] = qemu_chr_new(label, devname, NULL); @@ -2384,7 +2384,7 @@ static int sclp_parse(const char *devname) " to character backend '%s'\n", devname); return -1; } - qemu_opt_set(dev_opts, "chardev", label); + qemu_opt_set(dev_opts, "chardev", label, &error_abort); index++; return 0; @@ -2402,8 +2402,8 @@ static int debugcon_parse(const char *devname) fprintf(stderr, "qemu: already have a debugcon device\n"); exit(1); } - qemu_opt_set(opts, "driver", "isa-debugcon"); - qemu_opt_set(opts, "chardev", "debugcon"); + qemu_opt_set(opts, "driver", "isa-debugcon", &error_abort); + qemu_opt_set(opts, "chardev", "debugcon", &error_abort); return 0; } @@ -2973,19 +2973,23 @@ int main(int argc, char **argv, char **envp) if (hda_opts != NULL) { char num[16]; snprintf(num, sizeof(num), "%d", cyls); - qemu_opt_set(hda_opts, "cyls", num); + qemu_opt_set(hda_opts, "cyls", num, &error_abort); snprintf(num, sizeof(num), "%d", heads); - qemu_opt_set(hda_opts, "heads", num); + qemu_opt_set(hda_opts, "heads", num, &error_abort); snprintf(num, sizeof(num), "%d", secs); - qemu_opt_set(hda_opts, "secs", num); + qemu_opt_set(hda_opts, "secs", num, &error_abort); if (translation == BIOS_ATA_TRANSLATION_LARGE) { - qemu_opt_set(hda_opts, "trans", "large"); + qemu_opt_set(hda_opts, "trans", "large", + &error_abort); } else if (translation == BIOS_ATA_TRANSLATION_RECHS) { - qemu_opt_set(hda_opts, "trans", "rechs"); + qemu_opt_set(hda_opts, "trans", "rechs", + &error_abort); } else if (translation == BIOS_ATA_TRANSLATION_LBA) { - qemu_opt_set(hda_opts, "trans", "lba"); + qemu_opt_set(hda_opts, "trans", "lba", + &error_abort); } else if (translation == BIOS_ATA_TRANSLATION_NONE) { - qemu_opt_set(hda_opts, "trans", "none"); + qemu_opt_set(hda_opts, "trans", "none", + &error_abort); } } } @@ -3271,24 +3275,27 @@ int main(int argc, char **argv, char **envp) writeout = qemu_opt_get(opts, "writeout"); if (writeout) { #ifdef CONFIG_SYNC_FILE_RANGE - qemu_opt_set(fsdev, "writeout", writeout); + qemu_opt_set(fsdev, "writeout", writeout, &error_abort); #else fprintf(stderr, "writeout=immediate not supported on " "this platform\n"); exit(1); #endif } - qemu_opt_set(fsdev, "fsdriver", qemu_opt_get(opts, "fsdriver")); - qemu_opt_set(fsdev, "path", qemu_opt_get(opts, "path")); + qemu_opt_set(fsdev, "fsdriver", + qemu_opt_get(opts, "fsdriver"), &error_abort); + qemu_opt_set(fsdev, "path", qemu_opt_get(opts, "path"), + &error_abort); qemu_opt_set(fsdev, "security_model", - qemu_opt_get(opts, "security_model")); + qemu_opt_get(opts, "security_model"), + &error_abort); socket = qemu_opt_get(opts, "socket"); if (socket) { - qemu_opt_set(fsdev, "socket", socket); + qemu_opt_set(fsdev, "socket", socket, &error_abort); } sock_fd = qemu_opt_get(opts, "sock_fd"); if (sock_fd) { - qemu_opt_set(fsdev, "sock_fd", sock_fd); + qemu_opt_set(fsdev, "sock_fd", sock_fd, &error_abort); } qemu_opt_set_bool(fsdev, "readonly", @@ -3296,11 +3303,11 @@ int main(int argc, char **argv, char **envp) &error_abort); device = qemu_opts_create(qemu_find_opts("device"), NULL, 0, &error_abort); - qemu_opt_set(device, "driver", "virtio-9p-pci"); + qemu_opt_set(device, "driver", "virtio-9p-pci", &error_abort); qemu_opt_set(device, "fsdev", - qemu_opt_get(opts, "mount_tag")); + qemu_opt_get(opts, "mount_tag"), &error_abort); qemu_opt_set(device, "mount_tag", - qemu_opt_get(opts, "mount_tag")); + qemu_opt_get(opts, "mount_tag"), &error_abort); break; } case QEMU_OPTION_virtfs_synth: { @@ -3313,13 +3320,13 @@ int main(int argc, char **argv, char **envp) fprintf(stderr, "duplicate option: %s\n", "virtfs_synth"); exit(1); } - qemu_opt_set(fsdev, "fsdriver", "synth"); + qemu_opt_set(fsdev, "fsdriver", "synth", &error_abort); device = qemu_opts_create(qemu_find_opts("device"), NULL, 0, &error_abort); - qemu_opt_set(device, "driver", "virtio-9p-pci"); - qemu_opt_set(device, "fsdev", "v_synth"); - qemu_opt_set(device, "mount_tag", "v_synth"); + qemu_opt_set(device, "driver", "virtio-9p-pci", &error_abort); + qemu_opt_set(device, "fsdev", "v_synth", &error_abort); + qemu_opt_set(device, "mount_tag", "v_synth", &error_abort); break; } case QEMU_OPTION_serial: -- 1.9.3 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PULL 07/13] QemuOpts: Propagate errors through opts_do_parse() 2015-02-26 14:07 [Qemu-devel] [PULL 00/13] QemuOpts: Convert various setters to Error Markus Armbruster ` (5 preceding siblings ...) 2015-02-26 14:07 ` [Qemu-devel] [PULL 06/13] QemuOpts: Drop qemu_opt_set(), rename qemu_opt_set_err(), fix use Markus Armbruster @ 2015-02-26 14:07 ` Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 08/13] QemuOpts: Propagate errors through opts_parse() Markus Armbruster ` (6 subsequent siblings) 13 siblings, 0 replies; 15+ messages in thread From: Markus Armbruster @ 2015-02-26 14:07 UTC (permalink / raw) To: qemu-devel Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- util/qemu-option.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/util/qemu-option.c b/util/qemu-option.c index 928b86f..8a0f69e 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -750,8 +750,8 @@ void qemu_opts_print(QemuOpts *opts, const char *sep) } } -static int opts_do_parse(QemuOpts *opts, const char *params, - const char *firstname, bool prepend) +static void opts_do_parse(QemuOpts *opts, const char *params, + const char *firstname, bool prepend, Error **errp) { char option[128], value[1024]; const char *p,*pe,*pc; @@ -789,21 +789,27 @@ static int opts_do_parse(QemuOpts *opts, const char *params, /* store and parse */ opt_set(opts, option, value, prepend, &local_err); if (local_err) { - qerror_report_err(local_err); - error_free(local_err); - return -1; + error_propagate(errp, local_err); + return; } } if (*p != ',') { break; } } - return 0; } int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname) { - return opts_do_parse(opts, params, firstname, false); + Error *err = NULL; + + opts_do_parse(opts, params, firstname, false, &err); + if (err) { + qerror_report_err(err); + error_free(err); + return -1; + } + return 0; } static QemuOpts *opts_parse(QemuOptsList *list, const char *params, @@ -843,7 +849,10 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params, return NULL; } - if (opts_do_parse(opts, params, firstname, defaults) != 0) { + opts_do_parse(opts, params, firstname, defaults, &local_err); + if (local_err) { + qerror_report_err(local_err); + error_free(local_err); qemu_opts_del(opts); return NULL; } -- 1.9.3 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PULL 08/13] QemuOpts: Propagate errors through opts_parse() 2015-02-26 14:07 [Qemu-devel] [PULL 00/13] QemuOpts: Convert various setters to Error Markus Armbruster ` (6 preceding siblings ...) 2015-02-26 14:07 ` [Qemu-devel] [PULL 07/13] QemuOpts: Propagate errors through opts_do_parse() Markus Armbruster @ 2015-02-26 14:07 ` Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 09/13] qemu-img: Suppress unhelpful extra errors in convert, amend Markus Armbruster ` (5 subsequent siblings) 13 siblings, 0 replies; 15+ messages in thread From: Markus Armbruster @ 2015-02-26 14:07 UTC (permalink / raw) To: qemu-devel Since I'm touching qemu_opts_parse() anyway, write a function comment for it. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- util/qemu-option.c | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/util/qemu-option.c b/util/qemu-option.c index 8a0f69e..95f242e 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -813,7 +813,7 @@ int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname } static QemuOpts *opts_parse(QemuOptsList *list, const char *params, - int permit_abbrev, bool defaults) + int permit_abbrev, bool defaults, Error **errp) { const char *firstname; char value[1024], *id = NULL; @@ -842,17 +842,13 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params, assert(!defaults || list->merge_lists); opts = qemu_opts_create(list, id, !defaults, &local_err); if (opts == NULL) { - if (local_err) { - qerror_report_err(local_err); - error_free(local_err); - } + error_propagate(errp, local_err); return NULL; } opts_do_parse(opts, params, firstname, defaults, &local_err); if (local_err) { - qerror_report_err(local_err); - error_free(local_err); + error_propagate(errp, local_err); qemu_opts_del(opts); return NULL; } @@ -860,10 +856,25 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params, return opts; } +/** + * Create a QemuOpts in @list and with options parsed from @params. + * If @permit_abbrev, the first key=value in @params may omit key=, + * and is treated as if key was @list->implied_opt_name. + * Report errors with qerror_report_err(). + * Return the new QemuOpts on success, null pointer on error. + */ QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, int permit_abbrev) { - return opts_parse(list, params, permit_abbrev, false); + Error *err = NULL; + QemuOpts *opts; + + opts = opts_parse(list, params, permit_abbrev, false, &err); + if (!opts) { + qerror_report_err(err); + error_free(err); + } + return opts; } void qemu_opts_set_defaults(QemuOptsList *list, const char *params, @@ -871,7 +882,7 @@ void qemu_opts_set_defaults(QemuOptsList *list, const char *params, { QemuOpts *opts; - opts = opts_parse(list, params, permit_abbrev, true); + opts = opts_parse(list, params, permit_abbrev, true, NULL); assert(opts); } -- 1.9.3 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PULL 09/13] qemu-img: Suppress unhelpful extra errors in convert, amend 2015-02-26 14:07 [Qemu-devel] [PULL 00/13] QemuOpts: Convert various setters to Error Markus Armbruster ` (7 preceding siblings ...) 2015-02-26 14:07 ` [Qemu-devel] [PULL 08/13] QemuOpts: Propagate errors through opts_parse() Markus Armbruster @ 2015-02-26 14:07 ` Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 10/13] block: Simplify setting numeric options Markus Armbruster ` (4 subsequent siblings) 13 siblings, 0 replies; 15+ messages in thread From: Markus Armbruster @ 2015-02-26 14:07 UTC (permalink / raw) To: qemu-devel img_convert() and img_amend() use qemu_opts_do_parse(), which reports errors with qerror_report_err(). Its error messages aren't helpful here, the caller reports one that actually makes sense. Reproducer: $ qemu-img convert -o backing_format=raw in.img out.img qemu-img: Invalid parameter 'backing_format' qemu-img: Invalid options for file format 'raw' To fix, propagate errors through qemu_opts_do_parse(). This lifts the error reporting into callers. Drop it from img_convert() and img_amend(), keep it in qemu_chr_parse_compat(), bdrv_img_create(). Since I'm touching qemu_opts_do_parse() anyway, write a function comment for it. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- block.c | 5 ++++- include/qemu/option.h | 3 ++- qemu-char.c | 10 ++++++++-- qemu-img.c | 25 +++++++++++++++++-------- util/qemu-option.c | 19 +++++++++---------- 5 files changed, 40 insertions(+), 22 deletions(-) diff --git a/block.c b/block.c index ebbb515..28ea19a 100644 --- a/block.c +++ b/block.c @@ -5653,7 +5653,10 @@ void bdrv_img_create(const char *filename, const char *fmt, /* Parse -o options */ if (options) { - if (qemu_opts_do_parse(opts, options, NULL) != 0) { + qemu_opts_do_parse(opts, options, NULL, &local_err); + if (local_err) { + error_report_err(local_err); + local_err = NULL; error_setg(errp, "Invalid options for file format '%s'", fmt); goto out; } diff --git a/include/qemu/option.h b/include/qemu/option.h index a41ee92..f88b545 100644 --- a/include/qemu/option.h +++ b/include/qemu/option.h @@ -115,7 +115,8 @@ const char *qemu_opts_id(QemuOpts *opts); void qemu_opts_set_id(QemuOpts *opts, char *id); void qemu_opts_del(QemuOpts *opts); void qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc, Error **errp); -int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname); +void qemu_opts_do_parse(QemuOpts *opts, const char *params, + const char *firstname, Error **errp); QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, int permit_abbrev); void qemu_opts_set_defaults(QemuOptsList *list, const char *params, int permit_abbrev); diff --git a/qemu-char.c b/qemu-char.c index 3ed46ee..a405d76 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -3379,8 +3379,11 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename) qemu_opt_set(opts, "host", host, &error_abort); qemu_opt_set(opts, "port", port, &error_abort); if (p[pos] == ',') { - if (qemu_opts_do_parse(opts, p+pos+1, NULL) != 0) + qemu_opts_do_parse(opts, p+pos+1, NULL, &local_err); + if (local_err) { + error_report_err(local_err); goto fail; + } } if (strstart(filename, "telnet:", &p)) qemu_opt_set(opts, "telnet", "on", &error_abort); @@ -3411,8 +3414,11 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename) } if (strstart(filename, "unix:", &p)) { qemu_opt_set(opts, "backend", "socket", &error_abort); - if (qemu_opts_do_parse(opts, p, "path") != 0) + qemu_opts_do_parse(opts, p, "path", &local_err); + if (local_err) { + error_report_err(local_err); goto fail; + } return opts; } if (strstart(filename, "/dev/parport", NULL) || diff --git a/qemu-img.c b/qemu-img.c index 54e34bd..7ac7f56 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1550,10 +1550,14 @@ static int img_convert(int argc, char **argv) create_opts = qemu_opts_append(create_opts, proto_drv->create_opts); opts = qemu_opts_create(create_opts, NULL, 0, &error_abort); - if (options && qemu_opts_do_parse(opts, options, NULL)) { - error_report("Invalid options for file format '%s'", out_fmt); - ret = -1; - goto out; + if (options) { + qemu_opts_do_parse(opts, options, NULL, &local_err); + if (local_err) { + error_report("Invalid options for file format '%s'", out_fmt); + error_free(local_err); + ret = -1; + goto out; + } } qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_sectors * 512, @@ -2887,6 +2891,7 @@ static void amend_status_cb(BlockDriverState *bs, static int img_amend(int argc, char **argv) { + Error *err = NULL; int c, ret = 0; char *options = NULL; QemuOptsList *create_opts = NULL; @@ -2992,10 +2997,14 @@ static int img_amend(int argc, char **argv) create_opts = qemu_opts_append(create_opts, bs->drv->create_opts); opts = qemu_opts_create(create_opts, NULL, 0, &error_abort); - if (options && qemu_opts_do_parse(opts, options, NULL)) { - error_report("Invalid options for file format '%s'", fmt); - ret = -1; - goto out; + if (options) { + qemu_opts_do_parse(opts, options, NULL, &err); + if (err) { + error_report("Invalid options for file format '%s'", fmt); + error_free(err); + ret = -1; + goto out; + } } /* In case the driver does not call amend_status_cb() */ diff --git a/util/qemu-option.c b/util/qemu-option.c index 95f242e..fda4e5f 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -799,17 +799,16 @@ static void opts_do_parse(QemuOpts *opts, const char *params, } } -int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname) +/** + * Store options parsed from @params into @opts. + * If @firstname is non-null, the first key=value in @params may omit + * key=, and is treated as if key was @firstname. + * On error, store an error object through @errp if non-null. + */ +void qemu_opts_do_parse(QemuOpts *opts, const char *params, + const char *firstname, Error **errp) { - Error *err = NULL; - - opts_do_parse(opts, params, firstname, false, &err); - if (err) { - qerror_report_err(err); - error_free(err); - return -1; - } - return 0; + opts_do_parse(opts, params, firstname, false, errp); } static QemuOpts *opts_parse(QemuOptsList *list, const char *params, -- 1.9.3 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PULL 10/13] block: Simplify setting numeric options 2015-02-26 14:07 [Qemu-devel] [PULL 00/13] QemuOpts: Convert various setters to Error Markus Armbruster ` (8 preceding siblings ...) 2015-02-26 14:07 ` [Qemu-devel] [PULL 09/13] qemu-img: Suppress unhelpful extra errors in convert, amend Markus Armbruster @ 2015-02-26 14:07 ` Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 11/13] qemu-sockets: Simplify setting numeric and boolean options Markus Armbruster ` (3 subsequent siblings) 13 siblings, 0 replies; 15+ messages in thread From: Markus Armbruster @ 2015-02-26 14:07 UTC (permalink / raw) To: qemu-devel Don't convert numbers to strings for use with qemu_opt_set(), simply use qemu_opt_set_number() instead. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- blockdev.c | 4 +--- vl.c | 13 ++++++------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/blockdev.c b/blockdev.c index 5aa23b5..b9c1c0c 100644 --- a/blockdev.c +++ b/blockdev.c @@ -180,7 +180,6 @@ QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file, const char *optstr) { QemuOpts *opts; - char buf[32]; opts = drive_def(optstr); if (!opts) { @@ -190,8 +189,7 @@ QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file, qemu_opt_set(opts, "if", if_name[type], &error_abort); } if (index >= 0) { - snprintf(buf, sizeof(buf), "%d", index); - qemu_opt_set(opts, "index", buf, &error_abort); + qemu_opt_set_number(opts, "index", index, &error_abort); } if (file) qemu_opt_set(opts, "file", file, &error_abort); diff --git a/vl.c b/vl.c index 66b0ad8..e2c1950 100644 --- a/vl.c +++ b/vl.c @@ -2971,13 +2971,12 @@ int main(int argc, char **argv, char **envp) exit(1); } if (hda_opts != NULL) { - char num[16]; - snprintf(num, sizeof(num), "%d", cyls); - qemu_opt_set(hda_opts, "cyls", num, &error_abort); - snprintf(num, sizeof(num), "%d", heads); - qemu_opt_set(hda_opts, "heads", num, &error_abort); - snprintf(num, sizeof(num), "%d", secs); - qemu_opt_set(hda_opts, "secs", num, &error_abort); + qemu_opt_set_number(hda_opts, "cyls", cyls, + &error_abort); + qemu_opt_set_number(hda_opts, "heads", heads, + &error_abort); + qemu_opt_set_number(hda_opts, "secs", secs, + &error_abort); if (translation == BIOS_ATA_TRANSLATION_LARGE) { qemu_opt_set(hda_opts, "trans", "large", &error_abort); -- 1.9.3 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PULL 11/13] qemu-sockets: Simplify setting numeric and boolean options 2015-02-26 14:07 [Qemu-devel] [PULL 00/13] QemuOpts: Convert various setters to Error Markus Armbruster ` (9 preceding siblings ...) 2015-02-26 14:07 ` [Qemu-devel] [PULL 10/13] block: Simplify setting numeric options Markus Armbruster @ 2015-02-26 14:07 ` Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 12/13] pc: Use qemu_opt_set() instead of qemu_opts_parse() Markus Armbruster ` (2 subsequent siblings) 13 siblings, 0 replies; 15+ messages in thread From: Markus Armbruster @ 2015-02-26 14:07 UTC (permalink / raw) To: qemu-devel Don't convert numbers or bools to strings for use with qemu_opt_set(), simply use qemu_opt_set_number() or qemu_opt_set_bool() instead. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- util/qemu-sockets.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c index e908523..87c9bc6 100644 --- a/util/qemu-sockets.c +++ b/util/qemu-sockets.c @@ -199,13 +199,13 @@ listen: freeaddrinfo(res); return -1; } - snprintf(uport, sizeof(uport), "%d", inet_getport(e) - port_offset); qemu_opt_set(opts, "host", uaddr, &error_abort); - qemu_opt_set(opts, "port", uport, &error_abort); - qemu_opt_set(opts, "ipv6", (e->ai_family == PF_INET6) ? "on" : "off", - &error_abort); - qemu_opt_set(opts, "ipv4", (e->ai_family != PF_INET6) ? "on" : "off", - &error_abort); + qemu_opt_set_number(opts, "port", inet_getport(e) - port_offset, + &error_abort); + qemu_opt_set_bool(opts, "ipv6", e->ai_family == PF_INET6, + &error_abort); + qemu_opt_set_bool(opts, "ipv4", e->ai_family != PF_INET6, + &error_abort); freeaddrinfo(res); return slisten; } @@ -586,9 +586,7 @@ static void inet_addr_to_opts(QemuOpts *opts, const InetSocketAddress *addr) qemu_opt_set_bool(opts, "ipv6", ipv6, &error_abort); } if (addr->has_to) { - char to[20]; - snprintf(to, sizeof(to), "%d", addr->to); - qemu_opt_set(opts, "to", to, &error_abort); + qemu_opt_set_number(opts, "to", addr->to, &error_abort); } qemu_opt_set(opts, "host", addr->host, &error_abort); qemu_opt_set(opts, "port", addr->port, &error_abort); -- 1.9.3 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PULL 12/13] pc: Use qemu_opt_set() instead of qemu_opts_parse() 2015-02-26 14:07 [Qemu-devel] [PULL 00/13] QemuOpts: Convert various setters to Error Markus Armbruster ` (10 preceding siblings ...) 2015-02-26 14:07 ` [Qemu-devel] [PULL 11/13] qemu-sockets: Simplify setting numeric and boolean options Markus Armbruster @ 2015-02-26 14:07 ` Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 13/13] qtest: " Markus Armbruster 2015-03-03 10:03 ` [Qemu-devel] [PULL 00/13] QemuOpts: Convert various setters to Error Peter Maydell 13 siblings, 0 replies; 15+ messages in thread From: Markus Armbruster @ 2015-02-26 14:07 UTC (permalink / raw) To: qemu-devel Less code, same result. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- hw/i386/pc.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 05008cb..51b4cf5 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1137,15 +1137,11 @@ void pc_acpi_init(const char *default_dsdt) if (filename == NULL) { fprintf(stderr, "WARNING: failed to find %s\n", default_dsdt); } else { - char *arg; - QemuOpts *opts; + QemuOpts *opts = qemu_opts_create(qemu_find_opts("acpi"), NULL, 0, + &error_abort); Error *err = NULL; - arg = g_strdup_printf("file=%s", filename); - - /* creates a deep copy of "arg" */ - opts = qemu_opts_parse(qemu_find_opts("acpi"), arg, 0); - g_assert(opts != NULL); + qemu_opt_set(opts, "file", filename, &error_abort); acpi_table_add_builtin(opts, &err); if (err) { @@ -1153,7 +1149,6 @@ void pc_acpi_init(const char *default_dsdt) error_get_pretty(err)); error_free(err); } - g_free(arg); g_free(filename); } } -- 1.9.3 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PULL 13/13] qtest: Use qemu_opt_set() instead of qemu_opts_parse() 2015-02-26 14:07 [Qemu-devel] [PULL 00/13] QemuOpts: Convert various setters to Error Markus Armbruster ` (11 preceding siblings ...) 2015-02-26 14:07 ` [Qemu-devel] [PULL 12/13] pc: Use qemu_opt_set() instead of qemu_opts_parse() Markus Armbruster @ 2015-02-26 14:07 ` Markus Armbruster 2015-03-03 10:03 ` [Qemu-devel] [PULL 00/13] QemuOpts: Convert various setters to Error Peter Maydell 13 siblings, 0 replies; 15+ messages in thread From: Markus Armbruster @ 2015-02-26 14:07 UTC (permalink / raw) To: qemu-devel Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- qtest.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/qtest.c b/qtest.c index 2bca04e..8d1e66c 100644 --- a/qtest.c +++ b/qtest.c @@ -520,16 +520,13 @@ static void qtest_event(void *opaque, int event) } } -static void configure_qtest_icount(const char *options) +static int qtest_init_accel(MachineState *ms) { - QemuOpts *opts = qemu_opts_parse(qemu_find_opts("icount"), options, 1); + QemuOpts *opts = qemu_opts_create(qemu_find_opts("icount"), NULL, 0, + &error_abort); + qemu_opt_set(opts, "shift", "0", &error_abort); configure_icount(opts, &error_abort); qemu_opts_del(opts); -} - -static int qtest_init_accel(MachineState *ms) -{ - configure_qtest_icount("0"); return 0; } -- 1.9.3 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PULL 00/13] QemuOpts: Convert various setters to Error 2015-02-26 14:07 [Qemu-devel] [PULL 00/13] QemuOpts: Convert various setters to Error Markus Armbruster ` (12 preceding siblings ...) 2015-02-26 14:07 ` [Qemu-devel] [PULL 13/13] qtest: " Markus Armbruster @ 2015-03-03 10:03 ` Peter Maydell 13 siblings, 0 replies; 15+ messages in thread From: Peter Maydell @ 2015-03-03 10:03 UTC (permalink / raw) To: Markus Armbruster; +Cc: QEMU Developers On 26 February 2015 at 23:07, Markus Armbruster <armbru@redhat.com> wrote: > Bonus: improve qemu-img error reporting slightly. > > Changes since v1: > * Straightforward rebase > * Whitespace fix in PATCH 6 [Eric] > * Comment polished in PATCH 9 [Eric] > Because they're trivial, I'm requesting a pull without another round > of review. > > The following changes since commit c5c6d7f81a6950d8e32a3b5a0bafd37bfa5a8e88: > > Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2015-02-18' into staging (2015-02-26 07:01:08 +0000) > > are available in the git repository at: > > > git://repo.or.cz/qemu/armbru.git tags/pull-error-2015-02-26 > > for you to fetch changes up to b3adf5acb57dee14a74e57ab4f16cd1a83e5a7d2: > > qtest: Use qemu_opt_set() instead of qemu_opts_parse() (2015-02-26 14:52:13 +0100) > > ---------------------------------------------------------------- > QemuOpts: Convert various setters to Error > > ---------------------------------------------------------------- Applied, thanks. -- PMM ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2015-03-03 10:03 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-02-26 14:07 [Qemu-devel] [PULL 00/13] QemuOpts: Convert various setters to Error Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 01/13] QemuOpts: Convert qemu_opt_set_bool() to Error, fix its use Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 02/13] QemuOpts: Convert qemu_opt_set_number() " Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 03/13] QemuOpts: Convert qemu_opts_set() " Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 04/13] qemu-img: Suppress unhelpful extra errors in convert, resize Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 05/13] block: Suppress unhelpful extra errors in bdrv_img_create() Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 06/13] QemuOpts: Drop qemu_opt_set(), rename qemu_opt_set_err(), fix use Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 07/13] QemuOpts: Propagate errors through opts_do_parse() Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 08/13] QemuOpts: Propagate errors through opts_parse() Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 09/13] qemu-img: Suppress unhelpful extra errors in convert, amend Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 10/13] block: Simplify setting numeric options Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 11/13] qemu-sockets: Simplify setting numeric and boolean options Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 12/13] pc: Use qemu_opt_set() instead of qemu_opts_parse() Markus Armbruster 2015-02-26 14:07 ` [Qemu-devel] [PULL 13/13] qtest: " Markus Armbruster 2015-03-03 10:03 ` [Qemu-devel] [PULL 00/13] QemuOpts: Convert various setters to Error Peter Maydell
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).