From patchwork Mon May 21 17:41:44 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Luiz Capitulino X-Patchwork-Id: 160404 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 40C77B6F62 for ; Tue, 22 May 2012 04:10:32 +1000 (EST) Received: from localhost ([::1]:48887 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SWX3t-000693-Vp for incoming@patchwork.ozlabs.org; Mon, 21 May 2012 14:10:29 -0400 Received: from eggs.gnu.org ([208.118.235.92]:57801) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SWX3h-0005y0-Ry for qemu-devel@nongnu.org; Mon, 21 May 2012 14:10:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SWX3e-0000t4-Vq for qemu-devel@nongnu.org; Mon, 21 May 2012 14:10:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51465) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SWX3e-0000sr-Mp for qemu-devel@nongnu.org; Mon, 21 May 2012 14:10:14 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q4LI84d8005978 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 21 May 2012 14:10:03 -0400 Received: from localhost (ovpn-116-87.ams2.redhat.com [10.36.116.87]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id q4LHfhp0008583; Mon, 21 May 2012 13:41:44 -0400 From: Luiz Capitulino To: qemu-devel@nongnu.org Date: Mon, 21 May 2012 14:41:44 -0300 Message-Id: <1337622119-8416-2-git-send-email-lcapitulino@redhat.com> In-Reply-To: <1337622119-8416-1-git-send-email-lcapitulino@redhat.com> References: <1337622119-8416-1-git-send-email-lcapitulino@redhat.com> X-Scanned-By: MIMEDefang 2.67 on 10.5.11.11 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.132.183.28 Cc: pbonzini@redhat.com, aliguori@us.ibm.com, lersek@redhat.com, mdroth@linux.vnet.ibm.com, armbru@redhat.com Subject: [Qemu-devel] [PATCH 01/16] qemu-option: qemu_opts_create(): use error_set() X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org This commit converts qemu_opts_create() from qerror_report() to error_set(). Currently, most calls to qemu_opts_create() can't fail, so most callers don't need any changes. The two cases where code checks for qemu_opts_create() erros are: 1. Initialization code in vl.c. All of them print their own error messages directly to stderr, no need to pass the Error object 2. The functions opts_parse(), qemu_opts_from_qdict() and qemu_chr_parse_compat() make use of the error information and they can be called from HMP or QMP. In this case, to allow for incremental conversion, we propagate the error up using qerror_report_err(), which keeps the QError semantics Signed-off-by: Luiz Capitulino --- blockdev.c | 2 +- hw/usb/dev-storage.c | 2 +- hw/watchdog.c | 2 +- qemu-char.c | 8 ++++++-- qemu-config.c | 6 +++--- qemu-option.c | 37 +++++++++++++++++++++++++++---------- qemu-option.h | 4 +++- qemu-sockets.c | 8 ++++---- vl.c | 22 +++++++++++++--------- 9 files changed, 59 insertions(+), 32 deletions(-) diff --git a/blockdev.c b/blockdev.c index 67895b2..622ecba 100644 --- a/blockdev.c +++ b/blockdev.c @@ -569,7 +569,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) break; case IF_VIRTIO: /* add virtio block device */ - opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0); + opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0, NULL); if (arch_type == QEMU_ARCH_S390X) { qemu_opt_set(opts, "driver", "virtio-blk-s390"); } else { diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c index ae22fb1..a96c0b9 100644 --- a/hw/usb/dev-storage.c +++ b/hw/usb/dev-storage.c @@ -584,7 +584,7 @@ static USBDevice *usb_msd_init(USBBus *bus, const char *filename) /* parse -usbdevice disk: syntax into drive opts */ snprintf(id, sizeof(id), "usb%d", nr++); - opts = qemu_opts_create(qemu_find_opts("drive"), id, 0); + opts = qemu_opts_create(qemu_find_opts("drive"), id, 0, NULL); p1 = strchr(filename, ':'); if (p1++) { diff --git a/hw/watchdog.c b/hw/watchdog.c index 4c18965..a42124d 100644 --- a/hw/watchdog.c +++ b/hw/watchdog.c @@ -66,7 +66,7 @@ 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(qemu_find_opts("device"), NULL, 0); + opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0, NULL); qemu_opt_set(opts, "driver", p); return 0; } diff --git a/qemu-char.c b/qemu-char.c index fe1126f..0bd903f 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -2584,10 +2584,14 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename) int pos; const char *p; QemuOpts *opts; + Error *local_err = NULL; - opts = qemu_opts_create(qemu_find_opts("chardev"), label, 1); - if (NULL == opts) + opts = qemu_opts_create(qemu_find_opts("chardev"), label, 1, &local_err); + if (error_is_set(&local_err)) { + qerror_report_err(local_err); + error_free(local_err); return NULL; + } if (strstart(filename, "mon:", &p)) { filename = p; diff --git a/qemu-config.c b/qemu-config.c index be84a03..f876646 100644 --- a/qemu-config.c +++ b/qemu-config.c @@ -709,7 +709,7 @@ int qemu_global_option(const char *str) return -1; } - opts = qemu_opts_create(&qemu_global_opts, NULL, 0); + opts = qemu_opts_create(&qemu_global_opts, NULL, 0, NULL); qemu_opt_set(opts, "driver", driver); qemu_opt_set(opts, "property", property); qemu_opt_set(opts, "value", str+offset+1); @@ -781,7 +781,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname) list = find_list(lists, group); if (list == NULL) goto out; - opts = qemu_opts_create(list, id, 1); + opts = qemu_opts_create(list, id, 1, NULL); continue; } if (sscanf(line, "[%63[^]]]", group) == 1) { @@ -789,7 +789,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname) list = find_list(lists, group); if (list == NULL) goto out; - opts = qemu_opts_create(list, NULL, 0); + opts = qemu_opts_create(list, NULL, 0, NULL); continue; } if (sscanf(line, " %63s = \"%1023[^\"]\"", arg, value) == 2) { diff --git a/qemu-option.c b/qemu-option.c index 35cd609..9f531c8 100644 --- a/qemu-option.c +++ b/qemu-option.c @@ -30,6 +30,7 @@ #include "qemu-error.h" #include "qemu-objects.h" #include "qemu-option.h" +#include "error.h" #include "qerror.h" /* @@ -729,20 +730,21 @@ static int id_wellformed(const char *id) return 1; } -QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id, int fail_if_exists) +QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id, + int fail_if_exists, Error **errp) { QemuOpts *opts = NULL; if (id) { if (!id_wellformed(id)) { - qerror_report(QERR_INVALID_PARAMETER_VALUE, "id", "an identifier"); + error_set(errp,QERR_INVALID_PARAMETER_VALUE, "id", "an identifier"); error_printf_unless_qmp("Identifiers consist of letters, digits, '-', '.', '_', starting with a letter.\n"); return NULL; } opts = qemu_opts_find(list, id); if (opts != NULL) { if (fail_if_exists && !list->merge_lists) { - qerror_report(QERR_DUPLICATE_ID, id, list->name); + error_set(errp, QERR_DUPLICATE_ID, id, list->name); return NULL; } else { return opts; @@ -783,9 +785,12 @@ int qemu_opts_set(QemuOptsList *list, const char *id, const char *name, const char *value) { QemuOpts *opts; + Error *local_err = NULL; - opts = qemu_opts_create(list, id, 1); - if (opts == NULL) { + opts = qemu_opts_create(list, id, 1, &local_err); + if (error_is_set(&local_err)) { + qerror_report_err(local_err); + error_free(local_err); return -1; } return qemu_opt_set(opts, name, value); @@ -883,6 +888,7 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params, char value[1024], *id = NULL; const char *p; QemuOpts *opts; + Error *local_err = NULL; assert(!permit_abbrev || list->implied_opt_name); firstname = permit_abbrev ? list->implied_opt_name : NULL; @@ -898,13 +904,18 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params, if (!id && !QTAILQ_EMPTY(&list->head)) { opts = qemu_opts_find(list, NULL); } else { - opts = qemu_opts_create(list, id, 0); + opts = qemu_opts_create(list, id, 0, &local_err); } } else { - opts = qemu_opts_create(list, id, 1); + opts = qemu_opts_create(list, id, 1, &local_err); } - if (opts == NULL) + if (opts == NULL) { + if (error_is_set(&local_err)) { + qerror_report_err(local_err); + error_free(local_err); + } return NULL; + } if (opts_do_parse(opts, params, firstname, defaults) != 0) { qemu_opts_del(opts); @@ -975,11 +986,17 @@ static void qemu_opts_from_qdict_1(const char *key, QObject *obj, void *opaque) QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict) { QemuOpts *opts; + Error *local_err = NULL; - opts = qemu_opts_create(list, qdict_get_try_str(qdict, "id"), 1); - if (opts == NULL) + opts = qemu_opts_create(list, qdict_get_try_str(qdict, "id"), 1, + &local_err); + if (error_is_set(&local_err)) { + qerror_report_err(local_err); + error_free(local_err); return NULL; + } + assert(opts != NULL); qdict_iter(qdict, qemu_opts_from_qdict_1, opts); return opts; } diff --git a/qemu-option.h b/qemu-option.h index 3ca00c3..4d5b3d3 100644 --- a/qemu-option.h +++ b/qemu-option.h @@ -28,6 +28,7 @@ #include #include "qemu-queue.h" +#include "error.h" #include "qdict.h" enum QEMUOptionParType { @@ -116,7 +117,8 @@ int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque, int abort_on_failure); QemuOpts *qemu_opts_find(QemuOptsList *list, const char *id); -QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id, int fail_if_exists); +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, diff --git a/qemu-sockets.c b/qemu-sockets.c index 46c7619..2ae715d 100644 --- a/qemu-sockets.c +++ b/qemu-sockets.c @@ -461,7 +461,7 @@ int inet_listen(const char *str, char *ostr, int olen, char *optstr; int sock = -1; - opts = qemu_opts_create(&dummy_opts, NULL, 0); + opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL); if (inet_parse(opts, str) == 0) { sock = inet_listen_opts(opts, port_offset, errp); if (sock != -1 && ostr) { @@ -490,7 +490,7 @@ int inet_connect(const char *str, bool block, Error **errp) QemuOpts *opts; int sock = -1; - opts = qemu_opts_create(&dummy_opts, NULL, 0); + opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL); if (inet_parse(opts, str) == 0) { if (block) { qemu_opt_set(opts, "block", "on"); @@ -589,7 +589,7 @@ int unix_listen(const char *str, char *ostr, int olen) char *path, *optstr; int sock, len; - opts = qemu_opts_create(&dummy_opts, NULL, 0); + opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL); optstr = strchr(str, ','); if (optstr) { @@ -617,7 +617,7 @@ int unix_connect(const char *path) QemuOpts *opts; int sock; - opts = qemu_opts_create(&dummy_opts, NULL, 0); + opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL); qemu_opt_set(opts, "path", path); sock = unix_connect_opts(opts); qemu_opts_del(opts); diff --git a/vl.c b/vl.c index 23ab3a3..1485426 100644 --- a/vl.c +++ b/vl.c @@ -1786,7 +1786,7 @@ static int balloon_parse(const char *arg) return -1; } else { /* create empty opts */ - opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0); + opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0, NULL); } qemu_opt_set(opts, "driver", "virtio-balloon"); return 0; @@ -1921,7 +1921,7 @@ static void monitor_parse(const char *optarg, const char *mode) } } - opts = qemu_opts_create(qemu_find_opts("mon"), label, 1); + opts = qemu_opts_create(qemu_find_opts("mon"), label, 1, NULL); if (!opts) { fprintf(stderr, "duplicate chardev: %s\n", label); exit(1); @@ -2035,14 +2035,14 @@ static int virtcon_parse(const char *devname) exit(1); } - bus_opts = qemu_opts_create(device, NULL, 0); + bus_opts = qemu_opts_create(device, NULL, 0, NULL); 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(device, NULL, 0); + dev_opts = qemu_opts_create(device, NULL, 0, NULL); qemu_opt_set(dev_opts, "driver", "virtconsole"); snprintf(label, sizeof(label), "virtcon%d", index); @@ -2065,7 +2065,7 @@ static int debugcon_parse(const char *devname) if (!qemu_chr_new("debugcon", devname, NULL)) { exit(1); } - opts = qemu_opts_create(qemu_find_opts("device"), "debugcon", 1); + opts = qemu_opts_create(qemu_find_opts("device"), "debugcon", 1, NULL); if (!opts) { fprintf(stderr, "qemu: already have a debugcon device\n"); exit(1); @@ -2813,7 +2813,8 @@ int main(int argc, char **argv, char **envp) exit(1); } fsdev = qemu_opts_create(qemu_find_opts("fsdev"), - qemu_opt_get(opts, "mount_tag"), 1); + qemu_opt_get(opts, "mount_tag"), + 1, NULL); if (!fsdev) { fprintf(stderr, "duplicate fsdev id: %s\n", qemu_opt_get(opts, "mount_tag")); @@ -2845,7 +2846,8 @@ 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(qemu_find_opts("device"), NULL, 0); + device = qemu_opts_create(qemu_find_opts("device"), NULL, 0, + NULL); qemu_opt_set(device, "driver", "virtio-9p-pci"); qemu_opt_set(device, "fsdev", qemu_opt_get(opts, "mount_tag")); @@ -2857,14 +2859,16 @@ int main(int argc, char **argv, char **envp) QemuOpts *fsdev; QemuOpts *device; - fsdev = qemu_opts_create(qemu_find_opts("fsdev"), "v_synth", 1); + fsdev = qemu_opts_create(qemu_find_opts("fsdev"), "v_synth", + 1, NULL); if (!fsdev) { fprintf(stderr, "duplicate option: %s\n", "virtfs_synth"); exit(1); } qemu_opt_set(fsdev, "fsdriver", "synth"); - device = qemu_opts_create(qemu_find_opts("device"), NULL, 0); + device = qemu_opts_create(qemu_find_opts("device"), NULL, 0, + NULL); qemu_opt_set(device, "driver", "virtio-9p-pci"); qemu_opt_set(device, "fsdev", "v_synth"); qemu_opt_set(device, "mount_tag", "v_synth");