[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH v1 4/5] qemu-option: Remove qemu_opts_create
|
From: |
Peter Crosthwaite |
|
Subject: |
Re: [Qemu-devel] [RFC PATCH v1 4/5] qemu-option: Remove qemu_opts_create_nofail |
|
Date: |
Tue, 3 Dec 2013 20:17:43 +1000 |
On Tue, Dec 3, 2013 at 7:42 PM, Markus Armbruster <address@hidden> wrote:
> Peter Crosthwaite <address@hidden> writes:
>
>> This is a boiler-plate _nofail variant of qemu_opts_create. Remove and
>> use error_abort in call sites.
>>
>> A null argument needs to be added for the id field in affected callsites
>> due to inconsistency between the normal and no_fail variants.
>
> Two arguments, actually, NULL id and zero fail_if_exists:
>
Yep. I was assuming fail_if_exists was implicit as part of no-fail so
I wasn't considering that an inconsistency. But as _nofail doesn't
actually fail_if_exist's then its just as inconsistent. Will fix.
> QemuOpts *qemu_opts_create_nofail(QemuOptsList *list)
> {
> QemuOpts *opts;
> Error *errp = NULL;
> opts = qemu_opts_create(list, NULL, 0, &errp);
> assert_no_error(errp);
> return opts;
> }
>
>> Signed-off-by: Peter Crosthwaite <address@hidden>
>> ---
>>
>> block/blkdebug.c | 2 +-
>> block/blkverify.c | 2 +-
>> block/curl.c | 2 +-
>> block/gluster.c | 2 +-
>> block/iscsi.c | 2 +-
>> block/nbd.c | 2 +-
>> block/qcow2.c | 2 +-
>> block/raw-posix.c | 2 +-
>> block/raw-win32.c | 4 ++--
>> block/rbd.c | 2 +-
>> block/sheepdog.c | 2 +-
>> block/vvfat.c | 2 +-
>> blockdev.c | 6 ++++--
>> hw/watchdog/watchdog.c | 3 ++-
>> include/qemu/option.h | 1 -
>> qdev-monitor.c | 2 +-
>> qemu-img.c | 2 +-
>> util/qemu-config.c | 2 +-
>> util/qemu-option.c | 9 ---------
>> util/qemu-sockets.c | 18 +++++++++---------
>> vl.c | 17 ++++++++++-------
>> 21 files changed, 41 insertions(+), 45 deletions(-)
>>
>> diff --git a/block/blkdebug.c b/block/blkdebug.c
>> index 16d2b91..6b49df8 100644
>> --- a/block/blkdebug.c
>> +++ b/block/blkdebug.c
>> @@ -359,7 +359,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict
>> *options, int flags,
>> const char *filename, *config;
>> int ret;
>>
>> - opts = qemu_opts_create_nofail(&runtime_opts);
>> + opts = qemu_opts_create(&runtime_opts, NULL, 1, &error_abort);
>> qemu_opts_absorb_qdict(opts, options, &local_err);
>> if (error_is_set(&local_err)) {
>> error_propagate(errp, local_err);
>
> Err, doesn't this change fail_if_exists from zero to one? Same
> everywhere.
>
Yes. Rash assumption my by me that the nofail version fails_on_exist. Will fix.
>> diff --git a/block/blkverify.c b/block/blkverify.c
>> index 3c63528..d901852 100644
>> --- a/block/blkverify.c
>> +++ b/block/blkverify.c
>> @@ -125,7 +125,7 @@ static int blkverify_open(BlockDriverState *bs, QDict
>> *options, int flags,
>> const char *filename, *raw;
>> int ret;
>>
>> - opts = qemu_opts_create_nofail(&runtime_opts);
>> + opts = qemu_opts_create(&runtime_opts, NULL, 1, &error_abort);
>> qemu_opts_absorb_qdict(opts, options, &local_err);
>> if (error_is_set(&local_err)) {
>> error_propagate(errp, local_err);
>> diff --git a/block/curl.c b/block/curl.c
>> index 5a46f97..25dd433 100644
>> --- a/block/curl.c
>> +++ b/block/curl.c
>> @@ -413,7 +413,7 @@ static int curl_open(BlockDriverState *bs, QDict
>> *options, int flags,
>> return -EROFS;
>> }
>>
>> - opts = qemu_opts_create_nofail(&runtime_opts);
>> + opts = qemu_opts_create(&runtime_opts, NULL, 1, &error_abort);
>> qemu_opts_absorb_qdict(opts, options, &local_err);
>> if (error_is_set(&local_err)) {
>> qerror_report_err(local_err);
>> diff --git a/block/gluster.c b/block/gluster.c
>> index 877686a..38c1a72 100644
>> --- a/block/gluster.c
>> +++ b/block/gluster.c
>> @@ -298,7 +298,7 @@ static int qemu_gluster_open(BlockDriverState *bs,
>> QDict *options,
>> Error *local_err = NULL;
>> const char *filename;
>>
>> - opts = qemu_opts_create_nofail(&runtime_opts);
>> + opts = qemu_opts_create(&runtime_opts, NULL, 1, &error_abort);
>> qemu_opts_absorb_qdict(opts, options, &local_err);
>> if (error_is_set(&local_err)) {
>> qerror_report_err(local_err);
>> diff --git a/block/iscsi.c b/block/iscsi.c
>> index a2d578c..b5a8221 100644
>> --- a/block/iscsi.c
>> +++ b/block/iscsi.c
>> @@ -1241,7 +1241,7 @@ static int iscsi_open(BlockDriverState *bs, QDict
>> *options, int flags,
>> return -EINVAL;
>> }
>>
>> - opts = qemu_opts_create_nofail(&runtime_opts);
>> + opts = qemu_opts_create(&runtime_opts, NULL, 1, &error_abort);
>> qemu_opts_absorb_qdict(opts, options, &local_err);
>> if (error_is_set(&local_err)) {
>> qerror_report_err(local_err);
>> diff --git a/block/nbd.c b/block/nbd.c
>> index c8deeee..b281b9c 100644
>> --- a/block/nbd.c
>> +++ b/block/nbd.c
>> @@ -234,7 +234,7 @@ static int nbd_config(BDRVNBDState *s, QDict *options)
>> return -EINVAL;
>> }
>>
>> - s->socket_opts = qemu_opts_create_nofail(&socket_optslist);
>> + s->socket_opts = qemu_opts_create(&socket_optslist, NULL, 1,
>> &error_abort);
>
> Long line, please wrap.
>
>>
>> qemu_opts_absorb_qdict(s->socket_opts, options, &local_err);
>> if (error_is_set(&local_err)) {
>> diff --git a/block/qcow2.c b/block/qcow2.c
>> index 6e5d98d..ac63e06 100644
>> --- a/block/qcow2.c
>> +++ b/block/qcow2.c
>> @@ -669,7 +669,7 @@ static int qcow2_open(BlockDriverState *bs, QDict
>> *options, int flags,
>> }
>>
>> /* Enable lazy_refcounts according to image and command line options */
>> - opts = qemu_opts_create_nofail(&qcow2_runtime_opts);
>> + opts = qemu_opts_create(&qcow2_runtime_opts, NULL, 1, &error_abort);
>> qemu_opts_absorb_qdict(opts, options, &local_err);
>> if (error_is_set(&local_err)) {
>> error_propagate(errp, local_err);
>> diff --git a/block/raw-posix.c b/block/raw-posix.c
>> index f836c8e..996ec34 100644
>> --- a/block/raw-posix.c
>> +++ b/block/raw-posix.c
>> @@ -284,7 +284,7 @@ static int raw_open_common(BlockDriverState *bs, QDict
>> *options,
>> const char *filename;
>> int fd, ret;
>>
>> - opts = qemu_opts_create_nofail(&raw_runtime_opts);
>> + opts = qemu_opts_create(&raw_runtime_opts, NULL, 1, &error_abort);
>> qemu_opts_absorb_qdict(opts, options, &local_err);
>> if (error_is_set(&local_err)) {
>> error_propagate(errp, local_err);
>> diff --git a/block/raw-win32.c b/block/raw-win32.c
>> index 2bad5a3..5881836 100644
>> --- a/block/raw-win32.c
>> +++ b/block/raw-win32.c
>> @@ -248,7 +248,7 @@ static int raw_open(BlockDriverState *bs, QDict
>> *options, int flags,
>>
>> s->type = FTYPE_FILE;
>>
>> - opts = qemu_opts_create_nofail(&raw_runtime_opts);
>> + opts = qemu_opts_create(&raw_runtime_opts, NULL, 1, &error_abort);
>> qemu_opts_absorb_qdict(opts, options, &local_err);
>> if (error_is_set(&local_err)) {
>> error_propagate(errp, local_err);
>> @@ -550,7 +550,7 @@ static int hdev_open(BlockDriverState *bs, QDict
>> *options, int flags,
>> Error *local_err = NULL;
>> const char *filename;
>>
>> - QemuOpts *opts = qemu_opts_create_nofail(&raw_runtime_opts);
>> + QemuOpts *opts = qemu_opts_create(&raw_runtime_opts, NULL, 1,
>> &error_abort);
>
> Long line, please wrap.
>
hmm my checkpatch script must have issues. Sry.
>> qemu_opts_absorb_qdict(opts, options, &local_err);
>> if (error_is_set(&local_err)) {
>> error_propagate(errp, local_err);
>> diff --git a/block/rbd.c b/block/rbd.c
>> index 4a1ea5b..e7250dd 100644
>> --- a/block/rbd.c
>> +++ b/block/rbd.c
>> @@ -461,7 +461,7 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict
>> *options, int flags,
>> const char *filename;
>> int r;
>>
>> - opts = qemu_opts_create_nofail(&runtime_opts);
>> + opts = qemu_opts_create(&runtime_opts, NULL, 1, &error_abort);
>> qemu_opts_absorb_qdict(opts, options, &local_err);
>> if (error_is_set(&local_err)) {
>> qerror_report_err(local_err);
>> diff --git a/block/sheepdog.c b/block/sheepdog.c
>> index ef387de..12f0b3f 100644
>> --- a/block/sheepdog.c
>> +++ b/block/sheepdog.c
>> @@ -1375,7 +1375,7 @@ static int sd_open(BlockDriverState *bs, QDict
>> *options, int flags,
>>
>> s->bs = bs;
>>
>> - opts = qemu_opts_create_nofail(&runtime_opts);
>> + opts = qemu_opts_create(&runtime_opts, NULL, 1, &error_abort);
>> qemu_opts_absorb_qdict(opts, options, &local_err);
>> if (error_is_set(&local_err)) {
>> qerror_report_err(local_err);
>> diff --git a/block/vvfat.c b/block/vvfat.c
>> index 3ddaa0b..57bc0f9 100644
>> --- a/block/vvfat.c
>> +++ b/block/vvfat.c
>> @@ -1085,7 +1085,7 @@ DLOG(if (stderr == NULL) {
>> setbuf(stderr, NULL);
>> })
>>
>> - opts = qemu_opts_create_nofail(&runtime_opts);
>> + opts = qemu_opts_create(&runtime_opts, NULL, 1, &error_abort);
>> qemu_opts_absorb_qdict(opts, options, &local_err);
>> if (error_is_set(&local_err)) {
>> qerror_report_err(local_err);
>> diff --git a/blockdev.c b/blockdev.c
>> index 330aa4a..78e8455 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -682,7 +682,8 @@ DriveInfo *drive_init(QemuOpts *all_opts,
>> BlockInterfaceType block_default_type)
>> bs_opts = qdict_new();
>> qemu_opts_to_qdict(all_opts, bs_opts);
>>
>> - legacy_opts = qemu_opts_create_nofail(&qemu_legacy_drive_opts);
>> + legacy_opts = qemu_opts_create(&qemu_legacy_drive_opts, NULL, 1,
>> + &error_abort);
>> qemu_opts_absorb_qdict(legacy_opts, bs_opts, &local_err);
>> if (error_is_set(&local_err)) {
>> qerror_report_err(local_err);
>> @@ -853,7 +854,8 @@ DriveInfo *drive_init(QemuOpts *all_opts,
>> BlockInterfaceType block_default_type)
>>
>> if (type == IF_VIRTIO) {
>> QemuOpts *devopts;
>> - devopts = qemu_opts_create_nofail(qemu_find_opts("device"));
>> + devopts = qemu_opts_create(qemu_find_opts("device"), NULL, 1,
>> + &error_abort);
>> if (arch_type == QEMU_ARCH_S390X) {
>> qemu_opt_set(devopts, "driver", "virtio-blk-s390");
>> } else {
>> diff --git a/hw/watchdog/watchdog.c b/hw/watchdog/watchdog.c
>> index 387962e..0ab2a0e 100644
>> --- a/hw/watchdog/watchdog.c
>> +++ b/hw/watchdog/watchdog.c
>> @@ -66,7 +66,8 @@ int select_watchdog(const char *p)
>> QLIST_FOREACH(model, &watchdog_list, entry) {
>> if (strcasecmp(model->wdt_name, p) == 0) {
>> /* add the device */
>> - opts = qemu_opts_create_nofail(qemu_find_opts("device"));
>> + opts = qemu_opts_create(qemu_find_opts("device"), NULL, 1,
>> + &error_abort);
>> qemu_opt_set(opts, "driver", p);
>> return 0;
>> }
>> diff --git a/include/qemu/option.h b/include/qemu/option.h
>> index 5c0c6dd..3ea871a 100644
>> --- a/include/qemu/option.h
>> +++ b/include/qemu/option.h
>> @@ -136,7 +136,6 @@ int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc
>> func, void *opaque,
>> QemuOpts *qemu_opts_find(QemuOptsList *list, const char *id);
>> QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id,
>> int fail_if_exists, Error **errp);
>> -QemuOpts *qemu_opts_create_nofail(QemuOptsList *list);
>> void qemu_opts_reset(QemuOptsList *list);
>> void qemu_opts_loc_restore(QemuOpts *opts);
>> int qemu_opts_set(QemuOptsList *list, const char *id,
>> diff --git a/qdev-monitor.c b/qdev-monitor.c
>> index dc37a43..dd6e68c 100644
>> --- a/qdev-monitor.c
>> +++ b/qdev-monitor.c
>> @@ -730,7 +730,7 @@ int qemu_global_option(const char *str)
>> return -1;
>> }
>>
>> - opts = qemu_opts_create_nofail(&qemu_global_opts);
>> + opts = qemu_opts_create(&qemu_global_opts, NULL, 1, &error_abort);
>> qemu_opt_set(opts, "driver", driver);
>> qemu_opt_set(opts, "property", property);
>> qemu_opt_set(opts, "value", str+offset+1);
>> diff --git a/qemu-img.c b/qemu-img.c
>> index b6b5644..4f6b519 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -2477,7 +2477,7 @@ static int img_resize(int argc, char **argv)
>> }
>>
>> /* Parse size */
>> - param = qemu_opts_create_nofail(&resize_options);
>> + param = qemu_opts_create(&resize_options, NULL, 1, &error_abort);
>> if (qemu_opt_set(param, BLOCK_OPT_SIZE, size)) {
>> /* Error message already printed when size parsing fails */
>> ret = -1;
>> diff --git a/util/qemu-config.c b/util/qemu-config.c
>> index 04da942..a803922 100644
>> --- a/util/qemu-config.c
>> +++ b/util/qemu-config.c
>> @@ -311,7 +311,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists,
>> const char *fname)
>> error_free(local_err);
>> goto out;
>> }
>> - opts = qemu_opts_create_nofail(list);
>> + opts = qemu_opts_create(list, NULL, 1, &error_abort);
>> continue;
>> }
>> if (sscanf(line, " %63s = \"%1023[^\"]\"", arg, value) == 2) {
>> diff --git a/util/qemu-option.c b/util/qemu-option.c
>> index efcb5dc..668e5d9 100644
>> --- a/util/qemu-option.c
>> +++ b/util/qemu-option.c
>> @@ -791,15 +791,6 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const
>> char *id,
>> return opts;
>> }
>>
>> -QemuOpts *qemu_opts_create_nofail(QemuOptsList *list)
>> -{
>> - QemuOpts *opts;
>> - Error *errp = NULL;
>> - opts = qemu_opts_create(list, NULL, 0, &errp);
>> - assert_no_error(errp);
>> - return opts;
>> -}
>> -
>> void qemu_opts_reset(QemuOptsList *list)
>> {
>> QemuOpts *opts, *next_opts;
>> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
>> index 6b97dc1..9d50f43 100644
>> --- a/util/qemu-sockets.c
>> +++ b/util/qemu-sockets.c
>> @@ -578,7 +578,7 @@ int inet_listen(const char *str, char *ostr, int olen,
>>
>> addr = inet_parse(str, errp);
>> if (addr != NULL) {
>> - opts = qemu_opts_create_nofail(&socket_optslist);
>> + opts = qemu_opts_create(&socket_optslist, NULL, 1, &error_abort);
>> inet_addr_to_opts(opts, addr);
>> qapi_free_InetSocketAddress(addr);
>> sock = inet_listen_opts(opts, port_offset, errp);
>> @@ -617,7 +617,7 @@ int inet_connect(const char *str, Error **errp)
>>
>> addr = inet_parse(str, errp);
>> if (addr != NULL) {
>> - opts = qemu_opts_create_nofail(&socket_optslist);
>> + opts = qemu_opts_create(&socket_optslist, NULL, 1, &error_abort);
>> inet_addr_to_opts(opts, addr);
>> qapi_free_InetSocketAddress(addr);
>> sock = inet_connect_opts(opts, errp, NULL, NULL);
>> @@ -651,7 +651,7 @@ int inet_nonblocking_connect(const char *str,
>>
>> addr = inet_parse(str, errp);
>> if (addr != NULL) {
>> - opts = qemu_opts_create_nofail(&socket_optslist);
>> + opts = qemu_opts_create(&socket_optslist, NULL, 1, &error_abort);
>> inet_addr_to_opts(opts, addr);
>> qapi_free_InetSocketAddress(addr);
>> sock = inet_connect_opts(opts, errp, callback, opaque);
>> @@ -794,7 +794,7 @@ int unix_listen(const char *str, char *ostr, int olen,
>> Error **errp)
>> char *path, *optstr;
>> int sock, len;
>>
>> - opts = qemu_opts_create_nofail(&socket_optslist);
>> + opts = qemu_opts_create(&socket_optslist, NULL, 1, &error_abort);
>>
>> optstr = strchr(str, ',');
>> if (optstr) {
>> @@ -822,7 +822,7 @@ int unix_connect(const char *path, Error **errp)
>> QemuOpts *opts;
>> int sock;
>>
>> - opts = qemu_opts_create_nofail(&socket_optslist);
>> + opts = qemu_opts_create(&socket_optslist, NULL, 1, &error_abort);
>> qemu_opt_set(opts, "path", path);
>> sock = unix_connect_opts(opts, errp, NULL, NULL);
>> qemu_opts_del(opts);
>> @@ -839,7 +839,7 @@ int unix_nonblocking_connect(const char *path,
>>
>> g_assert(callback != NULL);
>>
>> - opts = qemu_opts_create_nofail(&socket_optslist);
>> + opts = qemu_opts_create(&socket_optslist, NULL, 1, &error_abort);
>> qemu_opt_set(opts, "path", path);
>> sock = unix_connect_opts(opts, errp, callback, opaque);
>> qemu_opts_del(opts);
>> @@ -889,7 +889,7 @@ int socket_connect(SocketAddress *addr, Error **errp,
>> QemuOpts *opts;
>> int fd;
>>
>> - opts = qemu_opts_create_nofail(&socket_optslist);
>> + opts = qemu_opts_create(&socket_optslist, NULL, 1, &error_abort);
>> switch (addr->kind) {
>> case SOCKET_ADDRESS_KIND_INET:
>> inet_addr_to_opts(opts, addr->inet);
>> @@ -921,7 +921,7 @@ int socket_listen(SocketAddress *addr, Error **errp)
>> QemuOpts *opts;
>> int fd;
>>
>> - opts = qemu_opts_create_nofail(&socket_optslist);
>> + opts = qemu_opts_create(&socket_optslist, NULL, 1, &error_abort);
>> switch (addr->kind) {
>> case SOCKET_ADDRESS_KIND_INET:
>> inet_addr_to_opts(opts, addr->inet);
>> @@ -949,7 +949,7 @@ int socket_dgram(SocketAddress *remote, SocketAddress
>> *local, Error **errp)
>> QemuOpts *opts;
>> int fd;
>>
>> - opts = qemu_opts_create_nofail(&socket_optslist);
>> + opts = qemu_opts_create(&socket_optslist, NULL, 1, &error_abort);
>> switch (remote->kind) {
>> case SOCKET_ADDRESS_KIND_INET:
>> qemu_opt_set(opts, "host", remote->inet->host);
>> diff --git a/vl.c b/vl.c
>> index 8d5d874..4d5704b 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -543,7 +543,7 @@ QemuOpts *qemu_get_machine_opts(void)
>> assert(list);
>> opts = qemu_opts_find(list, NULL);
>> if (!opts) {
>> - opts = qemu_opts_create_nofail(list);
>> + opts = qemu_opts_create(list, NULL, 1, &error_abort);
>> }
>> return opts;
>> }
>> @@ -2252,7 +2252,8 @@ static int balloon_parse(const char *arg)
>> return -1;
>> } else {
>> /* create empty opts */
>> - opts = qemu_opts_create_nofail(qemu_find_opts("device"));
>> + opts = qemu_opts_create(qemu_find_opts("device"), NULL, 1,
>> + &error_abort);
>> }
>> qemu_opt_set(opts, "driver", "virtio-balloon");
>> return 0;
>> @@ -2512,14 +2513,14 @@ static int virtcon_parse(const char *devname)
>> exit(1);
>> }
>>
>> - bus_opts = qemu_opts_create_nofail(device);
>> + bus_opts = qemu_opts_create(device, NULL, 1, &error_abort);
>> if (arch_type == QEMU_ARCH_S390X) {
>> qemu_opt_set(bus_opts, "driver", "virtio-serial-s390");
>> } else {
>> qemu_opt_set(bus_opts, "driver", "virtio-serial-pci");
>> }
>>
>> - dev_opts = qemu_opts_create_nofail(device);
>> + dev_opts = qemu_opts_create(device, NULL, 1, &error_abort);
>> qemu_opt_set(dev_opts, "driver", "virtconsole");
>>
>> snprintf(label, sizeof(label), "virtcon%d", index);
>> @@ -3376,8 +3377,9 @@ int main(int argc, char **argv, char **envp)
>> }
>>
>> qemu_opt_set_bool(fsdev, "readonly",
>> - qemu_opt_get_bool(opts, "readonly", 0));
>> - device = qemu_opts_create_nofail(qemu_find_opts("device"));
>> + qemu_opt_get_bool(opts, "readonly", 0));
>
> Spurious whitespace change, please fix.
>
Is the change incorrect? I just did it as it was right next to my
change and corrects relative to the local formatting.
Regards,
Peter
>> + device = qemu_opts_create(qemu_find_opts("device"), NULL, 1,
>> + &error_abort);
>> qemu_opt_set(device, "driver", "virtio-9p-pci");
>> qemu_opt_set(device, "fsdev",
>> qemu_opt_get(opts, "mount_tag"));
>> @@ -3397,7 +3399,8 @@ int main(int argc, char **argv, char **envp)
>> }
>> qemu_opt_set(fsdev, "fsdriver", "synth");
>>
>> - device = qemu_opts_create_nofail(qemu_find_opts("device"));
>> + device = qemu_opts_create(qemu_find_opts("device"), NULL, 1,
>> + &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");
>