From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50147) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z4WnN-0004wf-S1 for qemu-devel@nongnu.org; Mon, 15 Jun 2015 11:59:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z4WnJ-0007m0-S3 for qemu-devel@nongnu.org; Mon, 15 Jun 2015 11:59:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41584) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z4WnJ-0007j5-Io for qemu-devel@nongnu.org; Mon, 15 Jun 2015 11:59:29 -0400 Date: Mon, 15 Jun 2015 11:18:33 -0400 From: Luiz Capitulino Message-ID: <20150615111833.0d5398fe@redhat.com> In-Reply-To: <1434205258-1932-5-git-send-email-armbru@redhat.com> References: <1434205258-1932-1-git-send-email-armbru@redhat.com> <1434205258-1932-5-git-send-email-armbru@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 04/11] qerror: Eliminate QERR_DEVICE_NOT_FOUND List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: kwolf@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, mdroth@linux.vnet.ibm.com On Sat, 13 Jun 2015 16:20:51 +0200 Markus Armbruster wrote: > Error classes other than ERROR_CLASS_GENERIC_ERROR should not be used > in new code. Hiding them in QERR_ macros makes new uses hard to spot. > Fortunately, there's just one such macro left. Eliminate it with this > coccinelle semantic patch: > > @@ > expression EP, E; > @@ > -error_set(EP, QERR_DEVICE_NOT_FOUND, E) > +error_set(EP, ERROR_CLASS_DEVICE_NOT_FOUND, "Device '%s' not found", E) This is a bit minor, but I think I'd have created a new function instead, say error_set_enodev(). This avoids all the duplication. But I'm not asking you to change, as the patch is good and this can be done in the future if we so want. > > Signed-off-by: Markus Armbruster > --- > backends/rng-egd.c | 3 ++- > blockdev-nbd.c | 3 ++- > blockdev.c | 33 ++++++++++++++++++++++----------- > hmp.c | 6 ++++-- > include/qapi/qmp/qerror.h | 3 --- > net/net.c | 6 ++++-- > qdev-monitor.c | 6 ++++-- > qmp.c | 12 ++++++++---- > qom/object.c | 6 ++++-- > ui/input.c | 3 ++- > 10 files changed, 52 insertions(+), 29 deletions(-) > > diff --git a/backends/rng-egd.c b/backends/rng-egd.c > index 2962795..849bd7a 100644 > --- a/backends/rng-egd.c > +++ b/backends/rng-egd.c > @@ -147,7 +147,8 @@ static void rng_egd_opened(RngBackend *b, Error **errp) > > s->chr = qemu_chr_find(s->chr_name); > if (s->chr == NULL) { > - error_set(errp, QERR_DEVICE_NOT_FOUND, s->chr_name); > + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", s->chr_name); > return; > } > > diff --git a/blockdev-nbd.c b/blockdev-nbd.c > index 0d9df47..128e810 100644 > --- a/blockdev-nbd.c > +++ b/blockdev-nbd.c > @@ -91,7 +91,8 @@ void qmp_nbd_server_add(const char *device, bool has_writable, bool writable, > > blk = blk_by_name(device); > if (!blk) { > - error_set(errp, QERR_DEVICE_NOT_FOUND, device); > + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", device); > return; > } > if (!blk_is_inserted(blk)) { > diff --git a/blockdev.c b/blockdev.c > index a88ea76..8dec0cc 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -1102,7 +1102,8 @@ SnapshotInfo *qmp_blockdev_snapshot_delete_internal_sync(const char *device, > > blk = blk_by_name(device); > if (!blk) { > - error_set(errp, QERR_DEVICE_NOT_FOUND, device); > + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", device); > return NULL; > } > bs = blk_bs(blk); > @@ -1291,7 +1292,8 @@ static void internal_snapshot_prepare(BlkTransactionState *common, > /* 2. check for validation */ > blk = blk_by_name(device); > if (!blk) { > - error_set(errp, QERR_DEVICE_NOT_FOUND, device); > + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", device); > return; > } > bs = blk_bs(blk); > @@ -1571,7 +1573,8 @@ static void drive_backup_prepare(BlkTransactionState *common, Error **errp) > > blk = blk_by_name(backup->device); > if (!blk) { > - error_set(errp, QERR_DEVICE_NOT_FOUND, backup->device); > + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", backup->device); > return; > } > bs = blk_bs(blk); > @@ -1841,7 +1844,8 @@ void qmp_eject(const char *device, bool has_force, bool force, Error **errp) > > blk = blk_by_name(device); > if (!blk) { > - error_set(errp, QERR_DEVICE_NOT_FOUND, device); > + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", device); > return; > } > > @@ -1901,7 +1905,8 @@ void qmp_change_blockdev(const char *device, const char *filename, > > blk = blk_by_name(device); > if (!blk) { > - error_set(errp, QERR_DEVICE_NOT_FOUND, device); > + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", device); > return; > } > bs = blk_bs(blk); > @@ -1960,7 +1965,8 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd, > > blk = blk_by_name(device); > if (!blk) { > - error_set(errp, QERR_DEVICE_NOT_FOUND, device); > + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", device); > return; > } > bs = blk_bs(blk); > @@ -2275,7 +2281,8 @@ void qmp_block_stream(const char *device, > > blk = blk_by_name(device); > if (!blk) { > - error_set(errp, QERR_DEVICE_NOT_FOUND, device); > + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", device); > return; > } > bs = blk_bs(blk); > @@ -2349,7 +2356,8 @@ void qmp_block_commit(const char *device, > * scenario in which all optional arguments are omitted. */ > blk = blk_by_name(device); > if (!blk) { > - error_set(errp, QERR_DEVICE_NOT_FOUND, device); > + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", device); > return; > } > bs = blk_bs(blk); > @@ -2461,7 +2469,8 @@ void qmp_drive_backup(const char *device, const char *target, > > blk = blk_by_name(device); > if (!blk) { > - error_set(errp, QERR_DEVICE_NOT_FOUND, device); > + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", device); > return; > } > bs = blk_bs(blk); > @@ -2675,7 +2684,8 @@ void qmp_drive_mirror(const char *device, const char *target, > > blk = blk_by_name(device); > if (!blk) { > - error_set(errp, QERR_DEVICE_NOT_FOUND, device); > + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", device); > return; > } > bs = blk_bs(blk); > @@ -2941,7 +2951,8 @@ void qmp_change_backing_file(const char *device, > > blk = blk_by_name(device); > if (!blk) { > - error_set(errp, QERR_DEVICE_NOT_FOUND, device); > + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", device); > return; > } > bs = blk_bs(blk); > diff --git a/hmp.c b/hmp.c > index 05e4927..6f63927 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -1863,7 +1863,8 @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict) > if (blk) { > qemuio_command(blk, command); > } else { > - error_set(&err, QERR_DEVICE_NOT_FOUND, device); > + error_set(&err, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", device); > } > > hmp_handle_error(mon, &err); > @@ -1991,7 +1992,8 @@ void hmp_qom_set(Monitor *mon, const QDict *qdict) > > obj = object_resolve_path(path, &ambiguous); > if (obj == NULL) { > - error_set(&err, QERR_DEVICE_NOT_FOUND, path); > + error_set(&err, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", path); > } else { > if (ambiguous) { > monitor_printf(mon, "Warning: Path '%s' is ambiguous\n", path); > diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h > index 6468e40..2841344 100644 > --- a/include/qapi/qmp/qerror.h > +++ b/include/qapi/qmp/qerror.h > @@ -55,9 +55,6 @@ void qerror_report_err(Error *err); > #define QERR_DEVICE_NO_HOTPLUG \ > ERROR_CLASS_GENERIC_ERROR, "Device '%s' does not support hotplugging" > > -#define QERR_DEVICE_NOT_FOUND \ > - ERROR_CLASS_DEVICE_NOT_FOUND, "Device '%s' not found" > - > #define QERR_FD_NOT_FOUND \ > ERROR_CLASS_GENERIC_ERROR, "File descriptor named '%s' not found" > > diff --git a/net/net.c b/net/net.c > index 429012f..212762d 100644 > --- a/net/net.c > +++ b/net/net.c > @@ -1109,7 +1109,8 @@ void qmp_netdev_del(const char *id, Error **errp) > > nc = qemu_find_netdev(id); > if (!nc) { > - error_set(errp, QERR_DEVICE_NOT_FOUND, id); > + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", id); > return; > } > > @@ -1220,7 +1221,8 @@ void qmp_set_link(const char *name, bool up, Error **errp) > MAX_QUEUE_NUM); > > if (queues == 0) { > - error_set(errp, QERR_DEVICE_NOT_FOUND, name); > + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", name); > return; > } > nc = ncs[0]; > diff --git a/qdev-monitor.c b/qdev-monitor.c > index e1dd367..7bd7d25 100644 > --- a/qdev-monitor.c > +++ b/qdev-monitor.c > @@ -457,7 +457,8 @@ static BusState *qbus_find(const char *path, Error **errp) > pos += len; > dev = qbus_find_dev(bus, elem); > if (!dev) { > - error_set(errp, QERR_DEVICE_NOT_FOUND, elem); > + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", elem); > #if 0 /* conversion from qerror_report() to error_set() broke this: */ > if (!monitor_cur_is_qmp()) { > qbus_list_dev(bus); > @@ -793,7 +794,8 @@ void qmp_device_del(const char *id, Error **errp) > g_free(path); > > if (!obj) { > - error_set(errp, QERR_DEVICE_NOT_FOUND, id); > + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", id); > return; > } > > diff --git a/qmp.c b/qmp.c > index 60658b4..14f7536 100644 > --- a/qmp.c > +++ b/qmp.c > @@ -206,7 +206,8 @@ ObjectPropertyInfoList *qmp_qom_list(const char *path, Error **errp) > if (ambiguous) { > error_setg(errp, "Path '%s' is ambiguous", path); > } else { > - error_set(errp, QERR_DEVICE_NOT_FOUND, path); > + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", path); > } > return NULL; > } > @@ -236,7 +237,8 @@ int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject **ret) > > obj = object_resolve_path(path, NULL); > if (!obj) { > - error_set(&local_err, QERR_DEVICE_NOT_FOUND, path); > + error_set(&local_err, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", path); > goto out; > } > > @@ -261,7 +263,8 @@ int qmp_qom_get(Monitor *mon, const QDict *qdict, QObject **ret) > > obj = object_resolve_path(path, NULL); > if (!obj) { > - error_set(&local_err, QERR_DEVICE_NOT_FOUND, path); > + error_set(&local_err, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", path); > goto out; > } > > @@ -518,7 +521,8 @@ DevicePropertyInfoList *qmp_device_list_properties(const char *typename, > > klass = object_class_by_name(typename); > if (klass == NULL) { > - error_set(errp, QERR_DEVICE_NOT_FOUND, typename); > + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", typename); > return NULL; > } > > diff --git a/qom/object.c b/qom/object.c > index 96abd34..62fa44e 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -890,7 +890,8 @@ Object *object_property_get_link(Object *obj, const char *name, > if (str && *str) { > target = object_resolve_path(str, NULL); > if (!target) { > - error_set(errp, QERR_DEVICE_NOT_FOUND, str); > + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", str); > } > } > > @@ -1170,7 +1171,8 @@ static Object *object_resolve_link(Object *obj, const char *name, > if (target || ambiguous) { > error_set(errp, QERR_INVALID_PARAMETER_TYPE, name, target_type); > } else { > - error_set(errp, QERR_DEVICE_NOT_FOUND, path); > + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", path); > } > target = NULL; > } > diff --git a/ui/input.c b/ui/input.c > index eeeabe8..e96e1ea 100644 > --- a/ui/input.c > +++ b/ui/input.c > @@ -84,7 +84,8 @@ void qemu_input_handler_bind(QemuInputHandlerState *s, > > dev = qdev_find_recursive(sysbus_get_default(), device_id); > if (dev == NULL) { > - error_set(errp, QERR_DEVICE_NOT_FOUND, device_id); > + error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", device_id); > return; > } >