Re: [PATCH v2 01/22] qapi: Inline and remove QERR_BUS_NO_HOTPLUG definition
On 10/5/23 06:50, Philippe Mathieu-Daudé wrote: Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using sed, manually removing the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Cédric Le Goater Thanks, C. --- include/qapi/qmp/qerror.h | 3 --- hw/ppc/spapr_pci.c| 4 ++-- softmmu/qdev-monitor.c| 8 +--- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index 8dd9fcb071..1a9c2d3502 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_BUS_NO_HOTPLUG \ -"Bus '%s' does not support hotplugging" - #define QERR_DEVICE_HAS_NO_MEDIUM \ "Device '%s' has no medium" diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 370c5a90f2..7f063f5852 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -1550,7 +1550,7 @@ static void spapr_pci_pre_plug(HotplugHandler *plug_handler, * we need to let them know it's not enabled */ if (plugged_dev->hotplugged) { -error_setg(errp, QERR_BUS_NO_HOTPLUG, +error_setg(errp, "Bus '%s' does not support hotplugging", object_get_typename(OBJECT(phb))); return; } @@ -1671,7 +1671,7 @@ static void spapr_pci_unplug_request(HotplugHandler *plug_handler, SpaprDrc *drc = drc_from_dev(phb, pdev); if (!phb->dr_enabled) { -error_setg(errp, QERR_BUS_NO_HOTPLUG, +error_setg(errp, "Bus '%s' does not support hotplugging", object_get_typename(OBJECT(phb))); return; } diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c index 74f4e41338..3a9740dcbd 100644 --- a/softmmu/qdev-monitor.c +++ b/softmmu/qdev-monitor.c @@ -656,7 +656,8 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts, if (qdev_should_hide_device(opts, from_json, errp)) { if (bus && !qbus_is_hotpluggable(bus)) { -error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name); +error_setg(errp, "Bus '%s' does not support hotplugging", + bus->name); } return NULL; } else if (*errp) { @@ -664,7 +665,7 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts, } if (phase_check(PHASE_MACHINE_READY) && bus && !qbus_is_hotpluggable(bus)) { -error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name); +error_setg(errp, "Bus '%s' does not support hotplugging", bus->name); return NULL; } @@ -904,7 +905,8 @@ void qdev_unplug(DeviceState *dev, Error **errp) } if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) { -error_setg(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name); +error_setg(errp, "Bus '%s' does not support hotplugging", + dev->parent_bus->name); return; }
Re: [PATCH v3 10/16] ui/cocoa: Clean up global variable shadowing
On 2023/10/04 21:00, Philippe Mathieu-Daudé wrote: Fix: ui/cocoa.m:346:20: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] QemuCocoaView *cocoaView = userInfo; ^ ui/cocoa.m:342:16: note: previous declaration is here QemuCocoaView *cocoaView; ^ Signed-off-by: Philippe Mathieu-Daudé --- ui/cocoa.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/cocoa.m b/ui/cocoa.m index df6d13be38..15477288fd 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -343,9 +343,9 @@ - (void) raiseAllKeys; static CGEventRef handleTapEvent(CGEventTapProxy proxy, CGEventType type, CGEventRef cgEvent, void *userInfo) { -QemuCocoaView *cocoaView = userInfo; +QemuCocoaView *view = userInfo; NSEvent *event = [NSEvent eventWithCGEvent:cgEvent]; -if ([cocoaView isMouseGrabbed] && [cocoaView handleEvent:event]) { +if ([view isMouseGrabbed] && [view handleEvent:event]) { COCOA_DEBUG("Global events tap: qemu handled the event, capturing!\n"); return NULL; } Reviewed-by: Akihiko Odaki
[PATCH v2 10/22] qapi: Correct error message for 'vcpu_dirty_limit' parameter
QERR_INVALID_PARAMETER_VALUE is defined as: #define QERR_INVALID_PARAMETER_VALUE \ "Parameter '%s' expects %s" The current error is formatted as: "Parameter 'vcpu_dirty_limit' expects is invalid, it must greater then 1 MB/s" Replace by: "Parameter 'vcpu_dirty_limit' is invalid, it must greater then 1 MB/s" Signed-off-by: Philippe Mathieu-Daudé --- migration/options.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/migration/options.c b/migration/options.c index 1d1e1321b0..79fce0c3a9 100644 --- a/migration/options.c +++ b/migration/options.c @@ -1163,9 +1163,8 @@ bool migrate_params_check(MigrationParameters *params, Error **errp) if (params->has_vcpu_dirty_limit && (params->vcpu_dirty_limit < 1)) { -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, - "vcpu_dirty_limit", - "is invalid, it must greater then 1 MB/s"); +error_setg(errp, "Parameter 'vcpu_dirty_limit' is invalid," + " it must greater then 1 MB/s"); return false; } -- 2.41.0
[PATCH v2 07/22] qapi: Inline QERR_INVALID_PARAMETER_TYPE definition (constant param)
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using the following coccinelle semantic patch: @match@ expression errp; constant param; constant value; @@ error_setg(errp, QERR_INVALID_PARAMETER_TYPE, param, value); @script:python strformat depends on match@ param << match.param; value << match.value; fixedfmt; // new var @@ fixedfmt = f'"Invalid parameter type for \'{param[1:-1]}\', expected: {value[1:-1]}"' coccinelle.fixedfmt = cocci.make_ident(fixedfmt) @replace@ expression match.errp; constant match.param; constant match.value; identifier strformat.fixedfmt; @@ -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, param, value); +error_setg(errp, fixedfmt); Signed-off-by: Philippe Mathieu-Daudé Acked-by: Thomas Huth --- target/arm/arm-qmp-cmds.c| 3 ++- target/s390x/cpu_models_sysemu.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/target/arm/arm-qmp-cmds.c b/target/arm/arm-qmp-cmds.c index b53d5efe13..3c99fd8222 100644 --- a/target/arm/arm-qmp-cmds.c +++ b/target/arm/arm-qmp-cmds.c @@ -154,7 +154,8 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type, if (model->props) { qdict_in = qobject_to(QDict, model->props); if (!qdict_in) { -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, "props", "dict"); +error_setg(errp, + "Invalid parameter type for 'props', expected: dict"); return NULL; } } diff --git a/target/s390x/cpu_models_sysemu.c b/target/s390x/cpu_models_sysemu.c index 63981bf36b..4507714493 100644 --- a/target/s390x/cpu_models_sysemu.c +++ b/target/s390x/cpu_models_sysemu.c @@ -111,7 +111,8 @@ static void cpu_model_from_info(S390CPUModel *model, const CpuModelInfo *info, if (info->props) { qdict = qobject_to(QDict, info->props); if (!qdict) { -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, "props", "dict"); +error_setg(errp, + "Invalid parameter type for 'props', expected: dict"); return; } } -- 2.41.0
[PATCH v2 02/22] qapi: Inline and remove QERR_DEVICE_HAS_NO_MEDIUM definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using sed, manually removing the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé --- include/qapi/qmp/qerror.h | 3 --- block/snapshot.c | 4 ++-- blockdev.c| 2 +- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index 1a9c2d3502..168177bcd7 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_DEVICE_HAS_NO_MEDIUM \ -"Device '%s' has no medium" - #define QERR_DEVICE_IN_USE \ "Device '%s' is in use" diff --git a/block/snapshot.c b/block/snapshot.c index b86b5b24ad..eb43e957e1 100644 --- a/block/snapshot.c +++ b/block/snapshot.c @@ -345,7 +345,7 @@ int bdrv_snapshot_delete(BlockDriverState *bs, GLOBAL_STATE_CODE(); if (!drv) { -error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, bdrv_get_device_name(bs)); +error_setg(errp, "Device '%s' has no medium", bdrv_get_device_name(bs)); return -ENOMEDIUM; } if (!snapshot_id && !name) { @@ -420,7 +420,7 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs, GLOBAL_STATE_CODE(); if (!drv) { -error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, bdrv_get_device_name(bs)); +error_setg(errp, "Device '%s' has no medium", bdrv_get_device_name(bs)); return -ENOMEDIUM; } if (!snapshot_id && !name) { diff --git a/blockdev.c b/blockdev.c index 325b7a3bef..e5617faf0f 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1422,7 +1422,7 @@ static void external_snapshot_action(TransactionAction *action, bdrv_drained_begin(state->old_bs); if (!bdrv_is_inserted(state->old_bs)) { -error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, device); +error_setg(errp, "Device '%s' has no medium", device); goto out; } -- 2.41.0
[PATCH v2 17/22] qapi: Inline and remove QERR_MISSING_PARAMETER definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using sed, manually removing the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé --- include/qapi/qmp/qerror.h| 3 --- qapi/opts-visitor.c | 2 +- qapi/qapi-forward-visitor.c | 2 +- qapi/qobject-input-visitor.c | 2 +- 4 files changed, 3 insertions(+), 6 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index cc4dae1076..b0f48f22fe 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_MISSING_PARAMETER \ -"Parameter '%s' is missing" - #define QERR_PROPERTY_VALUE_BAD \ "Property '%s.%s' doesn't take value '%s'" diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c index 844db583f4..bf0d8acbd6 100644 --- a/qapi/opts-visitor.c +++ b/qapi/opts-visitor.c @@ -218,7 +218,7 @@ lookup_distinct(const OptsVisitor *ov, const char *name, Error **errp) list = g_hash_table_lookup(ov->unprocessed_opts, name); if (!list) { -error_setg(errp, QERR_MISSING_PARAMETER, name); +error_setg(errp, "Parameter '%s' is missing", name); } return list; } diff --git a/qapi/qapi-forward-visitor.c b/qapi/qapi-forward-visitor.c index e36d9bc9ba..3fb2c954aa 100644 --- a/qapi/qapi-forward-visitor.c +++ b/qapi/qapi-forward-visitor.c @@ -49,7 +49,7 @@ static bool forward_field_translate_name(ForwardFieldVisitor *v, const char **na *name = v->to; return true; } -error_setg(errp, QERR_MISSING_PARAMETER, *name); +error_setg(errp, "Parameter '%s' is missing", *name); return false; } diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c index f5fa6c1878..17e9f3b638 100644 --- a/qapi/qobject-input-visitor.c +++ b/qapi/qobject-input-visitor.c @@ -168,7 +168,7 @@ static QObject *qobject_input_get_object(QObjectInputVisitor *qiv, QObject *obj = qobject_input_try_get_object(qiv, name, consume); if (!obj) { -error_setg(errp, QERR_MISSING_PARAMETER, full_name(qiv, name)); +error_setg(errp, "Parameter '%s' is missing", full_name(qiv, name)); } return obj; } -- 2.41.0
[PATCH v2 18/22] qapi: Inline and remove QERR_PROPERTY_VALUE_BAD definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Manual change. Remove the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé --- include/qapi/qmp/qerror.h | 3 --- hw/core/qdev-properties.c | 2 +- target/i386/cpu.c | 2 +- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index b0f48f22fe..7862ac55a1 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_PROPERTY_VALUE_BAD \ -"Property '%s.%s' doesn't take value '%s'" - #define QERR_PROPERTY_VALUE_OUT_OF_RANGE \ "Property %s.%s doesn't take value %" PRId64 " (minimum: %" PRId64 ", maximum: %" PRId64 ")" diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index 357b8761b5..44fc1686e0 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -682,7 +682,7 @@ void error_set_from_qdev_prop_error(Error **errp, int ret, Object *obj, break; default: case -EINVAL: -error_setg(errp, QERR_PROPERTY_VALUE_BAD, +error_setg(errp, "Property '%s.%s' doesn't take value '%s'", object_get_typename(obj), name, value); break; case -ENOENT: diff --git a/target/i386/cpu.c b/target/i386/cpu.c index ed72883bf3..e5a14885ed 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -5190,7 +5190,7 @@ static void x86_cpuid_set_vendor(Object *obj, const char *value, int i; if (strlen(value) != CPUID_VENDOR_SZ) { -error_setg(errp, QERR_PROPERTY_VALUE_BAD, "", "vendor", value); +error_setg(errp, "Property 'vendor' doesn't take value '%s'", value); return; } -- 2.41.0
[PATCH v2 01/22] qapi: Inline and remove QERR_BUS_NO_HOTPLUG definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using sed, manually removing the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé --- include/qapi/qmp/qerror.h | 3 --- hw/ppc/spapr_pci.c| 4 ++-- softmmu/qdev-monitor.c| 8 +--- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index 8dd9fcb071..1a9c2d3502 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_BUS_NO_HOTPLUG \ -"Bus '%s' does not support hotplugging" - #define QERR_DEVICE_HAS_NO_MEDIUM \ "Device '%s' has no medium" diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 370c5a90f2..7f063f5852 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -1550,7 +1550,7 @@ static void spapr_pci_pre_plug(HotplugHandler *plug_handler, * we need to let them know it's not enabled */ if (plugged_dev->hotplugged) { -error_setg(errp, QERR_BUS_NO_HOTPLUG, +error_setg(errp, "Bus '%s' does not support hotplugging", object_get_typename(OBJECT(phb))); return; } @@ -1671,7 +1671,7 @@ static void spapr_pci_unplug_request(HotplugHandler *plug_handler, SpaprDrc *drc = drc_from_dev(phb, pdev); if (!phb->dr_enabled) { -error_setg(errp, QERR_BUS_NO_HOTPLUG, +error_setg(errp, "Bus '%s' does not support hotplugging", object_get_typename(OBJECT(phb))); return; } diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c index 74f4e41338..3a9740dcbd 100644 --- a/softmmu/qdev-monitor.c +++ b/softmmu/qdev-monitor.c @@ -656,7 +656,8 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts, if (qdev_should_hide_device(opts, from_json, errp)) { if (bus && !qbus_is_hotpluggable(bus)) { -error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name); +error_setg(errp, "Bus '%s' does not support hotplugging", + bus->name); } return NULL; } else if (*errp) { @@ -664,7 +665,7 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts, } if (phase_check(PHASE_MACHINE_READY) && bus && !qbus_is_hotpluggable(bus)) { -error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name); +error_setg(errp, "Bus '%s' does not support hotplugging", bus->name); return NULL; } @@ -904,7 +905,8 @@ void qdev_unplug(DeviceState *dev, Error **errp) } if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) { -error_setg(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name); +error_setg(errp, "Bus '%s' does not support hotplugging", + dev->parent_bus->name); return; } -- 2.41.0
[PATCH v2 13/22] qapi: Inline and remove QERR_INVALID_PARAMETER_VALUE definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Manually modify the error_report() call in softmmu/tpm.c, then use sed to mechanically transform the rest. Finally remove the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Stefan Berger --- include/qapi/qmp/qerror.h | 3 --- qapi/opts-visitor.c | 4 ++-- qapi/qapi-visit-core.c| 4 ++-- softmmu/tpm.c | 3 +-- 4 files changed, 5 insertions(+), 9 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index b723830eff..ac727d1c2d 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_INVALID_PARAMETER_VALUE \ -"Parameter '%s' expects %s" - #define QERR_IO_ERROR \ "An IO error has occurred" diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c index 0393704a73..844db583f4 100644 --- a/qapi/opts-visitor.c +++ b/qapi/opts-visitor.c @@ -441,7 +441,7 @@ opts_type_int64(Visitor *v, const char *name, int64_t *obj, Error **errp) } } } -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name, +error_setg(errp, "Parameter '%s' expects %s", opt->name, (ov->list_mode == LM_NONE) ? "an int64 value" : "an int64 value or range"); return false; @@ -494,7 +494,7 @@ opts_type_uint64(Visitor *v, const char *name, uint64_t *obj, Error **errp) } } } -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name, +error_setg(errp, "Parameter '%s' expects %s", opt->name, (ov->list_mode == LM_NONE) ? "a uint64 value" : "a uint64 value or range"); return false; diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c index 6c13510a2b..01793d6e74 100644 --- a/qapi/qapi-visit-core.c +++ b/qapi/qapi-visit-core.c @@ -194,7 +194,7 @@ static bool visit_type_uintN(Visitor *v, uint64_t *obj, const char *name, } if (value > max) { assert(v->type == VISITOR_INPUT); -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, +error_setg(errp, "Parameter '%s' expects %s", name ? name : "null", type); return false; } @@ -262,7 +262,7 @@ static bool visit_type_intN(Visitor *v, int64_t *obj, const char *name, } if (value < min || value > max) { assert(v->type == VISITOR_INPUT); -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, +error_setg(errp, "Parameter '%s' expects %s", name ? name : "null", type); return false; } diff --git a/softmmu/tpm.c b/softmmu/tpm.c index 578563f05a..8437c4efc3 100644 --- a/softmmu/tpm.c +++ b/softmmu/tpm.c @@ -120,8 +120,7 @@ static int tpm_init_tpmdev(void *dummy, QemuOpts *opts, Error **errp) i = qapi_enum_parse(_lookup, value, -1, NULL); be = i >= 0 ? tpm_be_find_by_type(i) : NULL; if (be == NULL) { -error_report(QERR_INVALID_PARAMETER_VALUE, - "type", "a TPM backend type"); +error_report("Parameter 'type' expects a TPM backend type"); tpm_display_backend_drivers(); return 1; } -- 2.41.0
[PATCH v2 20/22] qapi: Inline and remove QERR_QGA_COMMAND_FAILED definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using the following coccinelle semantic patch: @match exists@ expression errp; expression errmsg; @@ error_setg(errp, QERR_QGA_COMMAND_FAILED, errmsg); @script:python strformat depends on match@ errmsg << match.errmsg; fixedfmt; // new var @@ # Format skipping '"'. fixedfmt = f'"Guest agent command failed, error was \'{errmsg[1:-1]}\'"' coccinelle.fixedfmt = cocci.make_ident(fixedfmt) @replace@ expression match.errp; expression match.errmsg; identifier strformat.fixedfmt; @@ -error_setg(errp, QERR_QGA_COMMAND_FAILED, errmsg); +error_setg(errp, fixedfmt); then manually removing the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé --- include/qapi/qmp/qerror.h | 3 --- qga/commands-win32.c | 38 -- qga/commands.c| 7 --- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index e094f13114..840831cc6a 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_QGA_COMMAND_FAILED \ -"Guest agent command failed, error was '%s'" - #define QERR_UNSUPPORTED \ "this feature or command is not currently supported" diff --git a/qga/commands-win32.c b/qga/commands-win32.c index 946dbafbb6..aa8c9770d4 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -245,7 +245,8 @@ int64_t qmp_guest_file_open(const char *path, const char *mode, Error **errp) done: if (gerr) { -error_setg(errp, QERR_QGA_COMMAND_FAILED, gerr->message); +error_setg(errp, + "Guest agent command failed, error was 'err -> messag'"); g_error_free(gerr); } g_free(w_path); @@ -279,8 +280,8 @@ static void acquire_privilege(const char *name, Error **errp) TOKEN_ADJUST_PRIVILEGES | TOKEN_QUERY, )) { if (!LookupPrivilegeValue(NULL, name, [0].Luid)) { -error_setg(errp, QERR_QGA_COMMAND_FAILED, - "no luid for requested privilege"); +error_setg(errp, + "Guest agent command failed, error was 'no luid for requested privilege'"); goto out; } @@ -288,14 +289,14 @@ static void acquire_privilege(const char *name, Error **errp) priv.Privileges[0].Attributes = SE_PRIVILEGE_ENABLED; if (!AdjustTokenPrivileges(token, FALSE, , 0, NULL, 0)) { -error_setg(errp, QERR_QGA_COMMAND_FAILED, - "unable to acquire requested privilege"); +error_setg(errp, + "Guest agent command failed, error was 'unable to acquire requested privilege'"); goto out; } } else { -error_setg(errp, QERR_QGA_COMMAND_FAILED, - "failed to open privilege token"); +error_setg(errp, + "Guest agent command failed, error was 'failed to open privilege token'"); } out: @@ -309,8 +310,8 @@ static void execute_async(DWORD WINAPI (*func)(LPVOID), LPVOID opaque, { HANDLE thread = CreateThread(NULL, 0, func, opaque, 0, NULL); if (!thread) { -error_setg(errp, QERR_QGA_COMMAND_FAILED, - "failed to dispatch asynchronous command"); +error_setg(errp, + "Guest agent command failed, error was 'failed to dispatch asynchronous command'"); } } @@ -1423,22 +1424,22 @@ static void check_suspend_mode(GuestSuspendMode mode, Error **errp) ZeroMemory(_pwr_caps, sizeof(sys_pwr_caps)); if (!GetPwrCapabilities(_pwr_caps)) { -error_setg(errp, QERR_QGA_COMMAND_FAILED, - "failed to determine guest suspend capabilities"); +error_setg(errp, + "Guest agent command failed, error was 'failed to determine guest suspend capabilities'"); return; } switch (mode) { case GUEST_SUSPEND_MODE_DISK: if (!sys_pwr_caps.SystemS4) { -error_setg(errp, QERR_QGA_COMMAND_FAILED, - "suspend-to-disk not supported by OS"); +error_setg(errp, + "Guest agent command failed, error was 'suspend-to-disk not supported by OS'"); } break; case GUEST_SUSPEND_MODE_RAM: if (!sys_pwr_caps.SystemS3) { -error_setg(errp, QERR_QGA_COMMAND_FAILED, - "suspend-to-ram not supported by OS"); +error_setg(errp, + "Guest agent command failed, error was 'suspend-to-ram not supported by OS'"); } break; default: @@ -1971,7
[PATCH v2 15/22] qapi: Inline and remove QERR_MIGRATION_ACTIVE definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using sed, manually removing the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Juan Quintela --- include/qapi/qmp/qerror.h | 3 --- migration/migration.c | 2 +- migration/options.c | 4 ++-- migration/savevm.c| 2 +- 4 files changed, 4 insertions(+), 7 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index d95c4b84b9..cc4dae1076 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_MIGRATION_ACTIVE \ -"There's a migration process in progress" - #define QERR_MISSING_PARAMETER \ "Parameter '%s' is missing" diff --git a/migration/migration.c b/migration/migration.c index b7f6818a15..5703cc34ae 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -1616,7 +1616,7 @@ static bool migrate_prepare(MigrationState *s, bool blk, bool blk_inc, } if (migration_is_running(s->state)) { -error_setg(errp, QERR_MIGRATION_ACTIVE); +error_setg(errp, "There's a migration process in progress"); return false; } diff --git a/migration/options.c b/migration/options.c index 4f6c8e810c..7360a22252 100644 --- a/migration/options.c +++ b/migration/options.c @@ -618,7 +618,7 @@ bool migrate_cap_set(int cap, bool value, Error **errp) bool new_caps[MIGRATION_CAPABILITY__MAX]; if (migration_is_running(s->state)) { -error_setg(errp, QERR_MIGRATION_ACTIVE); +error_setg(errp, "There's a migration process in progress"); return false; } @@ -662,7 +662,7 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params, bool new_caps[MIGRATION_CAPABILITY__MAX]; if (migration_is_running(s->state) || migration_in_colo_state()) { -error_setg(errp, QERR_MIGRATION_ACTIVE); +error_setg(errp, "There's a migration process in progress"); return; } diff --git a/migration/savevm.c b/migration/savevm.c index 41c7f39ef5..c0e0585bc1 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -1634,7 +1634,7 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp) MigrationStatus status; if (migration_is_running(ms->state)) { -error_setg(errp, QERR_MIGRATION_ACTIVE); +error_setg(errp, "There's a migration process in progress"); return -EINVAL; } -- 2.41.0
[PATCH v2 19/22] qapi: Inline and remove QERR_PROPERTY_VALUE_OUT_OF_RANGE definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using sed, manually removing the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé --- include/qapi/qmp/qerror.h | 3 --- hw/intc/openpic.c | 3 ++- target/i386/cpu.c | 12 util/block-helpers.c | 3 ++- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index 7862ac55a1..e094f13114 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_PROPERTY_VALUE_OUT_OF_RANGE \ -"Property %s.%s doesn't take value %" PRId64 " (minimum: %" PRId64 ", maximum: %" PRId64 ")" - #define QERR_QGA_COMMAND_FAILED \ "Guest agent command failed, error was '%s'" diff --git a/hw/intc/openpic.c b/hw/intc/openpic.c index a6f91d4bcd..4f6ee930e2 100644 --- a/hw/intc/openpic.c +++ b/hw/intc/openpic.c @@ -1535,7 +1535,8 @@ static void openpic_realize(DeviceState *dev, Error **errp) }; if (opp->nb_cpus > MAX_CPU) { -error_setg(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, +error_setg(errp, "Property %s.%s doesn't take value %" PRId64 + " (minimum: %" PRId64 ", maximum: %" PRId64 ")", TYPE_OPENPIC, "nb_cpus", (uint64_t)opp->nb_cpus, (uint64_t)0, (uint64_t)MAX_CPU); return; diff --git a/target/i386/cpu.c b/target/i386/cpu.c index e5a14885ed..273f865228 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -5086,7 +5086,8 @@ static void x86_cpuid_version_set_family(Object *obj, Visitor *v, return; } if (value < min || value > max) { -error_setg(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "", +error_setg(errp, "Property %s doesn't take value %" PRId64 + " (minimum: %" PRId64 ", maximum: %" PRId64 ")", name ? name : "null", value, min, max); return; } @@ -5126,7 +5127,8 @@ static void x86_cpuid_version_set_model(Object *obj, Visitor *v, return; } if (value < min || value > max) { -error_setg(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "", +error_setg(errp, "Property %s doesn't take value %" PRId64 + " (minimum: %" PRId64 ", maximum: %" PRId64 ")", name ? name : "null", value, min, max); return; } @@ -5161,7 +5163,8 @@ static void x86_cpuid_version_set_stepping(Object *obj, Visitor *v, return; } if (value < min || value > max) { -error_setg(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "", +error_setg(errp, "Property %s doesn't take value %" PRId64 + " (minimum: %" PRId64 ", maximum: %" PRId64 ")", name ? name : "null", value, min, max); return; } @@ -5263,7 +5266,8 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, const char *name, return; } if (value < min || value > max) { -error_setg(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "", +error_setg(errp, "Property %s doesn't take value %" PRId64 + " (minimum: %" PRId64 ", maximum: %" PRId64 ")", name ? name : "null", value, min, max); return; } diff --git a/util/block-helpers.c b/util/block-helpers.c index c4851432f5..de94909bc4 100644 --- a/util/block-helpers.c +++ b/util/block-helpers.c @@ -30,7 +30,8 @@ void check_block_size(const char *id, const char *name, int64_t value, { /* value of 0 means "unset" */ if (value && (value < MIN_BLOCK_SIZE || value > MAX_BLOCK_SIZE)) { -error_setg(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, +error_setg(errp, "Property %s.%s doesn't take value %" PRId64 + " (minimum: %" PRId64 ", maximum: %" PRId64 ")", id, name, value, MIN_BLOCK_SIZE, MAX_BLOCK_SIZE); return; } -- 2.41.0
[PATCH v2 04/22] qapi: Inline and remove QERR_DEVICE_NO_HOTPLUG definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using sed, manually removing the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé --- include/qapi/qmp/qerror.h | 3 --- hw/core/qdev.c| 3 ++- softmmu/qdev-monitor.c| 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index daa889809b..e93211085a 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_DEVICE_NO_HOTPLUG \ -"Device '%s' does not support hotplugging" - #define QERR_INVALID_PARAMETER \ "Invalid parameter '%s'" diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 43d863b0c5..9b62e0573d 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -479,7 +479,8 @@ static void device_set_realized(Object *obj, bool value, Error **errp) static int unattached_count; if (dev->hotplugged && !dc->hotpluggable) { -error_setg(errp, QERR_DEVICE_NO_HOTPLUG, object_get_typename(obj)); +error_setg(errp, "Device '%s' does not support hotplugging", + object_get_typename(obj)); return; } diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c index 3a9740dcbd..a964bd80df 100644 --- a/softmmu/qdev-monitor.c +++ b/softmmu/qdev-monitor.c @@ -911,7 +911,7 @@ void qdev_unplug(DeviceState *dev, Error **errp) } if (!dc->hotpluggable) { -error_setg(errp, QERR_DEVICE_NO_HOTPLUG, +error_setg(errp, "Device '%s' does not support hotplugging", object_get_typename(OBJECT(dev))); return; } -- 2.41.0
[PATCH v2 12/22] qapi: Inline QERR_INVALID_PARAMETER_VALUE definition (constant param)
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using the following coccinelle semantic patch: @match@ identifier errp; expression param; // not constant constant value; @@ error_setg(errp, QERR_INVALID_PARAMETER_VALUE, param, value); @script:python strformat depends on match@ param << match.param; value << match.value; fixedfmt; // new var @@ fixedfmt = f"\"Parameter '%s' expects {value[1:-1]}\"" coccinelle.fixedfmt = cocci.make_ident(fixedfmt) @replace@ identifier match.errp; expression match.param; constant match.value; identifier strformat.fixedfmt; @@ -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, param, value); +error_setg(errp, fixedfmt, param); Signed-off-by: Philippe Mathieu-Daudé --- hw/core/qdev-properties-system.c | 5 +++-- qapi/opts-visitor.c | 3 +-- qapi/qapi-util.c | 3 +-- qapi/qobject-input-visitor.c | 18 -- qapi/string-input-visitor.c | 18 ++ util/qemu-option.c | 7 --- 6 files changed, 27 insertions(+), 27 deletions(-) diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c index 688340610e..7752c5fda5 100644 --- a/hw/core/qdev-properties-system.c +++ b/hw/core/qdev-properties-system.c @@ -769,8 +769,9 @@ static void set_pci_devfn(Object *obj, Visitor *v, const char *name, return; } if (value < -1 || value > 255) { -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, - name ? name : "null", "a value between -1 and 255"); +error_setg(errp, + "Parameter '%s' expects a value between -1 and 255", + name ? name : "null"); return; } *ptr = value; diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c index 3d1a28b419..0393704a73 100644 --- a/qapi/opts-visitor.c +++ b/qapi/opts-visitor.c @@ -515,8 +515,7 @@ opts_type_size(Visitor *v, const char *name, uint64_t *obj, Error **errp) err = qemu_strtosz(opt->str ? opt->str : "", NULL, obj); if (err < 0) { -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name, - "a size value"); +error_setg(errp, "Parameter '%s' expects a size value", opt->name); return false; } diff --git a/qapi/qapi-util.c b/qapi/qapi-util.c index 63596e11c5..82c3425566 100644 --- a/qapi/qapi-util.c +++ b/qapi/qapi-util.c @@ -101,8 +101,7 @@ bool qapi_bool_parse(const char *name, const char *value, bool *obj, Error **err return true; } -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name, - "'on' or 'off'"); +error_setg(errp, "Parameter '%s' expects 'on' or 'off'", name); return false; } diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c index f110a804b2..f5fa6c1878 100644 --- a/qapi/qobject-input-visitor.c +++ b/qapi/qobject-input-visitor.c @@ -424,8 +424,8 @@ static bool qobject_input_type_int64_keyval(Visitor *v, const char *name, if (qemu_strtoi64(str, NULL, 0, obj) < 0) { /* TODO report -ERANGE more nicely */ -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, - full_name(qiv, name), "integer"); +error_setg(errp, "Parameter '%s' expects integer", + full_name(qiv, name)); return false; } return true; @@ -458,8 +458,7 @@ static bool qobject_input_type_uint64(Visitor *v, const char *name, } err: -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, - full_name(qiv, name), "uint64"); +error_setg(errp, "Parameter '%s' expects uint64", full_name(qiv, name)); return false; } @@ -475,8 +474,8 @@ static bool qobject_input_type_uint64_keyval(Visitor *v, const char *name, if (qemu_strtou64(str, NULL, 0, obj) < 0) { /* TODO report -ERANGE more nicely */ -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, - full_name(qiv, name), "integer"); +error_setg(errp, "Parameter '%s' expects integer", + full_name(qiv, name)); return false; } return true; @@ -514,8 +513,8 @@ static bool qobject_input_type_bool_keyval(Visitor *v, const char *name, } if (!qapi_bool_parse(name, str, obj, NULL)) { -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, - full_name(qiv, name), "'on' or 'off'"); +error_setg(errp, "Parameter '%s' expects 'on' or 'off'", + full_name(qiv, name)); return false; } return true; @@ -643,8 +642,7 @@ static bool qobject_input_type_size_keyval(Visitor *v, const char *name, if (qemu_strtosz(str, NULL, obj) < 0) { /*
[RFC PATCH v2 21/22] qapi: Inline and remove QERR_UNSUPPORTED definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using: $ sed -i -e 's/QERR_UNSUPPORTED/"this feature or command is not currently supported"/' \ $(git grep -wl QERR_UNSUPPORTED) then manually removing the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé --- RFC: Not sure what is the best way to change the comment in qga/qapi-schema.json... --- qga/qapi-schema.json | 5 +++-- include/qapi/qmp/qerror.h | 3 --- qga/commands-bsd.c| 8 +++ qga/commands-posix.c | 46 +++ qga/commands-win32.c | 22 +-- 5 files changed, 41 insertions(+), 43 deletions(-) diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json index b720dd4379..51683f4dc2 100644 --- a/qga/qapi-schema.json +++ b/qga/qapi-schema.json @@ -6,8 +6,9 @@ # # "unsupported" is a higher-level error than the errors that # individual commands might document. The caller should always be -# prepared to receive QERR_UNSUPPORTED, even if the given command -# doesn't specify it, or doesn't document any failure mode at all. +# prepared to receive the "this feature or command is not currently supported" +# message, even if the given command doesn't specify it, or doesn't document +# any failure mode at all. ## ## diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index 840831cc6a..7606f4525d 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,7 +17,4 @@ * add new ones! */ -#define QERR_UNSUPPORTED \ -"this feature or command is not currently supported" - #endif /* QERROR_H */ diff --git a/qga/commands-bsd.c b/qga/commands-bsd.c index 17bddda1cf..11536f148f 100644 --- a/qga/commands-bsd.c +++ b/qga/commands-bsd.c @@ -152,25 +152,25 @@ int qmp_guest_fsfreeze_do_thaw(Error **errp) GuestFilesystemInfoList *qmp_guest_get_fsinfo(Error **errp) { -error_setg(errp, QERR_UNSUPPORTED); +error_setg(errp, "this feature or command is not currently supported"); return NULL; } GuestDiskInfoList *qmp_guest_get_disks(Error **errp) { -error_setg(errp, QERR_UNSUPPORTED); +error_setg(errp, "this feature or command is not currently supported"); return NULL; } GuestDiskStatsInfoList *qmp_guest_get_diskstats(Error **errp) { -error_setg(errp, QERR_UNSUPPORTED); +error_setg(errp, "this feature or command is not currently supported"); return NULL; } GuestCpuStatsList *qmp_guest_get_cpustats(Error **errp) { -error_setg(errp, QERR_UNSUPPORTED); +error_setg(errp, "this feature or command is not currently supported"); return NULL; } #endif /* CONFIG_FSFREEZE */ diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 6169bbf7a0..f510add366 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -165,7 +165,7 @@ void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp) } if (!hwclock_available) { -error_setg(errp, QERR_UNSUPPORTED); +error_setg(errp, "this feature or command is not currently supported"); return; } @@ -1540,7 +1540,7 @@ GuestDiskInfoList *qmp_guest_get_disks(Error **errp) GuestDiskInfoList *qmp_guest_get_disks(Error **errp) { -error_setg(errp, QERR_UNSUPPORTED); +error_setg(errp, "this feature or command is not currently supported"); return NULL; } @@ -2235,7 +2235,7 @@ void qmp_guest_set_user_password(const char *username, bool crypted, Error **errp) { -error_setg(errp, QERR_UNSUPPORTED); +error_setg(errp, "this feature or command is not currently supported"); } #endif /* __linux__ || __FreeBSD__ */ @@ -2751,47 +2751,47 @@ GuestCpuStatsList *qmp_guest_get_cpustats(Error **errp) void qmp_guest_suspend_disk(Error **errp) { -error_setg(errp, QERR_UNSUPPORTED); +error_setg(errp, "this feature or command is not currently supported"); } void qmp_guest_suspend_ram(Error **errp) { -error_setg(errp, QERR_UNSUPPORTED); +error_setg(errp, "this feature or command is not currently supported"); } void qmp_guest_suspend_hybrid(Error **errp) { -error_setg(errp, QERR_UNSUPPORTED); +error_setg(errp, "this feature or command is not currently supported"); } GuestLogicalProcessorList *qmp_guest_get_vcpus(Error **errp) { -error_setg(errp, QERR_UNSUPPORTED); +error_setg(errp, "this feature or command is not currently supported"); return NULL; } int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp) { -error_setg(errp, QERR_UNSUPPORTED); +error_setg(errp, "this feature or command is not currently supported"); return -1; } GuestMemoryBlockList *qmp_guest_get_memory_blocks(Error **errp) { -
[PATCH v2 08/22] qapi: Inline QERR_INVALID_PARAMETER_TYPE definition (constant value)
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using the following coccinelle semantic patch: @match@ expression errp; expression param; constant value; @@ error_setg(errp, QERR_INVALID_PARAMETER_TYPE, param, value); @script:python strformat depends on match@ value << match.value; fixedfmt; // new var @@ fixedfmt = f'"Invalid parameter type for \'%s\', expected: {value[1:-1]}"' coccinelle.fixedfmt = cocci.make_ident(fixedfmt) @replace@ expression match.errp; expression match.param; constant match.value; identifier strformat.fixedfmt; @@ -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, param, value); +error_setg(errp, fixedfmt, param); Signed-off-by: Philippe Mathieu-Daudé --- qapi/qobject-input-visitor.c | 32 qapi/string-input-visitor.c | 8 qom/object.c | 12 3 files changed, 28 insertions(+), 24 deletions(-) diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c index 3e8aca6b15..f110a804b2 100644 --- a/qapi/qobject-input-visitor.c +++ b/qapi/qobject-input-visitor.c @@ -288,8 +288,8 @@ static bool qobject_input_start_struct(Visitor *v, const char *name, void **obj, return false; } if (qobject_type(qobj) != QTYPE_QDICT) { -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, - full_name(qiv, name), "object"); +error_setg(errp, "Invalid parameter type for '%s', expected: object", + full_name(qiv, name)); return false; } @@ -326,8 +326,8 @@ static bool qobject_input_start_list(Visitor *v, const char *name, return false; } if (qobject_type(qobj) != QTYPE_QLIST) { -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, - full_name(qiv, name), "array"); +error_setg(errp, "Invalid parameter type for '%s', expected: array", + full_name(qiv, name)); return false; } @@ -405,8 +405,8 @@ static bool qobject_input_type_int64(Visitor *v, const char *name, int64_t *obj, } qnum = qobject_to(QNum, qobj); if (!qnum || !qnum_get_try_int(qnum, obj)) { -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, - full_name(qiv, name), "integer"); +error_setg(errp, "Invalid parameter type for '%s', expected: integer", + full_name(qiv, name)); return false; } return true; @@ -494,8 +494,8 @@ static bool qobject_input_type_bool(Visitor *v, const char *name, bool *obj, } qbool = qobject_to(QBool, qobj); if (!qbool) { -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, - full_name(qiv, name), "boolean"); +error_setg(errp, "Invalid parameter type for '%s', expected: boolean", + full_name(qiv, name)); return false; } @@ -534,8 +534,8 @@ static bool qobject_input_type_str(Visitor *v, const char *name, char **obj, } qstr = qobject_to(QString, qobj); if (!qstr) { -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, - full_name(qiv, name), "string"); +error_setg(errp, "Invalid parameter type for '%s', expected: string", + full_name(qiv, name)); return false; } @@ -565,8 +565,8 @@ static bool qobject_input_type_number(Visitor *v, const char *name, double *obj, } qnum = qobject_to(QNum, qobj); if (!qnum) { -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, - full_name(qiv, name), "number"); +error_setg(errp, "Invalid parameter type for '%s', expected: number", + full_name(qiv, name)); return false; } @@ -587,8 +587,8 @@ static bool qobject_input_type_number_keyval(Visitor *v, const char *name, if (qemu_strtod_finite(str, NULL, )) { /* TODO report -ERANGE more nicely */ -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, - full_name(qiv, name), "number"); +error_setg(errp, "Invalid parameter type for '%s', expected: number", + full_name(qiv, name)); return false; } @@ -623,8 +623,8 @@ static bool qobject_input_type_null(Visitor *v, const char *name, } if (qobject_type(qobj) != QTYPE_QNULL) { -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, - full_name(qiv, name), "null"); +error_setg(errp, "Invalid parameter type for '%s', expected: null", + full_name(qiv, name)); return false; } *obj = qnull(); diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c index 197139c1c0..3f1b9e9b41 100644 --- a/qapi/string-input-visitor.c +++
[PATCH v2 03/22] qapi: Inline and remove QERR_DEVICE_IN_USE definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using sed, manually removing the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé --- include/qapi/qmp/qerror.h | 3 --- blockdev.c| 2 +- chardev/char-fe.c | 2 +- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index 168177bcd7..daa889809b 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_DEVICE_IN_USE \ -"Device '%s' is in use" - #define QERR_DEVICE_NO_HOTPLUG \ "Device '%s' does not support hotplugging" diff --git a/blockdev.c b/blockdev.c index e5617faf0f..da39da457e 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2345,7 +2345,7 @@ void coroutine_fn qmp_block_resize(const char *device, const char *node_name, } if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_RESIZE, NULL)) { -error_setg(errp, QERR_DEVICE_IN_USE, device); +error_setg(errp, "Device '%s' is in use", device); return; } diff --git a/chardev/char-fe.c b/chardev/char-fe.c index 7789f7be9c..7d33b3ccd1 100644 --- a/chardev/char-fe.c +++ b/chardev/char-fe.c @@ -217,7 +217,7 @@ bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp) return true; unavailable: -error_setg(errp, QERR_DEVICE_IN_USE, s->label); +error_setg(errp, "Device '%s' is in use", s->label); return false; } -- 2.41.0
[PATCH v2 22/22] qapi: Remove 'qapi/qmp/qerror.h' header
This file is now empty. Avoid new definitions by killing it, paying off a 8 years old debt (with interests). Mechanical change using: $ git grep -l qapi/qmp/qerror.h | while read f; do \ gawk -i inplace '/#include "qapi\/qmp\/qerror.h"/ && !p {p++;next}1' $f; \ done $ git rm include/qapi/qmp/qerror.h Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Juan Quintela --- include/qapi/qmp/qerror.h| 20 backends/cryptodev-vhost-user.c | 1 - backends/dbus-vmstate.c | 1 - backends/rng-egd.c | 1 - backends/rng-random.c| 1 - block/gluster.c | 1 - block/monitor/block-hmp-cmds.c | 1 - block/quorum.c | 1 - block/snapshot.c | 1 - block/vmdk.c | 1 - blockdev.c | 1 - blockjob.c | 1 - chardev/char-fe.c| 1 - chardev/char.c | 1 - dump/dump.c | 1 - dump/win_dump.c | 1 - hw/core/qdev-properties-system.c | 1 - hw/core/qdev-properties.c| 1 - hw/core/qdev.c | 1 - hw/intc/openpic.c| 1 - hw/ppc/spapr_pci.c | 1 - hw/usb/redirect.c| 1 - migration/migration.c| 1 - migration/options.c | 1 - migration/page_cache.c | 1 - migration/ram.c | 1 - migration/savevm.c | 1 - monitor/fds.c| 1 - monitor/hmp-cmds.c | 1 - monitor/qmp-cmds.c | 1 - net/filter-buffer.c | 1 - net/filter.c | 1 - net/net.c| 1 - qapi/opts-visitor.c | 1 - qapi/qapi-forward-visitor.c | 1 - qapi/qapi-util.c | 1 - qapi/qapi-visit-core.c | 1 - qapi/qobject-input-visitor.c | 1 - qapi/string-input-visitor.c | 1 - qga/commands-bsd.c | 1 - qga/commands-posix.c | 1 - qga/commands-win32.c | 1 - qga/commands.c | 1 - qom/object.c | 1 - qom/object_interfaces.c | 1 - qom/qom-qmp-cmds.c | 1 - softmmu/balloon.c| 1 - softmmu/cpus.c | 1 - softmmu/qdev-monitor.c | 1 - softmmu/rtc.c| 1 - softmmu/tpm.c| 1 - softmmu/vl.c | 1 - target/arm/arm-qmp-cmds.c| 1 - target/i386/cpu.c| 1 - target/s390x/cpu_models_sysemu.c | 1 - ui/input-barrier.c | 1 - ui/ui-qmp-cmds.c | 1 - util/block-helpers.c | 1 - util/qemu-option.c | 1 - scripts/qapi/visit.py| 1 - 60 files changed, 79 deletions(-) delete mode 100644 include/qapi/qmp/qerror.h diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h deleted file mode 100644 index 7606f4525d..00 --- a/include/qapi/qmp/qerror.h +++ /dev/null @@ -1,20 +0,0 @@ -/* - * QError Module - * - * Copyright (C) 2009 Red Hat Inc. - * - * Authors: - * Luiz Capitulino - * - * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. - * See the COPYING.LIB file in the top-level directory. - */ -#ifndef QERROR_H -#define QERROR_H - -/* - * These macros will go away, please don't use in new code, and do not - * add new ones! - */ - -#endif /* QERROR_H */ diff --git a/backends/cryptodev-vhost-user.c b/backends/cryptodev-vhost-user.c index d93ccd5528..5a41aa7be8 100644 --- a/backends/cryptodev-vhost-user.c +++ b/backends/cryptodev-vhost-user.c @@ -23,7 +23,6 @@ #include "qemu/osdep.h" #include "qapi/error.h" -#include "qapi/qmp/qerror.h" #include "qemu/error-report.h" #include "hw/virtio/vhost-user.h" #include "standard-headers/linux/virtio_crypto.h" diff --git a/backends/dbus-vmstate.c b/backends/dbus-vmstate.c index e781ded17c..0006f8c400 100644 --- a/backends/dbus-vmstate.c +++ b/backends/dbus-vmstate.c @@ -16,7 +16,6 @@ #include "qemu/error-report.h" #include "qapi/error.h" #include "qom/object_interfaces.h" -#include "qapi/qmp/qerror.h" #include "migration/vmstate.h" #include "trace.h" #include "qom/object.h" diff --git a/backends/rng-egd.c b/backends/rng-egd.c index 8f101afadc..35f19257bd 100644 --- a/backends/rng-egd.c +++ b/backends/rng-egd.c @@ -14,7 +14,6 @@ #include "sysemu/rng.h" #include "chardev/char-fe.h" #include "qapi/error.h" -#include "qapi/qmp/qerror.h" #include "qemu/module.h" #include "qom/object.h" diff --git a/backends/rng-random.c b/backends/rng-random.c index 9cb7d26cb5..a49e4a4970 100644 --- a/backends/rng-random.c +++ b/backends/rng-random.c @@ -14,7 +14,6 @@ #include "sysemu/rng-random.h" #include "sysemu/rng.h" #include "qapi/error.h" -#include "qapi/qmp/qerror.h" #include "qemu/main-loop.h" #include "qemu/module.h" diff
[PATCH v2 11/22] qapi: Inline QERR_INVALID_PARAMETER_VALUE definition (constant value)
vcpu_dirty_limit Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using the following coccinelle semantic patch: @match@ expression errp; constant param; constant value; @@ error_setg(errp, QERR_INVALID_PARAMETER_VALUE, param, value); @script:python strformat depends on match@ param << match.param; value << match.value; fixedfmt; // new var @@ fixedfmt = "\"Parameter '%s' expects %s\"" % (param[1:-1], value[1:-1]) coccinelle.fixedfmt = cocci.make_ident(fixedfmt) @replace@ expression match.errp; constant match.param; constant match.value; identifier strformat.fixedfmt; @@ -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, param, value); +error_setg(errp, fixedfmt); Then manually splitting lines over 90 characters. Reviewed-by: Juan Quintela Signed-off-by: Philippe Mathieu-Daudé --- backends/cryptodev-vhost-user.c | 4 +- backends/rng-egd.c | 4 +- backends/rng-random.c | 3 +- block/quorum.c | 3 +- blockdev.c | 9 ++-- blockjob.c | 3 +- chardev/char.c | 3 +- hw/usb/redirect.c | 4 +- migration/migration.c | 4 +- migration/options.c | 92 ++--- migration/page_cache.c | 8 +-- migration/ram.c | 4 +- monitor/fds.c | 8 +-- monitor/qmp-cmds.c | 3 +- net/filter-buffer.c | 3 +- net/filter.c| 7 ++- net/net.c | 9 ++-- qga/commands-win32.c| 4 +- qom/object_interfaces.c | 2 +- qom/qom-qmp-cmds.c | 7 ++- softmmu/balloon.c | 2 +- softmmu/cpus.c | 3 +- softmmu/qdev-monitor.c | 11 ++-- ui/ui-qmp-cmds.c| 2 +- util/qemu-option.c | 3 +- 25 files changed, 89 insertions(+), 116 deletions(-) diff --git a/backends/cryptodev-vhost-user.c b/backends/cryptodev-vhost-user.c index c3283ba84a..d93ccd5528 100644 --- a/backends/cryptodev-vhost-user.c +++ b/backends/cryptodev-vhost-user.c @@ -136,8 +136,8 @@ cryptodev_vhost_claim_chardev(CryptoDevBackendVhostUser *s, Chardev *chr; if (s->chr_name == NULL) { -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, - "chardev", "a valid character device"); +error_setg(errp, + "Parameter 'chardev' expects a valid character device"); return NULL; } diff --git a/backends/rng-egd.c b/backends/rng-egd.c index 684c3cf3d6..8f101afadc 100644 --- a/backends/rng-egd.c +++ b/backends/rng-egd.c @@ -90,8 +90,8 @@ static void rng_egd_opened(RngBackend *b, Error **errp) Chardev *chr; if (s->chr_name == NULL) { -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, - "chardev", "a valid character device"); +error_setg(errp, + "Parameter 'chardev' expects a valid character device"); return; } diff --git a/backends/rng-random.c b/backends/rng-random.c index 80eb5be138..9cb7d26cb5 100644 --- a/backends/rng-random.c +++ b/backends/rng-random.c @@ -72,8 +72,7 @@ static void rng_random_opened(RngBackend *b, Error **errp) RngRandom *s = RNG_RANDOM(b); if (s->filename == NULL) { -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, - "filename", "a valid filename"); +error_setg(errp, "Parameter 'filename' expects a valid filename"); } else { s->fd = qemu_open_old(s->filename, O_RDONLY | O_NONBLOCK); if (s->fd == -1) { diff --git a/block/quorum.c b/block/quorum.c index 05220cab7f..8e9f279568 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -882,8 +882,7 @@ static int quorum_valid_threshold(int threshold, int num_children, Error **errp) { if (threshold < 1) { -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, - "vote-threshold", "a value >= 1"); +error_setg(errp, "Parameter 'vote-threshold' expects a value >= 1"); return -ERANGE; } diff --git a/blockdev.c b/blockdev.c index da39da457e..ed90365329 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2340,7 +2340,7 @@ void coroutine_fn qmp_block_resize(const char *device, const char *node_name, } if (size < 0) { -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "size", "a >0 size"); +error_setg(errp, "Parameter 'size' expects a >0 size"); return; } @@ -2905,13 +2905,12 @@ static void blockdev_mirror_common(const char *job_id, BlockDriverState *bs, } if (granularity != 0 && (granularity < 512 || granularity > 1048576 * 64)) { -error_setg(errp,
[PATCH v2 16/22] qapi: Inline QERR_MISSING_PARAMETER definition (constant parameter)
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using the following coccinelle semantic patches: @match@ expression errp; constant param; @@ error_setg(errp, QERR_MISSING_PARAMETER, param); @script:python strformat depends on match@ param << match.param; fixedfmt; // new var @@ if param[0] == '"': # Format skipping '"', fixedfmt = f'"Parameter \'{param[1:-1]}\' is missing"' else: # or use definition. fixedfmt = f'"Parameter " {param} " is missing"' coccinelle.fixedfmt = cocci.make_ident(fixedfmt) @replace@ expression match.errp; constant match.param; identifier strformat.fixedfmt; @@ -error_setg(errp, QERR_MISSING_PARAMETER, param); +error_setg(errp, fixedfmt); and: @match@ constant param; @@ error_report(QERR_MISSING_PARAMETER, param); @script:python strformat depends on match@ param << match.param; fixedfmt; // new var @@ fixedfmt = f'"Parameter \'{param[1:-1]}\' is missing"' coccinelle.fixedfmt = cocci.make_ident(fixedfmt) @replace@ constant match.param; identifier strformat.fixedfmt; @@ -error_report(QERR_MISSING_PARAMETER, param); +error_report(fixedfmt); Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Stefan Berger --- backends/dbus-vmstate.c| 2 +- block/gluster.c| 21 +++-- block/monitor/block-hmp-cmds.c | 6 +++--- dump/dump.c| 4 ++-- hw/usb/redirect.c | 2 +- softmmu/qdev-monitor.c | 2 +- softmmu/tpm.c | 4 ++-- softmmu/vl.c | 4 ++-- ui/input-barrier.c | 2 +- ui/ui-qmp-cmds.c | 2 +- 10 files changed, 25 insertions(+), 24 deletions(-) diff --git a/backends/dbus-vmstate.c b/backends/dbus-vmstate.c index 57369ec0f2..e781ded17c 100644 --- a/backends/dbus-vmstate.c +++ b/backends/dbus-vmstate.c @@ -413,7 +413,7 @@ dbus_vmstate_complete(UserCreatable *uc, Error **errp) } if (!self->dbus_addr) { -error_setg(errp, QERR_MISSING_PARAMETER, "addr"); +error_setg(errp, "Parameter 'addr' is missing"); return; } diff --git a/block/gluster.c b/block/gluster.c index ad5fadbe79..8d97d698c3 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -530,20 +530,20 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf, num_servers = qdict_array_entries(options, GLUSTER_OPT_SERVER_PATTERN); if (num_servers < 1) { -error_setg(_err, QERR_MISSING_PARAMETER, "server"); +error_setg(_err, "Parameter 'server' is missing"); goto out; } ptr = qemu_opt_get(opts, GLUSTER_OPT_VOLUME); if (!ptr) { -error_setg(_err, QERR_MISSING_PARAMETER, GLUSTER_OPT_VOLUME); +error_setg(_err, "Parameter " GLUSTER_OPT_VOLUME " is missing"); goto out; } gconf->volume = g_strdup(ptr); ptr = qemu_opt_get(opts, GLUSTER_OPT_PATH); if (!ptr) { -error_setg(_err, QERR_MISSING_PARAMETER, GLUSTER_OPT_PATH); +error_setg(_err, "Parameter " GLUSTER_OPT_PATH " is missing"); goto out; } gconf->path = g_strdup(ptr); @@ -562,7 +562,8 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf, ptr = qemu_opt_get(opts, GLUSTER_OPT_TYPE); if (!ptr) { -error_setg(_err, QERR_MISSING_PARAMETER, GLUSTER_OPT_TYPE); +error_setg(_err, + "Parameter " GLUSTER_OPT_TYPE " is missing"); error_append_hint(_err, GERR_INDEX_HINT, i); goto out; @@ -592,16 +593,16 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf, ptr = qemu_opt_get(opts, GLUSTER_OPT_HOST); if (!ptr) { -error_setg(_err, QERR_MISSING_PARAMETER, - GLUSTER_OPT_HOST); +error_setg(_err, + "Parameter " GLUSTER_OPT_HOST " is missing"); error_append_hint(_err, GERR_INDEX_HINT, i); goto out; } gsconf->u.inet.host = g_strdup(ptr); ptr = qemu_opt_get(opts, GLUSTER_OPT_PORT); if (!ptr) { -error_setg(_err, QERR_MISSING_PARAMETER, - GLUSTER_OPT_PORT); +error_setg(_err, + "Parameter " GLUSTER_OPT_PORT " is missing"); error_append_hint(_err, GERR_INDEX_HINT, i); goto out; } @@ -648,8 +649,8 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf, goto out; } if (!ptr) { -error_setg(_err,
[PATCH v2 09/22] qapi: Inline and remove QERR_INVALID_PARAMETER_TYPE definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Manual changes (escaping the format in qapi/visit.py). Remove the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé --- include/qapi/qmp/qerror.h | 3 --- qom/object.c | 3 ++- scripts/qapi/visit.py | 4 ++-- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index 63ab775dc5..b723830eff 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_INVALID_PARAMETER_TYPE \ -"Invalid parameter type for '%s', expected: %s" - #define QERR_INVALID_PARAMETER_VALUE \ "Parameter '%s' expects %s" diff --git a/qom/object.c b/qom/object.c index 890fa0a106..eea61a5068 100644 --- a/qom/object.c +++ b/qom/object.c @@ -1855,7 +1855,8 @@ static Object *object_resolve_link(Object *obj, const char *name, } else if (!target) { target = object_resolve_path(path, ); if (target || ambiguous) { -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name, target_type); +error_setg(errp, "Invalid parameter type for '%s', expected: %s", + name, target_type); } else { error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, "Device '%s' not found", path); diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py index c56ea4d724..4b4a442383 100644 --- a/scripts/qapi/visit.py +++ b/scripts/qapi/visit.py @@ -278,8 +278,8 @@ def gen_visit_alternate(name: str, variants: QAPISchemaVariants) -> str: abort(); default: assert(visit_is_input(v)); -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "%(name)s"); +error_setg(errp, "Invalid parameter type for '%%s', expected: %(name)s", + name ? name : "null"); /* Avoid passing invalid *obj to qapi_free_%(c_name)s() */ g_free(*obj); *obj = NULL; -- 2.41.0
[PATCH v2 00/22] qapi: Kill 'qapi/qmp/qerror.h' for good
Since v1: - Fixed checkpatch warnings (Juan) - Added R-b tags - New patch for 'vcpu_dirty_limit' Hi, This is kind of a selfish series. I'm really tired to grep and read this comment from 2015 in qapi/qmp/qerror.h: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Besides, these definitions are still added in recent code (see for example commit 09f9ec9913 from June 2023). So let's finish with this 8 years old technical debt. Overall it took me 3h: 1h to find the correct Coccinelle doc about Python use and read it again [*], then 1h to adapt the script for each patch, rest is testing and writing comments, so the scripts used could be used as reference later. Regards, Phil. [*] https://www.lrz.de/services/compute/courses/x_lecturenotes/hspc1w19.pdf Philippe Mathieu-Daudé (22): qapi: Inline and remove QERR_BUS_NO_HOTPLUG definition qapi: Inline and remove QERR_DEVICE_HAS_NO_MEDIUM definition qapi: Inline and remove QERR_DEVICE_IN_USE definition qapi: Inline and remove QERR_DEVICE_NO_HOTPLUG definition qapi: Inline QERR_INVALID_PARAMETER definition (constant parameter) qapi: Inline and remove QERR_INVALID_PARAMETER definition qapi: Inline QERR_INVALID_PARAMETER_TYPE definition (constant param) qapi: Inline QERR_INVALID_PARAMETER_TYPE definition (constant value) qapi: Inline and remove QERR_INVALID_PARAMETER_TYPE definition qapi: Correct error message for 'vcpu_dirty_limit' parameter qapi: Inline QERR_INVALID_PARAMETER_VALUE definition (constant value) qapi: Inline QERR_INVALID_PARAMETER_VALUE definition (constant param) qapi: Inline and remove QERR_INVALID_PARAMETER_VALUE definition qapi: Inline and remove QERR_IO_ERROR definition qapi: Inline and remove QERR_MIGRATION_ACTIVE definition qapi: Inline QERR_MISSING_PARAMETER definition (constant parameter) qapi: Inline and remove QERR_MISSING_PARAMETER definition qapi: Inline and remove QERR_PROPERTY_VALUE_BAD definition qapi: Inline and remove QERR_PROPERTY_VALUE_OUT_OF_RANGE definition qapi: Inline and remove QERR_QGA_COMMAND_FAILED definition RFC qapi: Inline and remove QERR_UNSUPPORTED definition qapi: Remove 'qapi/qmp/qerror.h' header qga/qapi-schema.json | 5 +- include/qapi/qmp/qerror.h| 62 --- backends/cryptodev-vhost-user.c | 5 +- backends/dbus-vmstate.c | 3 +- backends/rng-egd.c | 5 +- backends/rng-random.c| 4 +- block/gluster.c | 22 +++ block/monitor/block-hmp-cmds.c | 7 +-- block/quorum.c | 4 +- block/snapshot.c | 5 +- block/vmdk.c | 9 ++- blockdev.c | 16 +++-- blockjob.c | 4 +- chardev/char-fe.c| 3 +- chardev/char.c | 4 +- dump/dump.c | 11 ++-- dump/win_dump.c | 5 +- hw/core/qdev-properties-system.c | 6 +- hw/core/qdev-properties.c| 3 +- hw/core/qdev.c | 4 +- hw/intc/openpic.c| 4 +- hw/ppc/spapr_pci.c | 5 +- hw/usb/redirect.c| 7 +-- migration/migration.c| 7 +-- migration/options.c | 102 +-- migration/page_cache.c | 9 ++- migration/ram.c | 5 +- migration/savevm.c | 7 +-- monitor/fds.c| 9 ++- monitor/hmp-cmds.c | 3 +- monitor/qmp-cmds.c | 4 +- net/filter-buffer.c | 4 +- net/filter.c | 8 +-- net/net.c| 10 ++- qapi/opts-visitor.c | 12 ++-- qapi/qapi-forward-visitor.c | 3 +- qapi/qapi-util.c | 4 +- qapi/qapi-visit-core.c | 5 +- qapi/qobject-input-visitor.c | 53 qapi/string-input-visitor.c | 27 qga/commands-bsd.c | 9 ++- qga/commands-posix.c | 47 +++--- qga/commands-win32.c | 65 ++-- qga/commands.c | 10 +-- qom/object.c | 16 +++-- qom/object_interfaces.c | 3 +- qom/qom-qmp-cmds.c | 8 +-- softmmu/balloon.c| 3 +- softmmu/cpus.c | 8 +-- softmmu/qdev-monitor.c | 24 softmmu/rtc.c| 1 - softmmu/tpm.c| 8 +-- softmmu/vl.c | 5 +- target/arm/arm-qmp-cmds.c| 4 +- target/i386/cpu.c| 15 +++-- target/s390x/cpu_models_sysemu.c | 4 +- ui/input-barrier.c | 3 +- ui/ui-qmp-cmds.c | 7 +-- util/block-helpers.c | 4 +- util/qemu-option.c | 21 +++ scripts/qapi/visit.py|
[PATCH v2 14/22] qapi: Inline and remove QERR_IO_ERROR definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using: $ sed -i -e 's/QERR_IO_ERROR/"An IO error has occurred"/' \ $(git grep -wl QERR_IO_ERROR) then manually removing the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Juan Quintela --- include/qapi/qmp/qerror.h | 3 --- block/vmdk.c | 8 blockdev.c| 2 +- dump/win_dump.c | 4 ++-- migration/savevm.c| 4 ++-- softmmu/cpus.c| 4 ++-- 6 files changed, 11 insertions(+), 14 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index ac727d1c2d..d95c4b84b9 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_IO_ERROR \ -"An IO error has occurred" - #define QERR_MIGRATION_ACTIVE \ "There's a migration process in progress" diff --git a/block/vmdk.c b/block/vmdk.c index e90649c8bf..6779a181f0 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -2246,12 +2246,12 @@ vmdk_init_extent(BlockBackend *blk, int64_t filesize, bool flat, bool compress, /* write all the data */ ret = blk_co_pwrite(blk, 0, sizeof(magic), , 0); if (ret < 0) { -error_setg(errp, QERR_IO_ERROR); +error_setg(errp, "An IO error has occurred"); goto exit; } ret = blk_co_pwrite(blk, sizeof(magic), sizeof(header), , 0); if (ret < 0) { -error_setg(errp, QERR_IO_ERROR); +error_setg(errp, "An IO error has occurred"); goto exit; } @@ -2271,7 +2271,7 @@ vmdk_init_extent(BlockBackend *blk, int64_t filesize, bool flat, bool compress, ret = blk_co_pwrite(blk, le64_to_cpu(header.rgd_offset) * BDRV_SECTOR_SIZE, gd_buf_size, gd_buf, 0); if (ret < 0) { -error_setg(errp, QERR_IO_ERROR); +error_setg(errp, "An IO error has occurred"); goto exit; } @@ -2283,7 +2283,7 @@ vmdk_init_extent(BlockBackend *blk, int64_t filesize, bool flat, bool compress, ret = blk_co_pwrite(blk, le64_to_cpu(header.gd_offset) * BDRV_SECTOR_SIZE, gd_buf_size, gd_buf, 0); if (ret < 0) { -error_setg(errp, QERR_IO_ERROR); +error_setg(errp, "An IO error has occurred"); } ret = 0; diff --git a/blockdev.c b/blockdev.c index ed90365329..228cebf9a2 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1433,7 +1433,7 @@ static void external_snapshot_action(TransactionAction *action, if (!bdrv_is_read_only(state->old_bs)) { if (bdrv_flush(state->old_bs)) { -error_setg(errp, QERR_IO_ERROR); +error_setg(errp, "An IO error has occurred"); goto out; } } diff --git a/dump/win_dump.c b/dump/win_dump.c index b7bfaff379..0115a609e0 100644 --- a/dump/win_dump.c +++ b/dump/win_dump.c @@ -67,7 +67,7 @@ static size_t write_run(uint64_t base_page, uint64_t page_count, l = qemu_write_full(fd, buf, len); cpu_physical_memory_unmap(buf, addr, false, len); if (l != len) { -error_setg(errp, QERR_IO_ERROR); +error_setg(errp, "An IO error has occurred"); return 0; } @@ -459,7 +459,7 @@ void create_win_dump(DumpState *s, Error **errp) s->written_size = qemu_write_full(s->fd, h, hdr_size); if (s->written_size != hdr_size) { -error_setg(errp, QERR_IO_ERROR); +error_setg(errp, "An IO error has occurred"); goto out_restore; } diff --git a/migration/savevm.c b/migration/savevm.c index bb3e99194c..41c7f39ef5 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -3099,7 +3099,7 @@ void qmp_xen_save_devices_state(const char *filename, bool has_live, bool live, object_unref(OBJECT(ioc)); ret = qemu_save_device_state(f); if (ret < 0 || qemu_fclose(f) < 0) { -error_setg(errp, QERR_IO_ERROR); +error_setg(errp, "An IO error has occurred"); } else { /* libxl calls the QMP command "stop" before calling * "xen-save-devices-state" and in case of migration failure, libxl @@ -3148,7 +3148,7 @@ void qmp_xen_load_devices_state(const char *filename, Error **errp) ret = qemu_loadvm_state(f); qemu_fclose(f); if (ret < 0) { -error_setg(errp, QERR_IO_ERROR); +error_setg(errp, "An IO error has occurred"); } migration_incoming_state_destroy(); } diff --git a/softmmu/cpus.c b/softmmu/cpus.c index 7fc70f9166..f7c743b0ce 100644 --- a/softmmu/cpus.c +++ b/softmmu/cpus.c @@ -773,7 +773,7 @@ void qmp_memsave(int64_t addr, int64_t size, const char *filename, goto exit; } if (fwrite(buf, 1, l, f) != l) { -error_setg(errp, QERR_IO_ERROR); +
[PATCH v2 05/22] qapi: Inline QERR_INVALID_PARAMETER definition (constant parameter)
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using the following coccinelle semantic patch: @match@ expression errp; constant param; @@ error_setg(errp, QERR_INVALID_PARAMETER, param); @script:python strformat depends on match@ param << match.param; fixedfmt; // new var @@ fixedfmt = f'"Invalid parameter \'{param[1:-1]}\'"' # Format skipping '"'. coccinelle.fixedfmt = cocci.make_ident(fixedfmt) @replace@ expression match.errp; constant match.param; identifier strformat.fixedfmt; @@ -error_setg(errp, QERR_INVALID_PARAMETER, param); +error_setg(errp, fixedfmt); Signed-off-by: Philippe Mathieu-Daudé --- dump/dump.c| 6 +++--- qga/commands.c | 2 +- ui/ui-qmp-cmds.c | 2 +- util/qemu-option.c | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/dump/dump.c b/dump/dump.c index d4ef713cd0..e173f1f14c 100644 --- a/dump/dump.c +++ b/dump/dump.c @@ -1810,7 +1810,7 @@ static void dump_init(DumpState *s, int fd, bool has_format, s->fd = fd; if (has_filter && !length) { -error_setg(errp, QERR_INVALID_PARAMETER, "length"); +error_setg(errp, "Invalid parameter 'length'"); goto cleanup; } s->filter_area_begin = begin; @@ -1841,7 +1841,7 @@ static void dump_init(DumpState *s, int fd, bool has_format, /* Is the filter filtering everything? */ if (validate_start_block(s) == -1) { -error_setg(errp, QERR_INVALID_PARAMETER, "begin"); +error_setg(errp, "Invalid parameter 'begin'"); goto cleanup; } @@ -2145,7 +2145,7 @@ void qmp_dump_guest_memory(bool paging, const char *file, } if (fd == -1) { -error_setg(errp, QERR_INVALID_PARAMETER, "protocol"); +error_setg(errp, "Invalid parameter 'protocol'"); return; } diff --git a/qga/commands.c b/qga/commands.c index 09c683e263..871210ab0b 100644 --- a/qga/commands.c +++ b/qga/commands.c @@ -154,7 +154,7 @@ GuestExecStatus *qmp_guest_exec_status(int64_t pid, Error **errp) gei = guest_exec_info_find(pid); if (gei == NULL) { -error_setg(errp, QERR_INVALID_PARAMETER, "pid"); +error_setg(errp, "Invalid parameter 'pid'"); return NULL; } diff --git a/ui/ui-qmp-cmds.c b/ui/ui-qmp-cmds.c index debc07d678..41ca0100e7 100644 --- a/ui/ui-qmp-cmds.c +++ b/ui/ui-qmp-cmds.c @@ -44,7 +44,7 @@ void qmp_set_password(SetPasswordOptions *opts, Error **errp) assert(opts->protocol == DISPLAY_PROTOCOL_VNC); if (opts->connected != SET_PASSWORD_ACTION_KEEP) { /* vnc supports "connected=keep" only */ -error_setg(errp, QERR_INVALID_PARAMETER, "connected"); +error_setg(errp, "Invalid parameter 'connected'"); return; } /* diff --git a/util/qemu-option.c b/util/qemu-option.c index eedd08929b..fb391a7904 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -612,7 +612,7 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id, if (list->merge_lists) { if (id) { -error_setg(errp, QERR_INVALID_PARAMETER, "id"); +error_setg(errp, "Invalid parameter 'id'"); return NULL; } opts = qemu_opts_find(list, NULL); -- 2.41.0
[PATCH v2 06/22] qapi: Inline and remove QERR_INVALID_PARAMETER definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using: $ sed -i -e "s/QERR_INVALID_PARAMETER,/\"Invalid parameter '%s'\",/" \ $(git grep -lw QERR_INVALID_PARAMETER) then manually removing the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé --- include/qapi/qmp/qerror.h | 3 --- monitor/hmp-cmds.c| 2 +- qapi/opts-visitor.c | 2 +- util/qemu-option.c| 8 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index e93211085a..63ab775dc5 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_INVALID_PARAMETER \ -"Invalid parameter '%s'" - #define QERR_INVALID_PARAMETER_TYPE \ "Invalid parameter type for '%s', expected: %s" diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c index 6c559b48c8..9d6533643d 100644 --- a/monitor/hmp-cmds.c +++ b/monitor/hmp-cmds.c @@ -138,7 +138,7 @@ void hmp_sync_profile(Monitor *mon, const QDict *qdict) } else { Error *err = NULL; -error_setg(, QERR_INVALID_PARAMETER, op); +error_setg(, "Invalid parameter '%s'", op); hmp_handle_error(mon, err); } } diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c index 8f1efab8b9..3d1a28b419 100644 --- a/qapi/opts-visitor.c +++ b/qapi/opts-visitor.c @@ -184,7 +184,7 @@ opts_check_struct(Visitor *v, Error **errp) const QemuOpt *first; first = g_queue_peek_head(any); -error_setg(errp, QERR_INVALID_PARAMETER, first->name); +error_setg(errp, "Invalid parameter '%s'", first->name); return false; } return true; diff --git a/util/qemu-option.c b/util/qemu-option.c index fb391a7904..201f7a87f3 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -498,7 +498,7 @@ static bool opt_validate(QemuOpt *opt, Error **errp) desc = find_desc_by_name(list->desc, opt->name); if (!desc && !opts_accepts_any(list)) { -error_setg(errp, QERR_INVALID_PARAMETER, opt->name); +error_setg(errp, "Invalid parameter '%s'", opt->name); return false; } @@ -531,7 +531,7 @@ bool qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val, desc = find_desc_by_name(list->desc, name); if (!desc && !opts_accepts_any(list)) { -error_setg(errp, QERR_INVALID_PARAMETER, name); +error_setg(errp, "Invalid parameter '%s'", name); return false; } @@ -554,7 +554,7 @@ bool qemu_opt_set_number(QemuOpts *opts, const char *name, int64_t val, desc = find_desc_by_name(list->desc, name); if (!desc && !opts_accepts_any(list)) { -error_setg(errp, QERR_INVALID_PARAMETER, name); +error_setg(errp, "Invalid parameter '%s'", name); return false; } @@ -1103,7 +1103,7 @@ bool qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc, Error **errp) QTAILQ_FOREACH(opt, >head, next) { opt->desc = find_desc_by_name(desc, opt->name); if (!opt->desc) { -error_setg(errp, QERR_INVALID_PARAMETER, opt->name); +error_setg(errp, "Invalid parameter '%s'", opt->name); return false; } -- 2.41.0
Re: [PULL 0/9] Python patches
Hi John, On 4/10/23 21:46, John Snow wrote: The following changes since commit da1034094d375afe9e3d8ec8980550ea0f06f7e0: Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging (2023-10-03 07:43:44 -0400) are available in the Git repository at: https://gitlab.com/jsnow/qemu.git tags/python-pull-request for you to fetch changes up to 4d7a663cbe8343e884b88e44bd88d37dd0a470e5: Python: test Python 3.12 (2023-10-04 15:19:00 -0400) Python pullreq Buffering improvements for qemu machine, minor changes to support the newly released Python 3.12 John Snow (9): Python/iotests: Add type hint for nbd module python/machine: move socket setup out of _base_args property python/machine: close sock_pair in cleanup path python/console_socket: accept existing FD in initializer python/machine: use socketpair() for console connections python/machine: use socketpair() for qtest connection python/machine: remove unused sock_dir argument python/qmp: remove Server.wait_closed() call for Python 3.12 Python: test Python 3.12 Is that a pull request or a patch series to be reviewed?
Re: [PATCH 10/21] qapi: Inline QERR_INVALID_PARAMETER_VALUE definition (constant value)
On 4/10/23 20:15, Juan Quintela wrote: Philippe Mathieu-Daudé wrote: Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using the following coccinelle semantic patch: @match@ expression errp; constant param; constant value; @@ error_setg(errp, QERR_INVALID_PARAMETER_VALUE, param, value); @script:python strformat depends on match@ param << match.param; value << match.value; fixedfmt; // new var @@ fixedfmt = "\"Parameter '%s' expects %s\"" % (param[1:-1], value[1:-1]) coccinelle.fixedfmt = cocci.make_ident(fixedfmt) @replace@ expression match.errp; constant match.param; constant match.value; identifier strformat.fixedfmt; @@ -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, param, value); +error_setg(errp, fixedfmt); Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Juan Quintela And like the approach, but if (granularity != 0 && (granularity < 512 || granularity > 1048576 * 64)) { -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "granularity", - "a value in range [512B, 64MB]"); +error_setg(errp, + "Parameter 'granularity' expects a value in range [512B, 64MB]"); return; There are several lines like this one that become way bigger than 80 characters. Yes, I just realized I forgot to run checkpatch.pl :/ Now done, this is the single patch producing: ERROR: line over 90 characters Later, Juan. PD. No, I have no clue about how to convince coccinelle to obey qemu indentation rules. Well this use Python, so we could check the length and split (returning some tuple) but: $ ./scripts/checkpatch.pl origin/master.. | fgrep 'ERROR:' | wc -l 10 So I guess I'll just manually adapt :)
Re: [PATCH 15/21] qapi: Inline QERR_MISSING_PARAMETER definition (constant parameter)
On 10/4/23 13:31, Philippe Mathieu-Daudé wrote: Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using the following coccinelle semantic patches: @match@ expression errp; constant param; @@ error_setg(errp, QERR_MISSING_PARAMETER, param); @script:python strformat depends on match@ param << match.param; fixedfmt; // new var @@ if param[0] == '"': # Format skipping '"', fixedfmt = f'"Parameter \'{param[1:-1]}\' is missing"' else: # or use definition. fixedfmt = f'"Parameter " {param} " is missing"' coccinelle.fixedfmt = cocci.make_ident(fixedfmt) @replace@ expression match.errp; constant match.param; identifier strformat.fixedfmt; @@ -error_setg(errp, QERR_MISSING_PARAMETER, param); +error_setg(errp, fixedfmt); and: @match@ constant param; @@ error_report(QERR_MISSING_PARAMETER, param); @script:python strformat depends on match@ param << match.param; fixedfmt; // new var @@ fixedfmt = f'"Parameter \'{param[1:-1]}\' is missing"' coccinelle.fixedfmt = cocci.make_ident(fixedfmt) @replace@ constant match.param; identifier strformat.fixedfmt; @@ -error_report(QERR_MISSING_PARAMETER, param); +error_report(fixedfmt); Signed-off-by: Philippe Mathieu-Daud Reviewed-by: Stefan Berger --- backends/dbus-vmstate.c| 2 +- block/gluster.c| 21 +++-- block/monitor/block-hmp-cmds.c | 6 +++--- dump/dump.c| 4 ++-- hw/usb/redirect.c | 2 +- softmmu/qdev-monitor.c | 2 +- softmmu/tpm.c | 4 ++-- softmmu/vl.c | 4 ++-- ui/input-barrier.c | 2 +- ui/ui-qmp-cmds.c | 2 +- 10 files changed, 25 insertions(+), 24 deletions(-) diff --git a/backends/dbus-vmstate.c b/backends/dbus-vmstate.c index 57369ec0f2..e781ded17c 100644 --- a/backends/dbus-vmstate.c +++ b/backends/dbus-vmstate.c @@ -413,7 +413,7 @@ dbus_vmstate_complete(UserCreatable *uc, Error **errp) } if (!self->dbus_addr) { -error_setg(errp, QERR_MISSING_PARAMETER, "addr"); +error_setg(errp, "Parameter 'addr' is missing"); return; } diff --git a/block/gluster.c b/block/gluster.c index ad5fadbe79..8d97d698c3 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -530,20 +530,20 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf, num_servers = qdict_array_entries(options, GLUSTER_OPT_SERVER_PATTERN); if (num_servers < 1) { -error_setg(_err, QERR_MISSING_PARAMETER, "server"); +error_setg(_err, "Parameter 'server' is missing"); goto out; } ptr = qemu_opt_get(opts, GLUSTER_OPT_VOLUME); if (!ptr) { -error_setg(_err, QERR_MISSING_PARAMETER, GLUSTER_OPT_VOLUME); +error_setg(_err, "Parameter " GLUSTER_OPT_VOLUME " is missing"); goto out; } gconf->volume = g_strdup(ptr); ptr = qemu_opt_get(opts, GLUSTER_OPT_PATH); if (!ptr) { -error_setg(_err, QERR_MISSING_PARAMETER, GLUSTER_OPT_PATH); +error_setg(_err, "Parameter " GLUSTER_OPT_PATH " is missing"); goto out; } gconf->path = g_strdup(ptr); @@ -562,7 +562,8 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf, ptr = qemu_opt_get(opts, GLUSTER_OPT_TYPE); if (!ptr) { -error_setg(_err, QERR_MISSING_PARAMETER, GLUSTER_OPT_TYPE); +error_setg(_err, + "Parameter " GLUSTER_OPT_TYPE " is missing"); error_append_hint(_err, GERR_INDEX_HINT, i); goto out; @@ -592,16 +593,16 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf, ptr = qemu_opt_get(opts, GLUSTER_OPT_HOST); if (!ptr) { -error_setg(_err, QERR_MISSING_PARAMETER, - GLUSTER_OPT_HOST); +error_setg(_err, + "Parameter " GLUSTER_OPT_HOST " is missing"); error_append_hint(_err, GERR_INDEX_HINT, i); goto out; } gsconf->u.inet.host = g_strdup(ptr); ptr = qemu_opt_get(opts, GLUSTER_OPT_PORT); if (!ptr) { -error_setg(_err, QERR_MISSING_PARAMETER, - GLUSTER_OPT_PORT); +error_setg(_err, + "Parameter " GLUSTER_OPT_PORT " is missing"); error_append_hint(_err, GERR_INDEX_HINT, i); goto out; } @@ -648,8 +649,8 @@ static int
Re: [PATCH 12/21] qapi: Inline and remove QERR_INVALID_PARAMETER_VALUE definition
On 10/4/23 13:31, Philippe Mathieu-Daudé wrote: Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Manually modify the error_report() call in softmmu/tpm.c, then use sed to mechanically transform the rest. Finally remove the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Stefan Berger --- include/qapi/qmp/qerror.h | 3 --- qapi/opts-visitor.c | 4 ++-- qapi/qapi-visit-core.c| 4 ++-- softmmu/tpm.c | 3 +-- 4 files changed, 5 insertions(+), 9 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index b723830eff..ac727d1c2d 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_INVALID_PARAMETER_VALUE \ -"Parameter '%s' expects %s" - #define QERR_IO_ERROR \ "An IO error has occurred" diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c index 0393704a73..844db583f4 100644 --- a/qapi/opts-visitor.c +++ b/qapi/opts-visitor.c @@ -441,7 +441,7 @@ opts_type_int64(Visitor *v, const char *name, int64_t *obj, Error **errp) } } } -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name, +error_setg(errp, "Parameter '%s' expects %s", opt->name, (ov->list_mode == LM_NONE) ? "an int64 value" : "an int64 value or range"); return false; @@ -494,7 +494,7 @@ opts_type_uint64(Visitor *v, const char *name, uint64_t *obj, Error **errp) } } } -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name, +error_setg(errp, "Parameter '%s' expects %s", opt->name, (ov->list_mode == LM_NONE) ? "a uint64 value" : "a uint64 value or range"); return false; diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c index 6c13510a2b..01793d6e74 100644 --- a/qapi/qapi-visit-core.c +++ b/qapi/qapi-visit-core.c @@ -194,7 +194,7 @@ static bool visit_type_uintN(Visitor *v, uint64_t *obj, const char *name, } if (value > max) { assert(v->type == VISITOR_INPUT); -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, +error_setg(errp, "Parameter '%s' expects %s", name ? name : "null", type); return false; } @@ -262,7 +262,7 @@ static bool visit_type_intN(Visitor *v, int64_t *obj, const char *name, } if (value < min || value > max) { assert(v->type == VISITOR_INPUT); -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, +error_setg(errp, "Parameter '%s' expects %s", name ? name : "null", type); return false; } diff --git a/softmmu/tpm.c b/softmmu/tpm.c index 578563f05a..8437c4efc3 100644 --- a/softmmu/tpm.c +++ b/softmmu/tpm.c @@ -120,8 +120,7 @@ static int tpm_init_tpmdev(void *dummy, QemuOpts *opts, Error **errp) i = qapi_enum_parse(_lookup, value, -1, NULL); be = i >= 0 ? tpm_be_find_by_type(i) : NULL; if (be == NULL) { -error_report(QERR_INVALID_PARAMETER_VALUE, - "type", "a TPM backend type"); +error_report("Parameter 'type' expects a TPM backend type"); tpm_display_backend_drivers(); return 1; }
Re: [PATCH v7 12/12] nbd/server: Add FLAG_PAYLOAD support to CMD_BLOCK_STATUS
On Sat, Sep 30, 2023 at 04:24:02PM +0300, Vladimir Sementsov-Ogievskiy wrote: > On 25.09.23 22:22, Eric Blake wrote: > > Allow a client to request a subset of negotiated meta contexts. For > > example, a client may ask to use a single connection to learn about > > both block status and dirty bitmaps, but where the dirty bitmap > > queries only need to be performed on a subset of the disk; forcing the > > server to compute that information on block status queries in the rest > > of the disk is wasted effort (both at the server, and on the amount of > > traffic sent over the wire to be parsed and ignored by the client). > > > > Qemu as an NBD client never requests to use more than one meta > > context, so it has no need to use block status payloads. Testing this > > instead requires support from libnbd, which CAN access multiple meta > > contexts in parallel from a single NBD connection; an interop test > > submitted to the libnbd project at the same time as this patch > > demonstrates the feature working, as well as testing some corner cases > > (for example, when the payload length is longer than the export > > length), although other corner cases (like passing the same id > > duplicated) requires a protocol fuzzer because libnbd is not wired up > > to break the protocol that badly. > > > > This also includes tweaks to 'qemu-nbd --list' to show when a server > > is advertising the capability, and to the testsuite to reflect the > > addition to that output. > > > > Of note: qemu will always advertise the new feature bit during > > NBD_OPT_INFO if extended headers have alreay been negotiated > > (regardless of whether any NBD_OPT_SET_META_CONTEXT negotiation has > > occurred); but for NBD_OPT_GO, qemu only advertises the feature if > > block status is also enabled (that is, if the client does not > > negotiate any contexts, then NBD_CMD_BLOCK_STATUS cannot be used, so > > the feature is not advertised). > > > > Signed-off-by: Eric Blake > > --- > > > > [..] > > > > > +/* > > + * nbd_co_block_status_payload_read > > + * Called when a client wants a subset of negotiated contexts via a > > + * BLOCK_STATUS payload. Check the payload for valid length and > > + * contents. On success, return 0 with request updated to effective > > + * length. If request was invalid but all payload consumed, return 0 > > + * with request->len and request->contexts->count set to 0 (which will > > + * trigger an appropriate NBD_EINVAL response later on). Return > > + * negative errno if the payload was not fully consumed. > > + */ > > +static int > > +nbd_co_block_status_payload_read(NBDClient *client, NBDRequest *request, > > + Error **errp) > > [..] > > > +payload_len > (sizeof(NBDBlockStatusPayload) + > > + sizeof(id) * client->contexts.count)) { > > +goto skip; > > +} > > + > > +buf = g_malloc(payload_len); > > +if (nbd_read(client->ioc, buf, payload_len, > > + "CMD_BLOCK_STATUS data", errp) < 0) { > > +return -EIO; > > +} > > +trace_nbd_co_receive_request_payload_received(request->cookie, > > + payload_len); > > +request->contexts->bitmaps = g_new0(bool, nr_bitmaps); > > +count = (payload_len - sizeof(NBDBlockStatusPayload)) / sizeof(id); > > +payload_len = 0; > > + > > +for (i = 0; i < count; i++) { > > +id = ldl_be_p(buf + sizeof(NBDBlockStatusPayload) + sizeof(id) * > > i); > > +if (id == NBD_META_ID_BASE_ALLOCATION) { > > +if (request->contexts->base_allocation) { > > +goto skip; > > +} > > should we also check that base_allocation is negotiated? Oh, good point. Without that check, the client can pass in random id numbers that it never negotiated. I've queued 1-11 and will probably send a pull request for those this week, while respinning this patch to fix the remaining issues you pointed out. > > > +request->contexts->base_allocation = true; > > +} else if (id == NBD_META_ID_ALLOCATION_DEPTH) { > > +if (request->contexts->allocation_depth) { > > +goto skip; > > +} > > same here > > > +request->contexts->allocation_depth = true; > > +} else { > > +int idx = id - NBD_META_ID_DIRTY_BITMAP; > > + > > I think, we also should check that idx >=0 after this operation. > > > +if (idx > nr_bitmaps || request->contexts->bitmaps[idx]) { Or else make idx an unsigned value, instead of signed. Also a good catch. > > +goto skip; > > +} > > +request->contexts->bitmaps[idx] = true; > > +} > > +} > > + > > +request->len = ldq_be_p(buf); > > +request->contexts->count = count; > > +return 0; > > + > > + skip: > > +trace_nbd_co_receive_block_status_payload_compliance(request->from, > > +
[PULL 2/9] python/machine: move socket setup out of _base_args property
This property isn't meant to do much else besides return a list of strings, so move this setup back out into _pre_launch(). Signed-off-by: John Snow Reviewed-by: Ani Sinha Reviewed-by: Daniel P. Berrangé Message-id: 20230928044943.849073-2-js...@redhat.com Signed-off-by: John Snow --- python/qemu/machine/machine.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py index 35d5a672dbb..345610d6e46 100644 --- a/python/qemu/machine/machine.py +++ b/python/qemu/machine/machine.py @@ -301,9 +301,7 @@ def _base_args(self) -> List[str]: if self._qmp_set: if self._sock_pair: -fd = self._sock_pair[0].fileno() -os.set_inheritable(fd, True) -moncdev = f"socket,id=mon,fd={fd}" +moncdev = f"socket,id=mon,fd={self._sock_pair[0].fileno()}" elif isinstance(self._monitor_address, tuple): moncdev = "socket,id=mon,host={},port={}".format( *self._monitor_address @@ -340,6 +338,7 @@ def _pre_launch(self) -> None: if self._qmp_set: if self._monitor_address is None: self._sock_pair = socket.socketpair() +os.set_inheritable(self._sock_pair[0].fileno(), True) sock = self._sock_pair[1] if isinstance(self._monitor_address, str): self._remove_files.append(self._monitor_address) -- 2.41.0
[PULL 9/9] Python: test Python 3.12
Python 3.12 has released, so update the test infrastructure to test against this version. Signed-off-by: John Snow --- python/setup.cfg | 3 ++- tests/docker/dockerfiles/python.docker | 6 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/python/setup.cfg b/python/setup.cfg index 8c67dce4579..48668609d3e 100644 --- a/python/setup.cfg +++ b/python/setup.cfg @@ -18,6 +18,7 @@ classifiers = Programming Language :: Python :: 3.9 Programming Language :: Python :: 3.10 Programming Language :: Python :: 3.11 +Programming Language :: Python :: 3.12 Typing :: Typed [options] @@ -182,7 +183,7 @@ multi_line_output=3 # of python available on your system to run this test. [tox:tox] -envlist = py38, py39, py310, py311 +envlist = py38, py39, py310, py311, py312 skip_missing_interpreters = true [testenv] diff --git a/tests/docker/dockerfiles/python.docker b/tests/docker/dockerfiles/python.docker index 383ccbdc3a5..a3c1321190c 100644 --- a/tests/docker/dockerfiles/python.docker +++ b/tests/docker/dockerfiles/python.docker @@ -11,7 +11,11 @@ ENV PACKAGES \ python3-pip \ python3-tox \ python3-virtualenv \ -python3.10 +python3.10 \ +python3.11 \ +python3.12 \ +python3.8 \ +python3.9 RUN dnf install -y $PACKAGES RUN rpm -q $PACKAGES | sort > /packages.txt -- 2.41.0
[PULL 1/9] Python/iotests: Add type hint for nbd module
The test bails gracefully if this module isn't installed, but linters need a little help understanding that. It's enough to just declare the type in this case. (Fixes pylint complaining about use of an uninitialized variable because it isn't wise enough to understand the notrun call is noreturn.) Signed-off-by: John Snow --- tests/qemu-iotests/tests/nbd-multiconn | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/qemu-iotests/tests/nbd-multiconn b/tests/qemu-iotests/tests/nbd-multiconn index 478a1eaba27..7e686a786ea 100755 --- a/tests/qemu-iotests/tests/nbd-multiconn +++ b/tests/qemu-iotests/tests/nbd-multiconn @@ -20,6 +20,8 @@ import os from contextlib import contextmanager +from types import ModuleType + import iotests from iotests import qemu_img_create, qemu_io @@ -28,7 +30,7 @@ disk = os.path.join(iotests.test_dir, 'disk') size = '4M' nbd_sock = os.path.join(iotests.sock_dir, 'nbd_sock') nbd_uri = 'nbd+unix:///{}?socket=' + nbd_sock - +nbd: ModuleType @contextmanager def open_nbd(export_name): -- 2.41.0
[PULL 3/9] python/machine: close sock_pair in cleanup path
If everything has gone smoothly, we'll already have closed the socket we gave to the child during post_launch. The other half of the pair that we gave to the QMP connection should, likewise, be definitively closed by now. However, in the cleanup path, it's possible we've created the socketpair but flubbed the launch and need to clean up resources. These resources *would* be handled by the garbage collector, but that can happen at unpredictable times. Nicer to just clean them up synchronously on the exit path, here. Signed-off-by: John Snow Reviewed-by: Ani Sinha Reviewed-by: Daniel P. Berrangé Message-id: 20230928044943.849073-3-js...@redhat.com Signed-off-by: John Snow --- python/qemu/machine/machine.py | 5 + 1 file changed, 5 insertions(+) diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py index 345610d6e46..e26109e6f0e 100644 --- a/python/qemu/machine/machine.py +++ b/python/qemu/machine/machine.py @@ -396,6 +396,11 @@ def _post_shutdown(self) -> None: finally: assert self._qmp_connection is None +if self._sock_pair: +self._sock_pair[0].close() +self._sock_pair[1].close() +self._sock_pair = None + self._close_qemu_log_file() self._load_io_log() -- 2.41.0
[PULL 8/9] python/qmp: remove Server.wait_closed() call for Python 3.12
This patch is a backport from https://gitlab.com/qemu-project/python-qemu-qmp/-/commit/e03a3334b6a477beb09b293708632f2c06fe9f61 According to Guido in https://github.com/python/cpython/issues/104344 , this call was never meant to wait for the server to shut down - that is handled synchronously - but instead, this waits for all connections to close. Or, it would have, if it wasn't broken since it was introduced. 3.12 fixes the bug, which now causes a hang in our code. The fix is just to remove the wait. Signed-off-by: John Snow --- python/qemu/qmp/protocol.py | 1 - 1 file changed, 1 deletion(-) diff --git a/python/qemu/qmp/protocol.py b/python/qemu/qmp/protocol.py index 753182131fd..a4ffdfad51b 100644 --- a/python/qemu/qmp/protocol.py +++ b/python/qemu/qmp/protocol.py @@ -495,7 +495,6 @@ async def _stop_server(self) -> None: try: self.logger.debug("Stopping server.") self._server.close() -await self._server.wait_closed() self.logger.debug("Server stopped.") finally: self._server = None -- 2.41.0
[PULL 6/9] python/machine: use socketpair() for qtest connection
Like the QMP and console sockets, begin using socketpairs for the qtest connection, too. After this patch, we'll be able to remove the vestigial sock_dir argument, but that cleanup is best done in its own patch. Signed-off-by: John Snow Reviewed-by: Daniel P. Berrangé Message-id: 20230928044943.849073-6-js...@redhat.com Signed-off-by: John Snow --- python/qemu/machine/qtest.py | 49 +--- 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/python/qemu/machine/qtest.py b/python/qemu/machine/qtest.py index 1c46138bd0c..8180d3ab017 100644 --- a/python/qemu/machine/qtest.py +++ b/python/qemu/machine/qtest.py @@ -24,6 +24,7 @@ Optional, Sequence, TextIO, +Tuple, ) from qemu.qmp import SocketAddrT @@ -38,23 +39,41 @@ class QEMUQtestProtocol: :param address: QEMU address, can be either a unix socket path (string) or a tuple in the form ( address, port ) for a TCP connection -:param server: server mode, listens on the socket (bool) +:param sock: An existing socket can be provided as an alternative to + an address. One of address or sock must be provided. +:param server: server mode, listens on the socket. Only meaningful + in conjunction with an address and not an existing + socket. + :raise socket.error: on socket connection errors .. note:: No connection is established by __init__(), this is done by the connect() or accept() methods. """ -def __init__(self, address: SocketAddrT, +def __init__(self, + address: Optional[SocketAddrT] = None, + sock: Optional[socket.socket] = None, server: bool = False): +if address is None and sock is None: +raise ValueError("Either 'address' or 'sock' must be specified") +if address is not None and sock is not None: +raise ValueError( +"Either 'address' or 'sock' must be specified, but not both") +if sock is not None and server: +raise ValueError("server=True is meaningless when passing socket") + self._address = address -self._sock = self._get_sock() +self._sock = sock or self._get_sock() self._sockfile: Optional[TextIO] = None + if server: +assert self._address is not None self._sock.bind(self._address) self._sock.listen(1) def _get_sock(self) -> socket.socket: +assert self._address is not None if isinstance(self._address, tuple): family = socket.AF_INET else: @@ -67,7 +86,8 @@ def connect(self) -> None: @raise socket.error on socket connection errors """ -self._sock.connect(self._address) +if self._address is not None: +self._sock.connect(self._address) self._sockfile = self._sock.makefile(mode='r') def accept(self) -> None: @@ -127,29 +147,40 @@ def __init__(self, base_temp_dir=base_temp_dir, sock_dir=sock_dir, qmp_timer=qmp_timer) self._qtest: Optional[QEMUQtestProtocol] = None -self._qtest_path = os.path.join(sock_dir, name + "-qtest.sock") +self._qtest_sock_pair: Optional[ +Tuple[socket.socket, socket.socket]] = None @property def _base_args(self) -> List[str]: args = super()._base_args +assert self._qtest_sock_pair is not None +fd = self._qtest_sock_pair[0].fileno() args.extend([ -'-qtest', f"unix:path={self._qtest_path}", +'-chardev', f"socket,id=qtest,fd={fd}", +'-qtest', 'chardev:qtest', '-accel', 'qtest' ]) return args def _pre_launch(self) -> None: +self._qtest_sock_pair = socket.socketpair() +os.set_inheritable(self._qtest_sock_pair[0].fileno(), True) super()._pre_launch() -self._qtest = QEMUQtestProtocol(self._qtest_path, server=True) +self._qtest = QEMUQtestProtocol(sock=self._qtest_sock_pair[1]) def _post_launch(self) -> None: assert self._qtest is not None super()._post_launch() -self._qtest.accept() +if self._qtest_sock_pair: +self._qtest_sock_pair[0].close() +self._qtest.connect() def _post_shutdown(self) -> None: +if self._qtest_sock_pair: +self._qtest_sock_pair[0].close() +self._qtest_sock_pair[1].close() +self._qtest_sock_pair = None super()._post_shutdown() -self._remove_if_exists(self._qtest_path) def qtest(self, cmd: str) -> str: """ -- 2.41.0
[PULL 4/9] python/console_socket: accept existing FD in initializer
Useful if we want to use ConsoleSocket() for a socket created by socketpair(). Signed-off-by: John Snow Reviewed-by: Ani Sinha Reviewed-by: Daniel P. Berrangé Message-id: 20230928044943.849073-4-js...@redhat.com Signed-off-by: John Snow --- python/qemu/machine/console_socket.py | 29 +++ 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/python/qemu/machine/console_socket.py b/python/qemu/machine/console_socket.py index 4e28ba9bb23..0a4e09ffc73 100644 --- a/python/qemu/machine/console_socket.py +++ b/python/qemu/machine/console_socket.py @@ -24,19 +24,32 @@ class ConsoleSocket(socket.socket): """ ConsoleSocket represents a socket attached to a char device. -Optionally (if drain==True), drains the socket and places the bytes -into an in memory buffer for later processing. - -Optionally a file path can be passed in and we will also -dump the characters to this file for debugging purposes. +:param address: An AF_UNIX path or address. +:param sock_fd: Optionally, an existing socket file descriptor. +One of address or sock_fd must be specified. +:param file: Optionally, a filename to log to. +:param drain: Optionally, drains the socket and places the bytes + into an in memory buffer for later processing. """ -def __init__(self, address: str, file: Optional[str] = None, +def __init__(self, + address: Optional[str] = None, + sock_fd: Optional[int] = None, + file: Optional[str] = None, drain: bool = False): +if address is None and sock_fd is None: +raise ValueError("one of 'address' or 'sock_fd' must be specified") +if address is not None and sock_fd is not None: +raise ValueError("can't specify both 'address' and 'sock_fd'") + self._recv_timeout_sec = 300.0 self._sleep_time = 0.5 self._buffer: Deque[int] = deque() -socket.socket.__init__(self, socket.AF_UNIX, socket.SOCK_STREAM) -self.connect(address) +if address is not None: +socket.socket.__init__(self, socket.AF_UNIX, socket.SOCK_STREAM) +self.connect(address) +else: +assert sock_fd is not None +socket.socket.__init__(self, fileno=sock_fd) self._logfile = None if file: # pylint: disable=consider-using-with -- 2.41.0
[PULL 7/9] python/machine: remove unused sock_dir argument
By using a socketpair for all of the sockets managed by the VM class and its extensions, we don't need the sock_dir argument anymore, so remove it. We only added this argument so that we could specify a second, shorter temporary directory for cases where the temp/log dirs were "too long" as a socket name on macOS. We don't need it for this class now. In one case, avocado testing takes over responsibility for creating an appropriate sockdir. Signed-off-by: John Snow Reviewed-by: Daniel P. Berrangé Message-id: 20230928044943.849073-7-js...@redhat.com Signed-off-by: John Snow --- python/qemu/machine/machine.py | 18 -- python/qemu/machine/qtest.py | 5 + tests/avocado/acpi-bits.py | 5 + tests/avocado/avocado_qemu/__init__.py | 2 +- tests/avocado/machine_aspeed.py| 5 - tests/qemu-iotests/iotests.py | 2 +- tests/qemu-iotests/tests/copy-before-write | 3 +-- 7 files changed, 9 insertions(+), 31 deletions(-) diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py index 4156b8cf7d4..d539e91268a 100644 --- a/python/qemu/machine/machine.py +++ b/python/qemu/machine/machine.py @@ -127,7 +127,6 @@ def __init__(self, name: Optional[str] = None, base_temp_dir: str = "/var/tmp", monitor_address: Optional[SocketAddrT] = None, - sock_dir: Optional[str] = None, drain_console: bool = False, console_log: Optional[str] = None, log_dir: Optional[str] = None, @@ -141,7 +140,6 @@ def __init__(self, @param name: prefix for socket and log file names (default: qemu-PID) @param base_temp_dir: default location where temp files are created @param monitor_address: address for QMP monitor -@param sock_dir: where to create socket (defaults to base_temp_dir) @param drain_console: (optional) True to drain console socket to buffer @param console_log: (optional) path to console log file @param log_dir: where to create and keep log files @@ -163,7 +161,6 @@ def __init__(self, Tuple[socket.socket, socket.socket]] = None self._temp_dir: Optional[str] = None self._base_temp_dir = base_temp_dir -self._sock_dir = sock_dir self._log_dir = log_dir self._monitor_address = monitor_address @@ -189,9 +186,6 @@ def __init__(self, self._console_index = 0 self._console_set = False self._console_device_type: Optional[str] = None -self._console_address = os.path.join( -self.sock_dir, f"{self._name}.con" -) self._console_socket: Optional[socket.socket] = None self._console_file: Optional[socket.SocketIO] = None self._remove_files: List[str] = [] @@ -335,9 +329,6 @@ def args(self) -> List[str]: return self._args def _pre_launch(self) -> None: -if self._console_set: -self._remove_files.append(self._console_address) - if self._qmp_set: if self._monitor_address is None: self._sock_pair = socket.socketpair() @@ -937,15 +928,6 @@ def temp_dir(self) -> str: dir=self._base_temp_dir) return self._temp_dir -@property -def sock_dir(self) -> str: -""" -Returns the directory used for sockfiles by this machine. -""" -if self._sock_dir: -return self._sock_dir -return self.temp_dir - @property def log_dir(self) -> str: """ diff --git a/python/qemu/machine/qtest.py b/python/qemu/machine/qtest.py index 8180d3ab017..4f5ede85b23 100644 --- a/python/qemu/machine/qtest.py +++ b/python/qemu/machine/qtest.py @@ -135,17 +135,14 @@ def __init__(self, wrapper: Sequence[str] = (), name: Optional[str] = None, base_temp_dir: str = "/var/tmp", - sock_dir: Optional[str] = None, qmp_timer: Optional[float] = None): # pylint: disable=too-many-arguments if name is None: name = "qemu-%d" % os.getpid() -if sock_dir is None: -sock_dir = base_temp_dir super().__init__(binary, args, wrapper=wrapper, name=name, base_temp_dir=base_temp_dir, - sock_dir=sock_dir, qmp_timer=qmp_timer) + qmp_timer=qmp_timer) self._qtest: Optional[QEMUQtestProtocol] = None self._qtest_sock_pair: Optional[ Tuple[socket.socket, socket.socket]] = None diff --git a/tests/avocado/acpi-bits.py b/tests/avocado/acpi-bits.py index bb3f8186899..eca13dc5181 100644 --- a/tests/avocado/acpi-bits.py +++ b/tests/avocado/acpi-bits.py @@ -92,17 +92,14 @@ def __init__(self, base_temp_dir: str = "/var/tmp",
[PULL 5/9] python/machine: use socketpair() for console connections
Create a socketpair for the console output. This should help eliminate race conditions around console text early in the boot process that might otherwise have been dropped on the floor before being able to connect to QEMU under "server,nowait". Signed-off-by: John Snow Reviewed-by: Ani Sinha Reviewed-by: Daniel P. Berrangé Message-id: 20230928044943.849073-5-js...@redhat.com Signed-off-by: John Snow --- python/qemu/machine/machine.py | 30 +++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py index e26109e6f0e..4156b8cf7d4 100644 --- a/python/qemu/machine/machine.py +++ b/python/qemu/machine/machine.py @@ -159,6 +159,8 @@ def __init__(self, self._name = name or f"{id(self):x}" self._sock_pair: Optional[Tuple[socket.socket, socket.socket]] = None +self._cons_sock_pair: Optional[ +Tuple[socket.socket, socket.socket]] = None self._temp_dir: Optional[str] = None self._base_temp_dir = base_temp_dir self._sock_dir = sock_dir @@ -316,8 +318,9 @@ def _base_args(self) -> List[str]: for _ in range(self._console_index): args.extend(['-serial', 'null']) if self._console_set: -chardev = ('socket,id=console,path=%s,server=on,wait=off' % - self._console_address) +assert self._cons_sock_pair is not None +fd = self._cons_sock_pair[0].fileno() +chardev = f"socket,id=console,fd={fd}" args.extend(['-chardev', chardev]) if self._console_device_type is None: args.extend(['-serial', 'chardev:console']) @@ -352,6 +355,10 @@ def _pre_launch(self) -> None: nickname=self._name ) +if self._console_set: +self._cons_sock_pair = socket.socketpair() +os.set_inheritable(self._cons_sock_pair[0].fileno(), True) + # NOTE: Make sure any opened resources are *definitely* freed in # _post_shutdown()! # pylint: disable=consider-using-with @@ -369,6 +376,9 @@ def _pre_launch(self) -> None: def _post_launch(self) -> None: if self._sock_pair: self._sock_pair[0].close() +if self._cons_sock_pair: +self._cons_sock_pair[0].close() + if self._qmp_connection: if self._sock_pair: self._qmp.connect() @@ -524,6 +534,11 @@ def _early_cleanup(self) -> None: self._console_socket.close() self._console_socket = None +if self._cons_sock_pair: +self._cons_sock_pair[0].close() +self._cons_sock_pair[1].close() +self._cons_sock_pair = None + def _hard_shutdown(self) -> None: """ Perform early cleanup, kill the VM, and wait for it to terminate. @@ -885,10 +900,19 @@ def console_socket(self) -> socket.socket: """ if self._console_socket is None: LOG.debug("Opening console socket") +if not self._console_set: +raise QEMUMachineError( +"Attempt to access console socket with no connection") +assert self._cons_sock_pair is not None +# os.dup() is used here for sock_fd because otherwise we'd +# have two rich python socket objects that would each try to +# close the same underlying fd when either one gets garbage +# collected. self._console_socket = console_socket.ConsoleSocket( -self._console_address, +sock_fd=os.dup(self._cons_sock_pair[1].fileno()), file=self._console_log_path, drain=self._drain_console) +self._cons_sock_pair[1].close() return self._console_socket @property -- 2.41.0
[PULL 0/9] Python patches
The following changes since commit da1034094d375afe9e3d8ec8980550ea0f06f7e0: Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging (2023-10-03 07:43:44 -0400) are available in the Git repository at: https://gitlab.com/jsnow/qemu.git tags/python-pull-request for you to fetch changes up to 4d7a663cbe8343e884b88e44bd88d37dd0a470e5: Python: test Python 3.12 (2023-10-04 15:19:00 -0400) Python pullreq Buffering improvements for qemu machine, minor changes to support the newly released Python 3.12 John Snow (9): Python/iotests: Add type hint for nbd module python/machine: move socket setup out of _base_args property python/machine: close sock_pair in cleanup path python/console_socket: accept existing FD in initializer python/machine: use socketpair() for console connections python/machine: use socketpair() for qtest connection python/machine: remove unused sock_dir argument python/qmp: remove Server.wait_closed() call for Python 3.12 Python: test Python 3.12 python/qemu/machine/console_socket.py | 29 --- python/qemu/machine/machine.py | 58 +- python/qemu/machine/qtest.py | 54 +++- python/qemu/qmp/protocol.py| 1 - python/setup.cfg | 3 +- tests/avocado/acpi-bits.py | 5 +- tests/avocado/avocado_qemu/__init__.py | 2 +- tests/avocado/machine_aspeed.py| 5 +- tests/docker/dockerfiles/python.docker | 6 ++- tests/qemu-iotests/iotests.py | 2 +- tests/qemu-iotests/tests/copy-before-write | 3 +- tests/qemu-iotests/tests/nbd-multiconn | 4 +- 12 files changed, 114 insertions(+), 58 deletions(-) -- 2.41.0
Re: [PATCH v3 00/16] (few more) Steps towards enabling -Wshadow
On 10/4/23 05:00, Philippe Mathieu-Daudé wrote: Philippe Mathieu-Daudé (16): hw/audio/soundhw: Clean up global variable shadowing hw/ide/ahci: Clean up local variable shadowing net/net: Clean up global variable shadowing os-posix: Clean up global variable shadowing plugins/loader: Clean up global variable shadowing qemu-img: Clean up global variable shadowing qemu-io: Clean up global variable shadowing qom/object_interfaces: Clean up global variable shadowing semihosting: Clean up global variable shadowing ui/cocoa: Clean up global variable shadowing util/cutils: Clean up global variable shadowing in get_relocated_path() util/guest-random: Clean up global variable shadowing semihosting/arm-compat: Clean up local variable shadowing softmmu/vl: Clean up global variable shadowing sysemu/tpm: Clean up global variable shadowing trace/control: Clean up global variable shadowing Series: Reviewed-by: Richard Henderson r~
Re: [PATCH v3 11/16] util/cutils: Clean up global variable shadowing in get_relocated_path()
On Wed, Oct 04, 2023 at 02:00:14PM +0200, Philippe Mathieu-Daudé wrote: > Fix: > > util/cutils.c:1147:17: error: declaration shadows a variable in the global > scope [-Werror,-Wshadow] > const char *exec_dir = qemu_get_exec_dir(); > ^ > util/cutils.c:1035:20: note: previous declaration is here > static const char *exec_dir; > ^ > > Signed-off-by: Philippe Mathieu-Daudé > --- > util/cutils.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/util/cutils.c b/util/cutils.c > index 25373198ad..b44718a6a2 100644 > --- a/util/cutils.c > +++ b/util/cutils.c > @@ -1144,7 +1144,6 @@ char *get_relocated_path(const char *dir) > { > size_t prefix_len = strlen(CONFIG_PREFIX); > const char *bindir = CONFIG_BINDIR; > -const char *exec_dir = qemu_get_exec_dir(); > GString *result; > int len_dir, len_bindir; > > -- Took me a few seconds to see it, but since we have this just a few lines before: const char *qemu_get_exec_dir(void) { return exec_dir; } the deletion of the redundant local variable is just fine. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org
Re: [PATCH v3 07/16] qemu-io: Clean up global variable shadowing
On Wed, Oct 04, 2023 at 02:00:10PM +0200, Philippe Mathieu-Daudé wrote: > Fix: > > qemu-io.c:478:36: error: declaration shadows a variable in the global scope > [-Werror,-Wshadow] > static void add_user_command(char *optarg) > ^ > > /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: > note: previous declaration is here > extern char *optarg;/* getopt(3) external variables */ >^ > > Signed-off-by: Philippe Mathieu-Daudé > --- > qemu-io.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org
Re: [PATCH v3 06/16] qemu-img: Clean up global variable shadowing
On Wed, Oct 04, 2023 at 02:00:09PM +0200, Philippe Mathieu-Daudé wrote: > Fix: > > qemu-img.c:247:46: error: declaration shadows a variable in the global > scope [-Werror,-Wshadow] > static bool is_valid_option_list(const char *optarg) >^ > qemu-img.c:265:53: error: declaration shadows a variable in the global > scope [-Werror,-Wshadow] > static int accumulate_options(char **options, char *optarg) > ^ > > /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: > note: previous declaration is here > extern char *optarg;/* getopt(3) external variables */ >^ > > Signed-off-by: Philippe Mathieu-Daudé > --- > qemu-img.c | 22 +++--- > 1 file changed, 11 insertions(+), 11 deletions(-) > Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org
Re: [PATCH v3 04/16] os-posix: Clean up global variable shadowing
On Wed, Oct 04, 2023 at 02:00:07PM +0200, Philippe Mathieu-Daudé wrote: > Fix: > > os-posix.c:103:31: error: declaration shadows a variable in the global > scope [-Werror,-Wshadow] > bool os_set_runas(const char *optarg) > ^ > os-posix.c:176:32: error: declaration shadows a variable in the global > scope [-Werror,-Wshadow] > void os_set_chroot(const char *optarg) > ^ > > /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: > note: previous declaration is here > extern char *optarg;/* getopt(3) external variables */ >^ > > Signed-off-by: Philippe Mathieu-Daudé > --- > include/sysemu/os-posix.h | 4 ++-- > os-posix.c| 12 ++-- > 2 files changed, 8 insertions(+), 8 deletions(-) > Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org
Re: [PATCH 21/21] qapi: Remove 'qapi/qmp/qerror.h' header
Philippe Mathieu-Daudé wrote: > This file is now empty. Avoid new definitions by killing it, > paying off a 8 years old debt (with interests). > > Mechanical change using: > > $ git grep -l qapi/qmp/qerror.h | while read f; do \ > gawk -i inplace '/#include "qapi\/qmp\/qerror.h"/ && !p {p++;next}1' > $f; \ > done > > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Juan Quintela
Re: [PATCH 14/21] qapi: Inline and remove QERR_MIGRATION_ACTIVE definition
Philippe Mathieu-Daudé wrote: > Address the comment added in commit 4629ed1e98 > ("qerror: Finally unused, clean up"), from 2015: > > /* >* These macros will go away, please don't use >* in new code, and do not add new ones! >*/ > > Mechanical transformation using sed, manually > removing the definition in include/qapi/qmp/qerror.h. > > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Juan Quintela
Re: [PATCH 13/21] qapi: Inline and remove QERR_IO_ERROR definition
Philippe Mathieu-Daudé wrote: > Address the comment added in commit 4629ed1e98 > ("qerror: Finally unused, clean up"), from 2015: > > /* >* These macros will go away, please don't use >* in new code, and do not add new ones! >*/ > > Mechanical transformation using: > > $ sed -i -e 's/QERR_IO_ERROR/"An IO error has occurred"/' \ > $(git grep -wl QERR_IO_ERROR) > > then manually removing the definition in include/qapi/qmp/qerror.h. > > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Juan Quintela
Re: [PATCH 10/21] qapi: Inline QERR_INVALID_PARAMETER_VALUE definition (constant value)
Philippe Mathieu-Daudé wrote: > Address the comment added in commit 4629ed1e98 > ("qerror: Finally unused, clean up"), from 2015: > > /* >* These macros will go away, please don't use >* in new code, and do not add new ones! >*/ > > Mechanical transformation using the following > coccinelle semantic patch: > > @match@ > expression errp; > constant param; > constant value; > @@ > error_setg(errp, QERR_INVALID_PARAMETER_VALUE, param, value); > > @script:python strformat depends on match@ > param << match.param; > value << match.value; > fixedfmt; // new var > @@ > fixedfmt = "\"Parameter '%s' expects %s\"" % (param[1:-1], value[1:-1]) > coccinelle.fixedfmt = cocci.make_ident(fixedfmt) > > @replace@ > expression match.errp; > constant match.param; > constant match.value; > identifier strformat.fixedfmt; > @@ > -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, param, value); > +error_setg(errp, fixedfmt); > > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Juan Quintela And like the approach, but > if (granularity != 0 && (granularity < 512 || granularity > 1048576 * > 64)) { > -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "granularity", > - "a value in range [512B, 64MB]"); > +error_setg(errp, > + "Parameter 'granularity' expects a value in range [512B, > 64MB]"); > return; There are several lines like this one that become way bigger than 80 characters. Later, Juan. PD. No, I have no clue about how to convince coccinelle to obey qemu indentation rules.
[PATCH 12/21] qapi: Inline and remove QERR_INVALID_PARAMETER_VALUE definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Manually modify the error_report() call in softmmu/tpm.c, then use sed to mechanically transform the rest. Finally remove the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé --- include/qapi/qmp/qerror.h | 3 --- qapi/opts-visitor.c | 4 ++-- qapi/qapi-visit-core.c| 4 ++-- softmmu/tpm.c | 3 +-- 4 files changed, 5 insertions(+), 9 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index b723830eff..ac727d1c2d 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_INVALID_PARAMETER_VALUE \ -"Parameter '%s' expects %s" - #define QERR_IO_ERROR \ "An IO error has occurred" diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c index 0393704a73..844db583f4 100644 --- a/qapi/opts-visitor.c +++ b/qapi/opts-visitor.c @@ -441,7 +441,7 @@ opts_type_int64(Visitor *v, const char *name, int64_t *obj, Error **errp) } } } -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name, +error_setg(errp, "Parameter '%s' expects %s", opt->name, (ov->list_mode == LM_NONE) ? "an int64 value" : "an int64 value or range"); return false; @@ -494,7 +494,7 @@ opts_type_uint64(Visitor *v, const char *name, uint64_t *obj, Error **errp) } } } -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name, +error_setg(errp, "Parameter '%s' expects %s", opt->name, (ov->list_mode == LM_NONE) ? "a uint64 value" : "a uint64 value or range"); return false; diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c index 6c13510a2b..01793d6e74 100644 --- a/qapi/qapi-visit-core.c +++ b/qapi/qapi-visit-core.c @@ -194,7 +194,7 @@ static bool visit_type_uintN(Visitor *v, uint64_t *obj, const char *name, } if (value > max) { assert(v->type == VISITOR_INPUT); -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, +error_setg(errp, "Parameter '%s' expects %s", name ? name : "null", type); return false; } @@ -262,7 +262,7 @@ static bool visit_type_intN(Visitor *v, int64_t *obj, const char *name, } if (value < min || value > max) { assert(v->type == VISITOR_INPUT); -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, +error_setg(errp, "Parameter '%s' expects %s", name ? name : "null", type); return false; } diff --git a/softmmu/tpm.c b/softmmu/tpm.c index 578563f05a..8437c4efc3 100644 --- a/softmmu/tpm.c +++ b/softmmu/tpm.c @@ -120,8 +120,7 @@ static int tpm_init_tpmdev(void *dummy, QemuOpts *opts, Error **errp) i = qapi_enum_parse(_lookup, value, -1, NULL); be = i >= 0 ? tpm_be_find_by_type(i) : NULL; if (be == NULL) { -error_report(QERR_INVALID_PARAMETER_VALUE, - "type", "a TPM backend type"); +error_report("Parameter 'type' expects a TPM backend type"); tpm_display_backend_drivers(); return 1; } -- 2.41.0
Re: [PATCH 07/21] qapi: Inline QERR_INVALID_PARAMETER_TYPE definition (constant param)
On 04/10/2023 19.31, Philippe Mathieu-Daudé wrote: Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using the following coccinelle semantic patch: @match@ expression errp; constant param; constant value; @@ error_setg(errp, QERR_INVALID_PARAMETER_TYPE, param, value); @script:python strformat depends on match@ param << match.param; value << match.value; fixedfmt; // new var @@ fixedfmt = f'"Invalid parameter type for \'{param[1:-1]}\', expected: {value[1:-1]}"' coccinelle.fixedfmt = cocci.make_ident(fixedfmt) @replace@ expression match.errp; constant match.param; constant match.value; identifier strformat.fixedfmt; @@ -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, param, value); +error_setg(errp, fixedfmt); Signed-off-by: Philippe Mathieu-Daudé --- target/arm/arm-qmp-cmds.c| 3 ++- target/s390x/cpu_models_sysemu.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/target/arm/arm-qmp-cmds.c b/target/arm/arm-qmp-cmds.c index b53d5efe13..3c99fd8222 100644 --- a/target/arm/arm-qmp-cmds.c +++ b/target/arm/arm-qmp-cmds.c @@ -154,7 +154,8 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type, if (model->props) { qdict_in = qobject_to(QDict, model->props); if (!qdict_in) { -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, "props", "dict"); +error_setg(errp, + "Invalid parameter type for 'props', expected: dict"); return NULL; } } diff --git a/target/s390x/cpu_models_sysemu.c b/target/s390x/cpu_models_sysemu.c index 63981bf36b..4507714493 100644 --- a/target/s390x/cpu_models_sysemu.c +++ b/target/s390x/cpu_models_sysemu.c @@ -111,7 +111,8 @@ static void cpu_model_from_info(S390CPUModel *model, const CpuModelInfo *info, if (info->props) { qdict = qobject_to(QDict, info->props); if (!qdict) { -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, "props", "dict"); +error_setg(errp, + "Invalid parameter type for 'props', expected: dict"); return; } } Acked-by: Thomas Huth
[PATCH 08/21] qapi: Inline QERR_INVALID_PARAMETER_TYPE definition (constant value)
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using the following coccinelle semantic patch: @match@ expression errp; expression param; constant value; @@ error_setg(errp, QERR_INVALID_PARAMETER_TYPE, param, value); @script:python strformat depends on match@ value << match.value; fixedfmt; // new var @@ fixedfmt = f'"Invalid parameter type for \'%s\', expected: {value[1:-1]}"' coccinelle.fixedfmt = cocci.make_ident(fixedfmt) @replace@ expression match.errp; expression match.param; constant match.value; identifier strformat.fixedfmt; @@ -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, param, value); +error_setg(errp, fixedfmt, param); Signed-off-by: Philippe Mathieu-Daudé --- qapi/qobject-input-visitor.c | 32 qapi/string-input-visitor.c | 8 qom/object.c | 12 3 files changed, 28 insertions(+), 24 deletions(-) diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c index 3e8aca6b15..f110a804b2 100644 --- a/qapi/qobject-input-visitor.c +++ b/qapi/qobject-input-visitor.c @@ -288,8 +288,8 @@ static bool qobject_input_start_struct(Visitor *v, const char *name, void **obj, return false; } if (qobject_type(qobj) != QTYPE_QDICT) { -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, - full_name(qiv, name), "object"); +error_setg(errp, "Invalid parameter type for '%s', expected: object", + full_name(qiv, name)); return false; } @@ -326,8 +326,8 @@ static bool qobject_input_start_list(Visitor *v, const char *name, return false; } if (qobject_type(qobj) != QTYPE_QLIST) { -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, - full_name(qiv, name), "array"); +error_setg(errp, "Invalid parameter type for '%s', expected: array", + full_name(qiv, name)); return false; } @@ -405,8 +405,8 @@ static bool qobject_input_type_int64(Visitor *v, const char *name, int64_t *obj, } qnum = qobject_to(QNum, qobj); if (!qnum || !qnum_get_try_int(qnum, obj)) { -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, - full_name(qiv, name), "integer"); +error_setg(errp, "Invalid parameter type for '%s', expected: integer", + full_name(qiv, name)); return false; } return true; @@ -494,8 +494,8 @@ static bool qobject_input_type_bool(Visitor *v, const char *name, bool *obj, } qbool = qobject_to(QBool, qobj); if (!qbool) { -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, - full_name(qiv, name), "boolean"); +error_setg(errp, "Invalid parameter type for '%s', expected: boolean", + full_name(qiv, name)); return false; } @@ -534,8 +534,8 @@ static bool qobject_input_type_str(Visitor *v, const char *name, char **obj, } qstr = qobject_to(QString, qobj); if (!qstr) { -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, - full_name(qiv, name), "string"); +error_setg(errp, "Invalid parameter type for '%s', expected: string", + full_name(qiv, name)); return false; } @@ -565,8 +565,8 @@ static bool qobject_input_type_number(Visitor *v, const char *name, double *obj, } qnum = qobject_to(QNum, qobj); if (!qnum) { -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, - full_name(qiv, name), "number"); +error_setg(errp, "Invalid parameter type for '%s', expected: number", + full_name(qiv, name)); return false; } @@ -587,8 +587,8 @@ static bool qobject_input_type_number_keyval(Visitor *v, const char *name, if (qemu_strtod_finite(str, NULL, )) { /* TODO report -ERANGE more nicely */ -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, - full_name(qiv, name), "number"); +error_setg(errp, "Invalid parameter type for '%s', expected: number", + full_name(qiv, name)); return false; } @@ -623,8 +623,8 @@ static bool qobject_input_type_null(Visitor *v, const char *name, } if (qobject_type(qobj) != QTYPE_QNULL) { -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, - full_name(qiv, name), "null"); +error_setg(errp, "Invalid parameter type for '%s', expected: null", + full_name(qiv, name)); return false; } *obj = qnull(); diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c index 197139c1c0..3f1b9e9b41 100644 --- a/qapi/string-input-visitor.c +++
[PATCH 16/21] qapi: Inline and remove QERR_MISSING_PARAMETER definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using sed, manually removing the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé --- include/qapi/qmp/qerror.h| 3 --- qapi/opts-visitor.c | 2 +- qapi/qapi-forward-visitor.c | 2 +- qapi/qobject-input-visitor.c | 2 +- 4 files changed, 3 insertions(+), 6 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index cc4dae1076..b0f48f22fe 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_MISSING_PARAMETER \ -"Parameter '%s' is missing" - #define QERR_PROPERTY_VALUE_BAD \ "Property '%s.%s' doesn't take value '%s'" diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c index 844db583f4..bf0d8acbd6 100644 --- a/qapi/opts-visitor.c +++ b/qapi/opts-visitor.c @@ -218,7 +218,7 @@ lookup_distinct(const OptsVisitor *ov, const char *name, Error **errp) list = g_hash_table_lookup(ov->unprocessed_opts, name); if (!list) { -error_setg(errp, QERR_MISSING_PARAMETER, name); +error_setg(errp, "Parameter '%s' is missing", name); } return list; } diff --git a/qapi/qapi-forward-visitor.c b/qapi/qapi-forward-visitor.c index e36d9bc9ba..3fb2c954aa 100644 --- a/qapi/qapi-forward-visitor.c +++ b/qapi/qapi-forward-visitor.c @@ -49,7 +49,7 @@ static bool forward_field_translate_name(ForwardFieldVisitor *v, const char **na *name = v->to; return true; } -error_setg(errp, QERR_MISSING_PARAMETER, *name); +error_setg(errp, "Parameter '%s' is missing", *name); return false; } diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c index f5fa6c1878..17e9f3b638 100644 --- a/qapi/qobject-input-visitor.c +++ b/qapi/qobject-input-visitor.c @@ -168,7 +168,7 @@ static QObject *qobject_input_get_object(QObjectInputVisitor *qiv, QObject *obj = qobject_input_try_get_object(qiv, name, consume); if (!obj) { -error_setg(errp, QERR_MISSING_PARAMETER, full_name(qiv, name)); +error_setg(errp, "Parameter '%s' is missing", full_name(qiv, name)); } return obj; } -- 2.41.0
[PATCH 14/21] qapi: Inline and remove QERR_MIGRATION_ACTIVE definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using sed, manually removing the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé --- include/qapi/qmp/qerror.h | 3 --- migration/migration.c | 2 +- migration/options.c | 4 ++-- migration/savevm.c| 2 +- 4 files changed, 4 insertions(+), 7 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index d95c4b84b9..cc4dae1076 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_MIGRATION_ACTIVE \ -"There's a migration process in progress" - #define QERR_MISSING_PARAMETER \ "Parameter '%s' is missing" diff --git a/migration/migration.c b/migration/migration.c index b7f6818a15..5703cc34ae 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -1616,7 +1616,7 @@ static bool migrate_prepare(MigrationState *s, bool blk, bool blk_inc, } if (migration_is_running(s->state)) { -error_setg(errp, QERR_MIGRATION_ACTIVE); +error_setg(errp, "There's a migration process in progress"); return false; } diff --git a/migration/options.c b/migration/options.c index 3a2180b779..e363b4885f 100644 --- a/migration/options.c +++ b/migration/options.c @@ -618,7 +618,7 @@ bool migrate_cap_set(int cap, bool value, Error **errp) bool new_caps[MIGRATION_CAPABILITY__MAX]; if (migration_is_running(s->state)) { -error_setg(errp, QERR_MIGRATION_ACTIVE); +error_setg(errp, "There's a migration process in progress"); return false; } @@ -662,7 +662,7 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params, bool new_caps[MIGRATION_CAPABILITY__MAX]; if (migration_is_running(s->state) || migration_in_colo_state()) { -error_setg(errp, QERR_MIGRATION_ACTIVE); +error_setg(errp, "There's a migration process in progress"); return; } diff --git a/migration/savevm.c b/migration/savevm.c index 41c7f39ef5..c0e0585bc1 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -1634,7 +1634,7 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp) MigrationStatus status; if (migration_is_running(ms->state)) { -error_setg(errp, QERR_MIGRATION_ACTIVE); +error_setg(errp, "There's a migration process in progress"); return -EINVAL; } -- 2.41.0
[PATCH 18/21] qapi: Inline and remove QERR_PROPERTY_VALUE_OUT_OF_RANGE definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using sed, manually removing the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé --- include/qapi/qmp/qerror.h | 3 --- hw/intc/openpic.c | 3 ++- target/i386/cpu.c | 12 util/block-helpers.c | 3 ++- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index 7862ac55a1..e094f13114 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_PROPERTY_VALUE_OUT_OF_RANGE \ -"Property %s.%s doesn't take value %" PRId64 " (minimum: %" PRId64 ", maximum: %" PRId64 ")" - #define QERR_QGA_COMMAND_FAILED \ "Guest agent command failed, error was '%s'" diff --git a/hw/intc/openpic.c b/hw/intc/openpic.c index a6f91d4bcd..4f6ee930e2 100644 --- a/hw/intc/openpic.c +++ b/hw/intc/openpic.c @@ -1535,7 +1535,8 @@ static void openpic_realize(DeviceState *dev, Error **errp) }; if (opp->nb_cpus > MAX_CPU) { -error_setg(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, +error_setg(errp, "Property %s.%s doesn't take value %" PRId64 + " (minimum: %" PRId64 ", maximum: %" PRId64 ")", TYPE_OPENPIC, "nb_cpus", (uint64_t)opp->nb_cpus, (uint64_t)0, (uint64_t)MAX_CPU); return; diff --git a/target/i386/cpu.c b/target/i386/cpu.c index e5a14885ed..273f865228 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -5086,7 +5086,8 @@ static void x86_cpuid_version_set_family(Object *obj, Visitor *v, return; } if (value < min || value > max) { -error_setg(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "", +error_setg(errp, "Property %s doesn't take value %" PRId64 + " (minimum: %" PRId64 ", maximum: %" PRId64 ")", name ? name : "null", value, min, max); return; } @@ -5126,7 +5127,8 @@ static void x86_cpuid_version_set_model(Object *obj, Visitor *v, return; } if (value < min || value > max) { -error_setg(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "", +error_setg(errp, "Property %s doesn't take value %" PRId64 + " (minimum: %" PRId64 ", maximum: %" PRId64 ")", name ? name : "null", value, min, max); return; } @@ -5161,7 +5163,8 @@ static void x86_cpuid_version_set_stepping(Object *obj, Visitor *v, return; } if (value < min || value > max) { -error_setg(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "", +error_setg(errp, "Property %s doesn't take value %" PRId64 + " (minimum: %" PRId64 ", maximum: %" PRId64 ")", name ? name : "null", value, min, max); return; } @@ -5263,7 +5266,8 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, const char *name, return; } if (value < min || value > max) { -error_setg(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "", +error_setg(errp, "Property %s doesn't take value %" PRId64 + " (minimum: %" PRId64 ", maximum: %" PRId64 ")", name ? name : "null", value, min, max); return; } diff --git a/util/block-helpers.c b/util/block-helpers.c index c4851432f5..de94909bc4 100644 --- a/util/block-helpers.c +++ b/util/block-helpers.c @@ -30,7 +30,8 @@ void check_block_size(const char *id, const char *name, int64_t value, { /* value of 0 means "unset" */ if (value && (value < MIN_BLOCK_SIZE || value > MAX_BLOCK_SIZE)) { -error_setg(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, +error_setg(errp, "Property %s.%s doesn't take value %" PRId64 + " (minimum: %" PRId64 ", maximum: %" PRId64 ")", id, name, value, MIN_BLOCK_SIZE, MAX_BLOCK_SIZE); return; } -- 2.41.0
[PATCH 19/21] qapi: Inline and remove QERR_QGA_COMMAND_FAILED definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using the following coccinelle semantic patch: @match exists@ expression errp; expression errmsg; @@ error_setg(errp, QERR_QGA_COMMAND_FAILED, errmsg); @script:python strformat depends on match@ errmsg << match.errmsg; fixedfmt; // new var @@ # Format skipping '"'. fixedfmt = f'"Guest agent command failed, error was \'{errmsg[1:-1]}\'"' coccinelle.fixedfmt = cocci.make_ident(fixedfmt) @replace@ expression match.errp; expression match.errmsg; identifier strformat.fixedfmt; @@ -error_setg(errp, QERR_QGA_COMMAND_FAILED, errmsg); +error_setg(errp, fixedfmt); then manually removing the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé --- include/qapi/qmp/qerror.h | 3 --- qga/commands-win32.c | 38 -- qga/commands.c| 7 --- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index e094f13114..840831cc6a 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_QGA_COMMAND_FAILED \ -"Guest agent command failed, error was '%s'" - #define QERR_UNSUPPORTED \ "this feature or command is not currently supported" diff --git a/qga/commands-win32.c b/qga/commands-win32.c index 946dbafbb6..aa8c9770d4 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -245,7 +245,8 @@ int64_t qmp_guest_file_open(const char *path, const char *mode, Error **errp) done: if (gerr) { -error_setg(errp, QERR_QGA_COMMAND_FAILED, gerr->message); +error_setg(errp, + "Guest agent command failed, error was 'err -> messag'"); g_error_free(gerr); } g_free(w_path); @@ -279,8 +280,8 @@ static void acquire_privilege(const char *name, Error **errp) TOKEN_ADJUST_PRIVILEGES | TOKEN_QUERY, )) { if (!LookupPrivilegeValue(NULL, name, [0].Luid)) { -error_setg(errp, QERR_QGA_COMMAND_FAILED, - "no luid for requested privilege"); +error_setg(errp, + "Guest agent command failed, error was 'no luid for requested privilege'"); goto out; } @@ -288,14 +289,14 @@ static void acquire_privilege(const char *name, Error **errp) priv.Privileges[0].Attributes = SE_PRIVILEGE_ENABLED; if (!AdjustTokenPrivileges(token, FALSE, , 0, NULL, 0)) { -error_setg(errp, QERR_QGA_COMMAND_FAILED, - "unable to acquire requested privilege"); +error_setg(errp, + "Guest agent command failed, error was 'unable to acquire requested privilege'"); goto out; } } else { -error_setg(errp, QERR_QGA_COMMAND_FAILED, - "failed to open privilege token"); +error_setg(errp, + "Guest agent command failed, error was 'failed to open privilege token'"); } out: @@ -309,8 +310,8 @@ static void execute_async(DWORD WINAPI (*func)(LPVOID), LPVOID opaque, { HANDLE thread = CreateThread(NULL, 0, func, opaque, 0, NULL); if (!thread) { -error_setg(errp, QERR_QGA_COMMAND_FAILED, - "failed to dispatch asynchronous command"); +error_setg(errp, + "Guest agent command failed, error was 'failed to dispatch asynchronous command'"); } } @@ -1423,22 +1424,22 @@ static void check_suspend_mode(GuestSuspendMode mode, Error **errp) ZeroMemory(_pwr_caps, sizeof(sys_pwr_caps)); if (!GetPwrCapabilities(_pwr_caps)) { -error_setg(errp, QERR_QGA_COMMAND_FAILED, - "failed to determine guest suspend capabilities"); +error_setg(errp, + "Guest agent command failed, error was 'failed to determine guest suspend capabilities'"); return; } switch (mode) { case GUEST_SUSPEND_MODE_DISK: if (!sys_pwr_caps.SystemS4) { -error_setg(errp, QERR_QGA_COMMAND_FAILED, - "suspend-to-disk not supported by OS"); +error_setg(errp, + "Guest agent command failed, error was 'suspend-to-disk not supported by OS'"); } break; case GUEST_SUSPEND_MODE_RAM: if (!sys_pwr_caps.SystemS3) { -error_setg(errp, QERR_QGA_COMMAND_FAILED, - "suspend-to-ram not supported by OS"); +error_setg(errp, + "Guest agent command failed, error was 'suspend-to-ram not supported by OS'"); } break; default: @@ -1971,7
[PATCH 11/21] qapi: Inline QERR_INVALID_PARAMETER_VALUE definition (constant param)
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using the following coccinelle semantic patch: @match@ identifier errp; expression param; // not constant constant value; @@ error_setg(errp, QERR_INVALID_PARAMETER_VALUE, param, value); @script:python strformat depends on match@ param << match.param; value << match.value; fixedfmt; // new var @@ fixedfmt = f"\"Parameter '%s' expects {value[1:-1]}\"" coccinelle.fixedfmt = cocci.make_ident(fixedfmt) @replace@ identifier match.errp; expression match.param; constant match.value; identifier strformat.fixedfmt; @@ -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, param, value); +error_setg(errp, fixedfmt, param); Signed-off-by: Philippe Mathieu-Daudé --- hw/core/qdev-properties-system.c | 5 +++-- qapi/opts-visitor.c | 3 +-- qapi/qapi-util.c | 3 +-- qapi/qobject-input-visitor.c | 18 -- qapi/string-input-visitor.c | 18 ++ util/qemu-option.c | 7 --- 6 files changed, 27 insertions(+), 27 deletions(-) diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c index 688340610e..7752c5fda5 100644 --- a/hw/core/qdev-properties-system.c +++ b/hw/core/qdev-properties-system.c @@ -769,8 +769,9 @@ static void set_pci_devfn(Object *obj, Visitor *v, const char *name, return; } if (value < -1 || value > 255) { -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, - name ? name : "null", "a value between -1 and 255"); +error_setg(errp, + "Parameter '%s' expects a value between -1 and 255", + name ? name : "null"); return; } *ptr = value; diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c index 3d1a28b419..0393704a73 100644 --- a/qapi/opts-visitor.c +++ b/qapi/opts-visitor.c @@ -515,8 +515,7 @@ opts_type_size(Visitor *v, const char *name, uint64_t *obj, Error **errp) err = qemu_strtosz(opt->str ? opt->str : "", NULL, obj); if (err < 0) { -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name, - "a size value"); +error_setg(errp, "Parameter '%s' expects a size value", opt->name); return false; } diff --git a/qapi/qapi-util.c b/qapi/qapi-util.c index 63596e11c5..82c3425566 100644 --- a/qapi/qapi-util.c +++ b/qapi/qapi-util.c @@ -101,8 +101,7 @@ bool qapi_bool_parse(const char *name, const char *value, bool *obj, Error **err return true; } -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name, - "'on' or 'off'"); +error_setg(errp, "Parameter '%s' expects 'on' or 'off'", name); return false; } diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c index f110a804b2..f5fa6c1878 100644 --- a/qapi/qobject-input-visitor.c +++ b/qapi/qobject-input-visitor.c @@ -424,8 +424,8 @@ static bool qobject_input_type_int64_keyval(Visitor *v, const char *name, if (qemu_strtoi64(str, NULL, 0, obj) < 0) { /* TODO report -ERANGE more nicely */ -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, - full_name(qiv, name), "integer"); +error_setg(errp, "Parameter '%s' expects integer", + full_name(qiv, name)); return false; } return true; @@ -458,8 +458,7 @@ static bool qobject_input_type_uint64(Visitor *v, const char *name, } err: -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, - full_name(qiv, name), "uint64"); +error_setg(errp, "Parameter '%s' expects uint64", full_name(qiv, name)); return false; } @@ -475,8 +474,8 @@ static bool qobject_input_type_uint64_keyval(Visitor *v, const char *name, if (qemu_strtou64(str, NULL, 0, obj) < 0) { /* TODO report -ERANGE more nicely */ -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, - full_name(qiv, name), "integer"); +error_setg(errp, "Parameter '%s' expects integer", + full_name(qiv, name)); return false; } return true; @@ -514,8 +513,8 @@ static bool qobject_input_type_bool_keyval(Visitor *v, const char *name, } if (!qapi_bool_parse(name, str, obj, NULL)) { -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, - full_name(qiv, name), "'on' or 'off'"); +error_setg(errp, "Parameter '%s' expects 'on' or 'off'", + full_name(qiv, name)); return false; } return true; @@ -643,8 +642,7 @@ static bool qobject_input_type_size_keyval(Visitor *v, const char *name, if (qemu_strtosz(str, NULL, obj) < 0) { /*
[PATCH 13/21] qapi: Inline and remove QERR_IO_ERROR definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using: $ sed -i -e 's/QERR_IO_ERROR/"An IO error has occurred"/' \ $(git grep -wl QERR_IO_ERROR) then manually removing the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé --- include/qapi/qmp/qerror.h | 3 --- block/vmdk.c | 8 blockdev.c| 2 +- dump/win_dump.c | 4 ++-- migration/savevm.c| 4 ++-- softmmu/cpus.c| 4 ++-- 6 files changed, 11 insertions(+), 14 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index ac727d1c2d..d95c4b84b9 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_IO_ERROR \ -"An IO error has occurred" - #define QERR_MIGRATION_ACTIVE \ "There's a migration process in progress" diff --git a/block/vmdk.c b/block/vmdk.c index e90649c8bf..6779a181f0 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -2246,12 +2246,12 @@ vmdk_init_extent(BlockBackend *blk, int64_t filesize, bool flat, bool compress, /* write all the data */ ret = blk_co_pwrite(blk, 0, sizeof(magic), , 0); if (ret < 0) { -error_setg(errp, QERR_IO_ERROR); +error_setg(errp, "An IO error has occurred"); goto exit; } ret = blk_co_pwrite(blk, sizeof(magic), sizeof(header), , 0); if (ret < 0) { -error_setg(errp, QERR_IO_ERROR); +error_setg(errp, "An IO error has occurred"); goto exit; } @@ -2271,7 +2271,7 @@ vmdk_init_extent(BlockBackend *blk, int64_t filesize, bool flat, bool compress, ret = blk_co_pwrite(blk, le64_to_cpu(header.rgd_offset) * BDRV_SECTOR_SIZE, gd_buf_size, gd_buf, 0); if (ret < 0) { -error_setg(errp, QERR_IO_ERROR); +error_setg(errp, "An IO error has occurred"); goto exit; } @@ -2283,7 +2283,7 @@ vmdk_init_extent(BlockBackend *blk, int64_t filesize, bool flat, bool compress, ret = blk_co_pwrite(blk, le64_to_cpu(header.gd_offset) * BDRV_SECTOR_SIZE, gd_buf_size, gd_buf, 0); if (ret < 0) { -error_setg(errp, QERR_IO_ERROR); +error_setg(errp, "An IO error has occurred"); } ret = 0; diff --git a/blockdev.c b/blockdev.c index ed90365329..228cebf9a2 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1433,7 +1433,7 @@ static void external_snapshot_action(TransactionAction *action, if (!bdrv_is_read_only(state->old_bs)) { if (bdrv_flush(state->old_bs)) { -error_setg(errp, QERR_IO_ERROR); +error_setg(errp, "An IO error has occurred"); goto out; } } diff --git a/dump/win_dump.c b/dump/win_dump.c index b7bfaff379..0115a609e0 100644 --- a/dump/win_dump.c +++ b/dump/win_dump.c @@ -67,7 +67,7 @@ static size_t write_run(uint64_t base_page, uint64_t page_count, l = qemu_write_full(fd, buf, len); cpu_physical_memory_unmap(buf, addr, false, len); if (l != len) { -error_setg(errp, QERR_IO_ERROR); +error_setg(errp, "An IO error has occurred"); return 0; } @@ -459,7 +459,7 @@ void create_win_dump(DumpState *s, Error **errp) s->written_size = qemu_write_full(s->fd, h, hdr_size); if (s->written_size != hdr_size) { -error_setg(errp, QERR_IO_ERROR); +error_setg(errp, "An IO error has occurred"); goto out_restore; } diff --git a/migration/savevm.c b/migration/savevm.c index bb3e99194c..41c7f39ef5 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -3099,7 +3099,7 @@ void qmp_xen_save_devices_state(const char *filename, bool has_live, bool live, object_unref(OBJECT(ioc)); ret = qemu_save_device_state(f); if (ret < 0 || qemu_fclose(f) < 0) { -error_setg(errp, QERR_IO_ERROR); +error_setg(errp, "An IO error has occurred"); } else { /* libxl calls the QMP command "stop" before calling * "xen-save-devices-state" and in case of migration failure, libxl @@ -3148,7 +3148,7 @@ void qmp_xen_load_devices_state(const char *filename, Error **errp) ret = qemu_loadvm_state(f); qemu_fclose(f); if (ret < 0) { -error_setg(errp, QERR_IO_ERROR); +error_setg(errp, "An IO error has occurred"); } migration_incoming_state_destroy(); } diff --git a/softmmu/cpus.c b/softmmu/cpus.c index 7fc70f9166..f7c743b0ce 100644 --- a/softmmu/cpus.c +++ b/softmmu/cpus.c @@ -773,7 +773,7 @@ void qmp_memsave(int64_t addr, int64_t size, const char *filename, goto exit; } if (fwrite(buf, 1, l, f) != l) { -error_setg(errp, QERR_IO_ERROR); +error_setg(errp, "An IO error has
[RFC PATCH 20/21] qapi: Inline and remove QERR_UNSUPPORTED definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using: $ sed -i -e 's/QERR_UNSUPPORTED/"this feature or command is not currently supported"/' \ $(git grep -wl QERR_UNSUPPORTED) then manually removing the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé --- RFC: Not sure what is the best way to change the comment in qga/qapi-schema.json... --- qga/qapi-schema.json | 5 +++-- include/qapi/qmp/qerror.h | 3 --- qga/commands-bsd.c| 8 +++ qga/commands-posix.c | 46 +++ qga/commands-win32.c | 22 +-- 5 files changed, 41 insertions(+), 43 deletions(-) diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json index b720dd4379..51683f4dc2 100644 --- a/qga/qapi-schema.json +++ b/qga/qapi-schema.json @@ -6,8 +6,9 @@ # # "unsupported" is a higher-level error than the errors that # individual commands might document. The caller should always be -# prepared to receive QERR_UNSUPPORTED, even if the given command -# doesn't specify it, or doesn't document any failure mode at all. +# prepared to receive the "this feature or command is not currently supported" +# message, even if the given command doesn't specify it, or doesn't document +# any failure mode at all. ## ## diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index 840831cc6a..7606f4525d 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,7 +17,4 @@ * add new ones! */ -#define QERR_UNSUPPORTED \ -"this feature or command is not currently supported" - #endif /* QERROR_H */ diff --git a/qga/commands-bsd.c b/qga/commands-bsd.c index 17bddda1cf..11536f148f 100644 --- a/qga/commands-bsd.c +++ b/qga/commands-bsd.c @@ -152,25 +152,25 @@ int qmp_guest_fsfreeze_do_thaw(Error **errp) GuestFilesystemInfoList *qmp_guest_get_fsinfo(Error **errp) { -error_setg(errp, QERR_UNSUPPORTED); +error_setg(errp, "this feature or command is not currently supported"); return NULL; } GuestDiskInfoList *qmp_guest_get_disks(Error **errp) { -error_setg(errp, QERR_UNSUPPORTED); +error_setg(errp, "this feature or command is not currently supported"); return NULL; } GuestDiskStatsInfoList *qmp_guest_get_diskstats(Error **errp) { -error_setg(errp, QERR_UNSUPPORTED); +error_setg(errp, "this feature or command is not currently supported"); return NULL; } GuestCpuStatsList *qmp_guest_get_cpustats(Error **errp) { -error_setg(errp, QERR_UNSUPPORTED); +error_setg(errp, "this feature or command is not currently supported"); return NULL; } #endif /* CONFIG_FSFREEZE */ diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 6169bbf7a0..f510add366 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -165,7 +165,7 @@ void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp) } if (!hwclock_available) { -error_setg(errp, QERR_UNSUPPORTED); +error_setg(errp, "this feature or command is not currently supported"); return; } @@ -1540,7 +1540,7 @@ GuestDiskInfoList *qmp_guest_get_disks(Error **errp) GuestDiskInfoList *qmp_guest_get_disks(Error **errp) { -error_setg(errp, QERR_UNSUPPORTED); +error_setg(errp, "this feature or command is not currently supported"); return NULL; } @@ -2235,7 +2235,7 @@ void qmp_guest_set_user_password(const char *username, bool crypted, Error **errp) { -error_setg(errp, QERR_UNSUPPORTED); +error_setg(errp, "this feature or command is not currently supported"); } #endif /* __linux__ || __FreeBSD__ */ @@ -2751,47 +2751,47 @@ GuestCpuStatsList *qmp_guest_get_cpustats(Error **errp) void qmp_guest_suspend_disk(Error **errp) { -error_setg(errp, QERR_UNSUPPORTED); +error_setg(errp, "this feature or command is not currently supported"); } void qmp_guest_suspend_ram(Error **errp) { -error_setg(errp, QERR_UNSUPPORTED); +error_setg(errp, "this feature or command is not currently supported"); } void qmp_guest_suspend_hybrid(Error **errp) { -error_setg(errp, QERR_UNSUPPORTED); +error_setg(errp, "this feature or command is not currently supported"); } GuestLogicalProcessorList *qmp_guest_get_vcpus(Error **errp) { -error_setg(errp, QERR_UNSUPPORTED); +error_setg(errp, "this feature or command is not currently supported"); return NULL; } int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp) { -error_setg(errp, QERR_UNSUPPORTED); +error_setg(errp, "this feature or command is not currently supported"); return -1; } GuestMemoryBlockList *qmp_guest_get_memory_blocks(Error **errp) { -
[PATCH 21/21] qapi: Remove 'qapi/qmp/qerror.h' header
This file is now empty. Avoid new definitions by killing it, paying off a 8 years old debt (with interests). Mechanical change using: $ git grep -l qapi/qmp/qerror.h | while read f; do \ gawk -i inplace '/#include "qapi\/qmp\/qerror.h"/ && !p {p++;next}1' $f; \ done Signed-off-by: Philippe Mathieu-Daudé --- include/qapi/qmp/qerror.h| 20 backends/cryptodev-vhost-user.c | 1 - backends/dbus-vmstate.c | 1 - backends/rng-egd.c | 1 - backends/rng-random.c| 1 - block/gluster.c | 1 - block/monitor/block-hmp-cmds.c | 1 - block/quorum.c | 1 - block/snapshot.c | 1 - block/vmdk.c | 1 - blockdev.c | 1 - blockjob.c | 1 - chardev/char-fe.c| 1 - chardev/char.c | 1 - dump/dump.c | 1 - dump/win_dump.c | 1 - hw/core/qdev-properties-system.c | 1 - hw/core/qdev-properties.c| 1 - hw/core/qdev.c | 1 - hw/intc/openpic.c| 1 - hw/ppc/spapr_pci.c | 1 - hw/usb/redirect.c| 1 - migration/migration.c| 1 - migration/options.c | 1 - migration/page_cache.c | 1 - migration/ram.c | 1 - migration/savevm.c | 1 - monitor/fds.c| 1 - monitor/hmp-cmds.c | 1 - monitor/qmp-cmds.c | 1 - net/filter-buffer.c | 1 - net/filter.c | 1 - net/net.c| 1 - qapi/opts-visitor.c | 1 - qapi/qapi-forward-visitor.c | 1 - qapi/qapi-util.c | 1 - qapi/qapi-visit-core.c | 1 - qapi/qobject-input-visitor.c | 1 - qapi/string-input-visitor.c | 1 - qga/commands-bsd.c | 1 - qga/commands-posix.c | 1 - qga/commands-win32.c | 1 - qga/commands.c | 1 - qom/object.c | 1 - qom/object_interfaces.c | 1 - qom/qom-qmp-cmds.c | 1 - softmmu/balloon.c| 1 - softmmu/cpus.c | 1 - softmmu/qdev-monitor.c | 1 - softmmu/rtc.c| 1 - softmmu/tpm.c| 1 - softmmu/vl.c | 1 - target/arm/arm-qmp-cmds.c| 1 - target/i386/cpu.c| 1 - target/s390x/cpu_models_sysemu.c | 1 - ui/input-barrier.c | 1 - ui/ui-qmp-cmds.c | 1 - util/block-helpers.c | 1 - util/qemu-option.c | 1 - scripts/qapi/visit.py| 1 - 60 files changed, 79 deletions(-) delete mode 100644 include/qapi/qmp/qerror.h diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h deleted file mode 100644 index 7606f4525d..00 --- a/include/qapi/qmp/qerror.h +++ /dev/null @@ -1,20 +0,0 @@ -/* - * QError Module - * - * Copyright (C) 2009 Red Hat Inc. - * - * Authors: - * Luiz Capitulino - * - * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. - * See the COPYING.LIB file in the top-level directory. - */ -#ifndef QERROR_H -#define QERROR_H - -/* - * These macros will go away, please don't use in new code, and do not - * add new ones! - */ - -#endif /* QERROR_H */ diff --git a/backends/cryptodev-vhost-user.c b/backends/cryptodev-vhost-user.c index d93ccd5528..5a41aa7be8 100644 --- a/backends/cryptodev-vhost-user.c +++ b/backends/cryptodev-vhost-user.c @@ -23,7 +23,6 @@ #include "qemu/osdep.h" #include "qapi/error.h" -#include "qapi/qmp/qerror.h" #include "qemu/error-report.h" #include "hw/virtio/vhost-user.h" #include "standard-headers/linux/virtio_crypto.h" diff --git a/backends/dbus-vmstate.c b/backends/dbus-vmstate.c index e781ded17c..0006f8c400 100644 --- a/backends/dbus-vmstate.c +++ b/backends/dbus-vmstate.c @@ -16,7 +16,6 @@ #include "qemu/error-report.h" #include "qapi/error.h" #include "qom/object_interfaces.h" -#include "qapi/qmp/qerror.h" #include "migration/vmstate.h" #include "trace.h" #include "qom/object.h" diff --git a/backends/rng-egd.c b/backends/rng-egd.c index 8f101afadc..35f19257bd 100644 --- a/backends/rng-egd.c +++ b/backends/rng-egd.c @@ -14,7 +14,6 @@ #include "sysemu/rng.h" #include "chardev/char-fe.h" #include "qapi/error.h" -#include "qapi/qmp/qerror.h" #include "qemu/module.h" #include "qom/object.h" diff --git a/backends/rng-random.c b/backends/rng-random.c index 9cb7d26cb5..a49e4a4970 100644 --- a/backends/rng-random.c +++ b/backends/rng-random.c @@ -14,7 +14,6 @@ #include "sysemu/rng-random.h" #include "sysemu/rng.h" #include "qapi/error.h" -#include "qapi/qmp/qerror.h" #include "qemu/main-loop.h" #include "qemu/module.h" diff --git a/block/gluster.c b/block/gluster.c index
[PATCH 10/21] qapi: Inline QERR_INVALID_PARAMETER_VALUE definition (constant value)
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using the following coccinelle semantic patch: @match@ expression errp; constant param; constant value; @@ error_setg(errp, QERR_INVALID_PARAMETER_VALUE, param, value); @script:python strformat depends on match@ param << match.param; value << match.value; fixedfmt; // new var @@ fixedfmt = "\"Parameter '%s' expects %s\"" % (param[1:-1], value[1:-1]) coccinelle.fixedfmt = cocci.make_ident(fixedfmt) @replace@ expression match.errp; constant match.param; constant match.value; identifier strformat.fixedfmt; @@ -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, param, value); +error_setg(errp, fixedfmt); Signed-off-by: Philippe Mathieu-Daudé --- backends/cryptodev-vhost-user.c | 4 +- backends/rng-egd.c | 4 +- backends/rng-random.c | 3 +- block/quorum.c | 3 +- blockdev.c | 9 ++-- blockjob.c | 3 +- chardev/char.c | 3 +- hw/usb/redirect.c | 4 +- migration/migration.c | 4 +- migration/options.c | 94 + migration/page_cache.c | 8 +-- migration/ram.c | 4 +- monitor/fds.c | 8 +-- monitor/qmp-cmds.c | 3 +- net/filter-buffer.c | 3 +- net/filter.c| 7 ++- net/net.c | 9 ++-- qga/commands-win32.c| 4 +- qom/object_interfaces.c | 2 +- qom/qom-qmp-cmds.c | 7 ++- softmmu/balloon.c | 2 +- softmmu/cpus.c | 3 +- softmmu/qdev-monitor.c | 11 ++-- ui/ui-qmp-cmds.c| 2 +- util/qemu-option.c | 3 +- 25 files changed, 88 insertions(+), 119 deletions(-) diff --git a/backends/cryptodev-vhost-user.c b/backends/cryptodev-vhost-user.c index c3283ba84a..d93ccd5528 100644 --- a/backends/cryptodev-vhost-user.c +++ b/backends/cryptodev-vhost-user.c @@ -136,8 +136,8 @@ cryptodev_vhost_claim_chardev(CryptoDevBackendVhostUser *s, Chardev *chr; if (s->chr_name == NULL) { -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, - "chardev", "a valid character device"); +error_setg(errp, + "Parameter 'chardev' expects a valid character device"); return NULL; } diff --git a/backends/rng-egd.c b/backends/rng-egd.c index 684c3cf3d6..8f101afadc 100644 --- a/backends/rng-egd.c +++ b/backends/rng-egd.c @@ -90,8 +90,8 @@ static void rng_egd_opened(RngBackend *b, Error **errp) Chardev *chr; if (s->chr_name == NULL) { -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, - "chardev", "a valid character device"); +error_setg(errp, + "Parameter 'chardev' expects a valid character device"); return; } diff --git a/backends/rng-random.c b/backends/rng-random.c index 80eb5be138..9cb7d26cb5 100644 --- a/backends/rng-random.c +++ b/backends/rng-random.c @@ -72,8 +72,7 @@ static void rng_random_opened(RngBackend *b, Error **errp) RngRandom *s = RNG_RANDOM(b); if (s->filename == NULL) { -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, - "filename", "a valid filename"); +error_setg(errp, "Parameter 'filename' expects a valid filename"); } else { s->fd = qemu_open_old(s->filename, O_RDONLY | O_NONBLOCK); if (s->fd == -1) { diff --git a/block/quorum.c b/block/quorum.c index 05220cab7f..8e9f279568 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -882,8 +882,7 @@ static int quorum_valid_threshold(int threshold, int num_children, Error **errp) { if (threshold < 1) { -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, - "vote-threshold", "a value >= 1"); +error_setg(errp, "Parameter 'vote-threshold' expects a value >= 1"); return -ERANGE; } diff --git a/blockdev.c b/blockdev.c index da39da457e..ed90365329 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2340,7 +2340,7 @@ void coroutine_fn qmp_block_resize(const char *device, const char *node_name, } if (size < 0) { -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "size", "a >0 size"); +error_setg(errp, "Parameter 'size' expects a >0 size"); return; } @@ -2905,13 +2905,12 @@ static void blockdev_mirror_common(const char *job_id, BlockDriverState *bs, } if (granularity != 0 && (granularity < 512 || granularity > 1048576 * 64)) { -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "granularity", - "a value in range [512B, 64MB]"); +
[PATCH 03/21] qapi: Inline and remove QERR_DEVICE_IN_USE definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using sed, manually removing the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé --- include/qapi/qmp/qerror.h | 3 --- blockdev.c| 2 +- chardev/char-fe.c | 2 +- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index 168177bcd7..daa889809b 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_DEVICE_IN_USE \ -"Device '%s' is in use" - #define QERR_DEVICE_NO_HOTPLUG \ "Device '%s' does not support hotplugging" diff --git a/blockdev.c b/blockdev.c index e5617faf0f..da39da457e 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2345,7 +2345,7 @@ void coroutine_fn qmp_block_resize(const char *device, const char *node_name, } if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_RESIZE, NULL)) { -error_setg(errp, QERR_DEVICE_IN_USE, device); +error_setg(errp, "Device '%s' is in use", device); return; } diff --git a/chardev/char-fe.c b/chardev/char-fe.c index 7789f7be9c..7d33b3ccd1 100644 --- a/chardev/char-fe.c +++ b/chardev/char-fe.c @@ -217,7 +217,7 @@ bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error **errp) return true; unavailable: -error_setg(errp, QERR_DEVICE_IN_USE, s->label); +error_setg(errp, "Device '%s' is in use", s->label); return false; } -- 2.41.0
[PATCH 07/21] qapi: Inline QERR_INVALID_PARAMETER_TYPE definition (constant param)
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using the following coccinelle semantic patch: @match@ expression errp; constant param; constant value; @@ error_setg(errp, QERR_INVALID_PARAMETER_TYPE, param, value); @script:python strformat depends on match@ param << match.param; value << match.value; fixedfmt; // new var @@ fixedfmt = f'"Invalid parameter type for \'{param[1:-1]}\', expected: {value[1:-1]}"' coccinelle.fixedfmt = cocci.make_ident(fixedfmt) @replace@ expression match.errp; constant match.param; constant match.value; identifier strformat.fixedfmt; @@ -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, param, value); +error_setg(errp, fixedfmt); Signed-off-by: Philippe Mathieu-Daudé --- target/arm/arm-qmp-cmds.c| 3 ++- target/s390x/cpu_models_sysemu.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/target/arm/arm-qmp-cmds.c b/target/arm/arm-qmp-cmds.c index b53d5efe13..3c99fd8222 100644 --- a/target/arm/arm-qmp-cmds.c +++ b/target/arm/arm-qmp-cmds.c @@ -154,7 +154,8 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type, if (model->props) { qdict_in = qobject_to(QDict, model->props); if (!qdict_in) { -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, "props", "dict"); +error_setg(errp, + "Invalid parameter type for 'props', expected: dict"); return NULL; } } diff --git a/target/s390x/cpu_models_sysemu.c b/target/s390x/cpu_models_sysemu.c index 63981bf36b..4507714493 100644 --- a/target/s390x/cpu_models_sysemu.c +++ b/target/s390x/cpu_models_sysemu.c @@ -111,7 +111,8 @@ static void cpu_model_from_info(S390CPUModel *model, const CpuModelInfo *info, if (info->props) { qdict = qobject_to(QDict, info->props); if (!qdict) { -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, "props", "dict"); +error_setg(errp, + "Invalid parameter type for 'props', expected: dict"); return; } } -- 2.41.0
[PATCH 09/21] qapi: Inline and remove QERR_INVALID_PARAMETER_TYPE definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Manual changes (escaping the format in qapi/visit.py). Remove the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé --- include/qapi/qmp/qerror.h | 3 --- qom/object.c | 3 ++- scripts/qapi/visit.py | 4 ++-- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index 63ab775dc5..b723830eff 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_INVALID_PARAMETER_TYPE \ -"Invalid parameter type for '%s', expected: %s" - #define QERR_INVALID_PARAMETER_VALUE \ "Parameter '%s' expects %s" diff --git a/qom/object.c b/qom/object.c index 890fa0a106..eea61a5068 100644 --- a/qom/object.c +++ b/qom/object.c @@ -1855,7 +1855,8 @@ static Object *object_resolve_link(Object *obj, const char *name, } else if (!target) { target = object_resolve_path(path, ); if (target || ambiguous) { -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name, target_type); +error_setg(errp, "Invalid parameter type for '%s', expected: %s", + name, target_type); } else { error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, "Device '%s' not found", path); diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py index c56ea4d724..4b4a442383 100644 --- a/scripts/qapi/visit.py +++ b/scripts/qapi/visit.py @@ -278,8 +278,8 @@ def gen_visit_alternate(name: str, variants: QAPISchemaVariants) -> str: abort(); default: assert(visit_is_input(v)); -error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "%(name)s"); +error_setg(errp, "Invalid parameter type for '%%s', expected: %(name)s", + name ? name : "null"); /* Avoid passing invalid *obj to qapi_free_%(c_name)s() */ g_free(*obj); *obj = NULL; -- 2.41.0
[PATCH 15/21] qapi: Inline QERR_MISSING_PARAMETER definition (constant parameter)
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using the following coccinelle semantic patches: @match@ expression errp; constant param; @@ error_setg(errp, QERR_MISSING_PARAMETER, param); @script:python strformat depends on match@ param << match.param; fixedfmt; // new var @@ if param[0] == '"': # Format skipping '"', fixedfmt = f'"Parameter \'{param[1:-1]}\' is missing"' else: # or use definition. fixedfmt = f'"Parameter " {param} " is missing"' coccinelle.fixedfmt = cocci.make_ident(fixedfmt) @replace@ expression match.errp; constant match.param; identifier strformat.fixedfmt; @@ -error_setg(errp, QERR_MISSING_PARAMETER, param); +error_setg(errp, fixedfmt); and: @match@ constant param; @@ error_report(QERR_MISSING_PARAMETER, param); @script:python strformat depends on match@ param << match.param; fixedfmt; // new var @@ fixedfmt = f'"Parameter \'{param[1:-1]}\' is missing"' coccinelle.fixedfmt = cocci.make_ident(fixedfmt) @replace@ constant match.param; identifier strformat.fixedfmt; @@ -error_report(QERR_MISSING_PARAMETER, param); +error_report(fixedfmt); Signed-off-by: Philippe Mathieu-Daudé --- backends/dbus-vmstate.c| 2 +- block/gluster.c| 21 +++-- block/monitor/block-hmp-cmds.c | 6 +++--- dump/dump.c| 4 ++-- hw/usb/redirect.c | 2 +- softmmu/qdev-monitor.c | 2 +- softmmu/tpm.c | 4 ++-- softmmu/vl.c | 4 ++-- ui/input-barrier.c | 2 +- ui/ui-qmp-cmds.c | 2 +- 10 files changed, 25 insertions(+), 24 deletions(-) diff --git a/backends/dbus-vmstate.c b/backends/dbus-vmstate.c index 57369ec0f2..e781ded17c 100644 --- a/backends/dbus-vmstate.c +++ b/backends/dbus-vmstate.c @@ -413,7 +413,7 @@ dbus_vmstate_complete(UserCreatable *uc, Error **errp) } if (!self->dbus_addr) { -error_setg(errp, QERR_MISSING_PARAMETER, "addr"); +error_setg(errp, "Parameter 'addr' is missing"); return; } diff --git a/block/gluster.c b/block/gluster.c index ad5fadbe79..8d97d698c3 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -530,20 +530,20 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf, num_servers = qdict_array_entries(options, GLUSTER_OPT_SERVER_PATTERN); if (num_servers < 1) { -error_setg(_err, QERR_MISSING_PARAMETER, "server"); +error_setg(_err, "Parameter 'server' is missing"); goto out; } ptr = qemu_opt_get(opts, GLUSTER_OPT_VOLUME); if (!ptr) { -error_setg(_err, QERR_MISSING_PARAMETER, GLUSTER_OPT_VOLUME); +error_setg(_err, "Parameter " GLUSTER_OPT_VOLUME " is missing"); goto out; } gconf->volume = g_strdup(ptr); ptr = qemu_opt_get(opts, GLUSTER_OPT_PATH); if (!ptr) { -error_setg(_err, QERR_MISSING_PARAMETER, GLUSTER_OPT_PATH); +error_setg(_err, "Parameter " GLUSTER_OPT_PATH " is missing"); goto out; } gconf->path = g_strdup(ptr); @@ -562,7 +562,8 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf, ptr = qemu_opt_get(opts, GLUSTER_OPT_TYPE); if (!ptr) { -error_setg(_err, QERR_MISSING_PARAMETER, GLUSTER_OPT_TYPE); +error_setg(_err, + "Parameter " GLUSTER_OPT_TYPE " is missing"); error_append_hint(_err, GERR_INDEX_HINT, i); goto out; @@ -592,16 +593,16 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf, ptr = qemu_opt_get(opts, GLUSTER_OPT_HOST); if (!ptr) { -error_setg(_err, QERR_MISSING_PARAMETER, - GLUSTER_OPT_HOST); +error_setg(_err, + "Parameter " GLUSTER_OPT_HOST " is missing"); error_append_hint(_err, GERR_INDEX_HINT, i); goto out; } gsconf->u.inet.host = g_strdup(ptr); ptr = qemu_opt_get(opts, GLUSTER_OPT_PORT); if (!ptr) { -error_setg(_err, QERR_MISSING_PARAMETER, - GLUSTER_OPT_PORT); +error_setg(_err, + "Parameter " GLUSTER_OPT_PORT " is missing"); error_append_hint(_err, GERR_INDEX_HINT, i); goto out; } @@ -648,8 +649,8 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf, goto out; } if (!ptr) { -error_setg(_err, QERR_MISSING_PARAMETER, -
[PATCH 00/21] qapi: Kill 'qapi/qmp/qerror.h' for good
Hi, This is kind of a selfish series. I'm really tired to grep and read this comment from 2015 in qapi/qmp/qerror.h: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Besides, these definitions are still added in recent code (see for example commit 09f9ec9913 from June 2023). So let's finish with this 8 years old technical debt. Overall it took me 3h: 1h to find the correct Coccinelle doc about Python use and read it again [*], then 1h to adapt the script for each patch, rest is testing and writing comments, so the scripts used could be used as reference later. Regards, Phil. [*] https://www.lrz.de/services/compute/courses/x_lecturenotes/hspc1w19.pdf Philippe Mathieu-Daudé (21): qapi: Inline and remove QERR_BUS_NO_HOTPLUG definition qapi: Inline and remove QERR_DEVICE_HAS_NO_MEDIUM definition qapi: Inline and remove QERR_DEVICE_IN_USE definition qapi: Inline and remove QERR_DEVICE_NO_HOTPLUG definition qapi: Inline QERR_INVALID_PARAMETER definition (constant parameter) qapi: Inline and remove QERR_INVALID_PARAMETER definition qapi: Inline QERR_INVALID_PARAMETER_TYPE definition (constant param) qapi: Inline QERR_INVALID_PARAMETER_TYPE definition (constant value) qapi: Inline and remove QERR_INVALID_PARAMETER_TYPE definition qapi: Inline QERR_INVALID_PARAMETER_VALUE definition (constant value) qapi: Inline QERR_INVALID_PARAMETER_VALUE definition (constant param) qapi: Inline and remove QERR_INVALID_PARAMETER_VALUE definition qapi: Inline and remove QERR_IO_ERROR definition qapi: Inline and remove QERR_MIGRATION_ACTIVE definition qapi: Inline QERR_MISSING_PARAMETER definition (constant parameter) qapi: Inline and remove QERR_MISSING_PARAMETER definition qapi: Inline and remove QERR_PROPERTY_VALUE_BAD definition qapi: Inline and remove QERR_PROPERTY_VALUE_OUT_OF_RANGE definition qapi: Inline and remove QERR_QGA_COMMAND_FAILED definition RFC qapi: Inline and remove QERR_UNSUPPORTED definition qapi: Remove 'qapi/qmp/qerror.h' header qga/qapi-schema.json | 5 +- include/qapi/qmp/qerror.h| 62 backends/cryptodev-vhost-user.c | 5 +- backends/dbus-vmstate.c | 3 +- backends/rng-egd.c | 5 +- backends/rng-random.c| 4 +- block/gluster.c | 22 +++ block/monitor/block-hmp-cmds.c | 7 +-- block/quorum.c | 4 +- block/snapshot.c | 5 +- block/vmdk.c | 9 ++- blockdev.c | 16 +++--- blockjob.c | 4 +- chardev/char-fe.c| 3 +- chardev/char.c | 4 +- dump/dump.c | 11 ++-- dump/win_dump.c | 5 +- hw/core/qdev-properties-system.c | 6 +- hw/core/qdev-properties.c| 3 +- hw/core/qdev.c | 4 +- hw/intc/openpic.c| 4 +- hw/ppc/spapr_pci.c | 5 +- hw/usb/redirect.c| 7 +-- migration/migration.c| 7 +-- migration/options.c | 99 +--- migration/page_cache.c | 9 ++- migration/ram.c | 5 +- migration/savevm.c | 7 +-- monitor/fds.c| 9 ++- monitor/hmp-cmds.c | 3 +- monitor/qmp-cmds.c | 4 +- net/filter-buffer.c | 4 +- net/filter.c | 8 +-- net/net.c| 10 ++-- qapi/opts-visitor.c | 12 ++-- qapi/qapi-forward-visitor.c | 3 +- qapi/qapi-util.c | 4 +- qapi/qapi-visit-core.c | 5 +- qapi/qobject-input-visitor.c | 53 - qapi/string-input-visitor.c | 27 - qga/commands-bsd.c | 9 ++- qga/commands-posix.c | 47 --- qga/commands-win32.c | 65 ++--- qga/commands.c | 10 ++-- qom/object.c | 16 -- qom/object_interfaces.c | 3 +- qom/qom-qmp-cmds.c | 8 +-- softmmu/balloon.c| 3 +- softmmu/cpus.c | 8 +-- softmmu/qdev-monitor.c | 24 softmmu/rtc.c| 1 - softmmu/tpm.c| 8 +-- softmmu/vl.c | 5 +- target/arm/arm-qmp-cmds.c| 4 +- target/i386/cpu.c| 15 +++-- target/s390x/cpu_models_sysemu.c | 4 +- ui/input-barrier.c | 3 +- ui/ui-qmp-cmds.c | 7 +-- util/block-helpers.c | 4 +- util/qemu-option.c | 21 --- scripts/qapi/visit.py| 5 +- 61 files changed, 305 insertions(+), 437 deletions(-) delete mode 100644 include/qapi/qmp/qerror.h -- 2.41.0
[PATCH 04/21] qapi: Inline and remove QERR_DEVICE_NO_HOTPLUG definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using sed, manually removing the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé --- include/qapi/qmp/qerror.h | 3 --- hw/core/qdev.c| 3 ++- softmmu/qdev-monitor.c| 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index daa889809b..e93211085a 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_DEVICE_NO_HOTPLUG \ -"Device '%s' does not support hotplugging" - #define QERR_INVALID_PARAMETER \ "Invalid parameter '%s'" diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 43d863b0c5..9b62e0573d 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -479,7 +479,8 @@ static void device_set_realized(Object *obj, bool value, Error **errp) static int unattached_count; if (dev->hotplugged && !dc->hotpluggable) { -error_setg(errp, QERR_DEVICE_NO_HOTPLUG, object_get_typename(obj)); +error_setg(errp, "Device '%s' does not support hotplugging", + object_get_typename(obj)); return; } diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c index 3a9740dcbd..a964bd80df 100644 --- a/softmmu/qdev-monitor.c +++ b/softmmu/qdev-monitor.c @@ -911,7 +911,7 @@ void qdev_unplug(DeviceState *dev, Error **errp) } if (!dc->hotpluggable) { -error_setg(errp, QERR_DEVICE_NO_HOTPLUG, +error_setg(errp, "Device '%s' does not support hotplugging", object_get_typename(OBJECT(dev))); return; } -- 2.41.0
[PATCH 17/21] qapi: Inline and remove QERR_PROPERTY_VALUE_BAD definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Manual change. Remove the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé --- include/qapi/qmp/qerror.h | 3 --- hw/core/qdev-properties.c | 2 +- target/i386/cpu.c | 2 +- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index b0f48f22fe..7862ac55a1 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_PROPERTY_VALUE_BAD \ -"Property '%s.%s' doesn't take value '%s'" - #define QERR_PROPERTY_VALUE_OUT_OF_RANGE \ "Property %s.%s doesn't take value %" PRId64 " (minimum: %" PRId64 ", maximum: %" PRId64 ")" diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index 357b8761b5..44fc1686e0 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -682,7 +682,7 @@ void error_set_from_qdev_prop_error(Error **errp, int ret, Object *obj, break; default: case -EINVAL: -error_setg(errp, QERR_PROPERTY_VALUE_BAD, +error_setg(errp, "Property '%s.%s' doesn't take value '%s'", object_get_typename(obj), name, value); break; case -ENOENT: diff --git a/target/i386/cpu.c b/target/i386/cpu.c index ed72883bf3..e5a14885ed 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -5190,7 +5190,7 @@ static void x86_cpuid_set_vendor(Object *obj, const char *value, int i; if (strlen(value) != CPUID_VENDOR_SZ) { -error_setg(errp, QERR_PROPERTY_VALUE_BAD, "", "vendor", value); +error_setg(errp, "Property 'vendor' doesn't take value '%s'", value); return; } -- 2.41.0
[PATCH 06/21] qapi: Inline and remove QERR_INVALID_PARAMETER definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using: $ sed -i -e "s/QERR_INVALID_PARAMETER,/\"Invalid parameter '%s'\",/" \ $(git grep -lw QERR_INVALID_PARAMETER) then manually removing the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé --- include/qapi/qmp/qerror.h | 3 --- monitor/hmp-cmds.c| 2 +- qapi/opts-visitor.c | 2 +- util/qemu-option.c| 8 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index e93211085a..63ab775dc5 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_INVALID_PARAMETER \ -"Invalid parameter '%s'" - #define QERR_INVALID_PARAMETER_TYPE \ "Invalid parameter type for '%s', expected: %s" diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c index 6c559b48c8..9d6533643d 100644 --- a/monitor/hmp-cmds.c +++ b/monitor/hmp-cmds.c @@ -138,7 +138,7 @@ void hmp_sync_profile(Monitor *mon, const QDict *qdict) } else { Error *err = NULL; -error_setg(, QERR_INVALID_PARAMETER, op); +error_setg(, "Invalid parameter '%s'", op); hmp_handle_error(mon, err); } } diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c index 8f1efab8b9..3d1a28b419 100644 --- a/qapi/opts-visitor.c +++ b/qapi/opts-visitor.c @@ -184,7 +184,7 @@ opts_check_struct(Visitor *v, Error **errp) const QemuOpt *first; first = g_queue_peek_head(any); -error_setg(errp, QERR_INVALID_PARAMETER, first->name); +error_setg(errp, "Invalid parameter '%s'", first->name); return false; } return true; diff --git a/util/qemu-option.c b/util/qemu-option.c index fb391a7904..201f7a87f3 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -498,7 +498,7 @@ static bool opt_validate(QemuOpt *opt, Error **errp) desc = find_desc_by_name(list->desc, opt->name); if (!desc && !opts_accepts_any(list)) { -error_setg(errp, QERR_INVALID_PARAMETER, opt->name); +error_setg(errp, "Invalid parameter '%s'", opt->name); return false; } @@ -531,7 +531,7 @@ bool qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val, desc = find_desc_by_name(list->desc, name); if (!desc && !opts_accepts_any(list)) { -error_setg(errp, QERR_INVALID_PARAMETER, name); +error_setg(errp, "Invalid parameter '%s'", name); return false; } @@ -554,7 +554,7 @@ bool qemu_opt_set_number(QemuOpts *opts, const char *name, int64_t val, desc = find_desc_by_name(list->desc, name); if (!desc && !opts_accepts_any(list)) { -error_setg(errp, QERR_INVALID_PARAMETER, name); +error_setg(errp, "Invalid parameter '%s'", name); return false; } @@ -1103,7 +1103,7 @@ bool qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc, Error **errp) QTAILQ_FOREACH(opt, >head, next) { opt->desc = find_desc_by_name(desc, opt->name); if (!opt->desc) { -error_setg(errp, QERR_INVALID_PARAMETER, opt->name); +error_setg(errp, "Invalid parameter '%s'", opt->name); return false; } -- 2.41.0
[PATCH 01/21] qapi: Inline and remove QERR_BUS_NO_HOTPLUG definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using sed, manually removing the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé --- include/qapi/qmp/qerror.h | 3 --- hw/ppc/spapr_pci.c| 4 ++-- softmmu/qdev-monitor.c| 8 +--- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index 8dd9fcb071..1a9c2d3502 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_BUS_NO_HOTPLUG \ -"Bus '%s' does not support hotplugging" - #define QERR_DEVICE_HAS_NO_MEDIUM \ "Device '%s' has no medium" diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 370c5a90f2..7f063f5852 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -1550,7 +1550,7 @@ static void spapr_pci_pre_plug(HotplugHandler *plug_handler, * we need to let them know it's not enabled */ if (plugged_dev->hotplugged) { -error_setg(errp, QERR_BUS_NO_HOTPLUG, +error_setg(errp, "Bus '%s' does not support hotplugging", object_get_typename(OBJECT(phb))); return; } @@ -1671,7 +1671,7 @@ static void spapr_pci_unplug_request(HotplugHandler *plug_handler, SpaprDrc *drc = drc_from_dev(phb, pdev); if (!phb->dr_enabled) { -error_setg(errp, QERR_BUS_NO_HOTPLUG, +error_setg(errp, "Bus '%s' does not support hotplugging", object_get_typename(OBJECT(phb))); return; } diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c index 74f4e41338..3a9740dcbd 100644 --- a/softmmu/qdev-monitor.c +++ b/softmmu/qdev-monitor.c @@ -656,7 +656,8 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts, if (qdev_should_hide_device(opts, from_json, errp)) { if (bus && !qbus_is_hotpluggable(bus)) { -error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name); +error_setg(errp, "Bus '%s' does not support hotplugging", + bus->name); } return NULL; } else if (*errp) { @@ -664,7 +665,7 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts, } if (phase_check(PHASE_MACHINE_READY) && bus && !qbus_is_hotpluggable(bus)) { -error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name); +error_setg(errp, "Bus '%s' does not support hotplugging", bus->name); return NULL; } @@ -904,7 +905,8 @@ void qdev_unplug(DeviceState *dev, Error **errp) } if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) { -error_setg(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name); +error_setg(errp, "Bus '%s' does not support hotplugging", + dev->parent_bus->name); return; } -- 2.41.0
[PATCH 02/21] qapi: Inline and remove QERR_DEVICE_HAS_NO_MEDIUM definition
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using sed, manually removing the definition in include/qapi/qmp/qerror.h. Signed-off-by: Philippe Mathieu-Daudé --- include/qapi/qmp/qerror.h | 3 --- block/snapshot.c | 4 ++-- blockdev.c| 2 +- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index 1a9c2d3502..168177bcd7 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -17,9 +17,6 @@ * add new ones! */ -#define QERR_DEVICE_HAS_NO_MEDIUM \ -"Device '%s' has no medium" - #define QERR_DEVICE_IN_USE \ "Device '%s' is in use" diff --git a/block/snapshot.c b/block/snapshot.c index b86b5b24ad..eb43e957e1 100644 --- a/block/snapshot.c +++ b/block/snapshot.c @@ -345,7 +345,7 @@ int bdrv_snapshot_delete(BlockDriverState *bs, GLOBAL_STATE_CODE(); if (!drv) { -error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, bdrv_get_device_name(bs)); +error_setg(errp, "Device '%s' has no medium", bdrv_get_device_name(bs)); return -ENOMEDIUM; } if (!snapshot_id && !name) { @@ -420,7 +420,7 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs, GLOBAL_STATE_CODE(); if (!drv) { -error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, bdrv_get_device_name(bs)); +error_setg(errp, "Device '%s' has no medium", bdrv_get_device_name(bs)); return -ENOMEDIUM; } if (!snapshot_id && !name) { diff --git a/blockdev.c b/blockdev.c index 325b7a3bef..e5617faf0f 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1422,7 +1422,7 @@ static void external_snapshot_action(TransactionAction *action, bdrv_drained_begin(state->old_bs); if (!bdrv_is_inserted(state->old_bs)) { -error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, device); +error_setg(errp, "Device '%s' has no medium", device); goto out; } -- 2.41.0
[PATCH 05/21] qapi: Inline QERR_INVALID_PARAMETER definition (constant parameter)
Address the comment added in commit 4629ed1e98 ("qerror: Finally unused, clean up"), from 2015: /* * These macros will go away, please don't use * in new code, and do not add new ones! */ Mechanical transformation using the following coccinelle semantic patch: @match@ expression errp; constant param; @@ error_setg(errp, QERR_INVALID_PARAMETER, param); @script:python strformat depends on match@ param << match.param; fixedfmt; // new var @@ fixedfmt = f'"Invalid parameter \'{param[1:-1]}\'"' # Format skipping '"'. coccinelle.fixedfmt = cocci.make_ident(fixedfmt) @replace@ expression match.errp; constant match.param; identifier strformat.fixedfmt; @@ -error_setg(errp, QERR_INVALID_PARAMETER, param); +error_setg(errp, fixedfmt); Signed-off-by: Philippe Mathieu-Daudé --- dump/dump.c| 6 +++--- qga/commands.c | 2 +- ui/ui-qmp-cmds.c | 2 +- util/qemu-option.c | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/dump/dump.c b/dump/dump.c index d4ef713cd0..e173f1f14c 100644 --- a/dump/dump.c +++ b/dump/dump.c @@ -1810,7 +1810,7 @@ static void dump_init(DumpState *s, int fd, bool has_format, s->fd = fd; if (has_filter && !length) { -error_setg(errp, QERR_INVALID_PARAMETER, "length"); +error_setg(errp, "Invalid parameter 'length'"); goto cleanup; } s->filter_area_begin = begin; @@ -1841,7 +1841,7 @@ static void dump_init(DumpState *s, int fd, bool has_format, /* Is the filter filtering everything? */ if (validate_start_block(s) == -1) { -error_setg(errp, QERR_INVALID_PARAMETER, "begin"); +error_setg(errp, "Invalid parameter 'begin'"); goto cleanup; } @@ -2145,7 +2145,7 @@ void qmp_dump_guest_memory(bool paging, const char *file, } if (fd == -1) { -error_setg(errp, QERR_INVALID_PARAMETER, "protocol"); +error_setg(errp, "Invalid parameter 'protocol'"); return; } diff --git a/qga/commands.c b/qga/commands.c index 09c683e263..871210ab0b 100644 --- a/qga/commands.c +++ b/qga/commands.c @@ -154,7 +154,7 @@ GuestExecStatus *qmp_guest_exec_status(int64_t pid, Error **errp) gei = guest_exec_info_find(pid); if (gei == NULL) { -error_setg(errp, QERR_INVALID_PARAMETER, "pid"); +error_setg(errp, "Invalid parameter 'pid'"); return NULL; } diff --git a/ui/ui-qmp-cmds.c b/ui/ui-qmp-cmds.c index debc07d678..41ca0100e7 100644 --- a/ui/ui-qmp-cmds.c +++ b/ui/ui-qmp-cmds.c @@ -44,7 +44,7 @@ void qmp_set_password(SetPasswordOptions *opts, Error **errp) assert(opts->protocol == DISPLAY_PROTOCOL_VNC); if (opts->connected != SET_PASSWORD_ACTION_KEEP) { /* vnc supports "connected=keep" only */ -error_setg(errp, QERR_INVALID_PARAMETER, "connected"); +error_setg(errp, "Invalid parameter 'connected'"); return; } /* diff --git a/util/qemu-option.c b/util/qemu-option.c index eedd08929b..fb391a7904 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -612,7 +612,7 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id, if (list->merge_lists) { if (id) { -error_setg(errp, QERR_INVALID_PARAMETER, "id"); +error_setg(errp, "Invalid parameter 'id'"); return NULL; } opts = qemu_opts_find(list, NULL); -- 2.41.0
Re: deadlock when using iothread during backup_clean()
On 28.09.23 11:06, Fiona Ebner wrote: Am 05.09.23 um 13:42 schrieb Paolo Bonzini: On 9/5/23 12:01, Fiona Ebner wrote: Can we assume block_job_remove_all_bdrv() to always hold the job's AioContext? I think so, see job_unref_locked(), job_prepare_locked() and job_finalize_single_locked(). These call the callbacks that ultimately get to block_job_remove_all_bdrv(). And if yes, can we just tell bdrv_graph_wrlock() that it needs to release that before polling to fix the deadlock? No, but I think it should be released and re-acquired in block_job_remove_all_bdrv() itself. For fixing the backup cancel deadlock, I tried the following: diff --git a/blockjob.c b/blockjob.c index 58c5d64539..fd6132ebfe 100644 --- a/blockjob.c +++ b/blockjob.c @@ -198,7 +198,9 @@ void block_job_remove_all_bdrv(BlockJob *job) * one to make sure that such a concurrent access does not attempt * to process an already freed BdrvChild. */ +aio_context_release(job->job.aio_context); bdrv_graph_wrlock(NULL); +aio_context_acquire(job->job.aio_context); while (job->nodes) { GSList *l = job->nodes; BdrvChild *c = l->data; but it's not enough unfortunately. And I don't just mean with the later deadlock during bdrv_close() (via bdrv_cbw_drop()) as mentioned in the other mail. Even when I got lucky and that deadlock didn't trigger by chance or with an additional change to try and avoid that one diff --git a/block.c b/block.c index e7f349b25c..02d2c4e777 100644 --- a/block.c +++ b/block.c @@ -5165,7 +5165,7 @@ static void bdrv_close(BlockDriverState *bs) bs->drv = NULL; } -bdrv_graph_wrlock(NULL); +bdrv_graph_wrlock(bs); QLIST_FOREACH_SAFE(child, >children, next, next) { bdrv_unref_child(bs, child); } often guest IO would get completely stuck after canceling the backup. There's nothing obvious to me in the backtraces at that point, but it seems the vCPU and main threads running like usual, while the IO thread is stuck in aio_poll(), i.e. never returns from the __ppoll() call. This would happen with both, a VirtIO SCSI and a VirtIO block disk and with both aio=io_uring and aio=threads. When IO is stuck, it may be helpful to look at bs->tracked_requests list in gdb. If there are requests, each one has field .co, which points to the coroutine of request. Next step is to look at coroutine stack. Something like (in gdb): source scripts/qemu-gdb.py qemu coroutine may work. ("may", because it was long ago when I used this last time) I should also mention I'm using fio --name=file --size=4k --direct=1 --rw=randwrite --bs=4k --ioengine=psync --numjobs=5 --runtime=6000 --time_based inside the guest during canceling of the backup. I'd be glad for any pointers what to look for and happy to provide more information. Best Regards, Fiona -- Best regards, Vladimir
Re: deadlock when using iothread during backup_clean()
On 05.09.23 14:25, Fiona Ebner wrote: Am 05.09.23 um 12:01 schrieb Fiona Ebner: Can we assume block_job_remove_all_bdrv() to always hold the job's AioContext? And if yes, can we just tell bdrv_graph_wrlock() that it needs to release that before polling to fix the deadlock? I tried a doing something similar as a proof-of-concept diff --git a/blockjob.c b/blockjob.c index 58c5d64539..1a696241a0 100644 --- a/blockjob.c +++ b/blockjob.c @@ -198,19 +198,19 @@ void block_job_remove_all_bdrv(BlockJob *job) * one to make sure that such a concurrent access does not attempt * to process an already freed BdrvChild. */ -bdrv_graph_wrlock(NULL); while (job->nodes) { GSList *l = job->nodes; BdrvChild *c = l->data; job->nodes = l->next; +bdrv_graph_wrlock(c->bs); bdrv_op_unblock_all(c->bs, job->blocker); bdrv_root_unref_child(c); +bdrv_graph_wrunlock(); g_slist_free_1(l); } -bdrv_graph_wrunlock(); } and while it did get slightly further, I ran into another deadlock with #0 0x7f1941155136 in __ppoll (fds=0x55992068fb20, nfds=2, timeout=, sigmask=0x0) at ../sysdeps/unix/sysv/linux/ppoll.c:42 #1 0x55991c6a1a3f in qemu_poll_ns (fds=0x55992068fb20, nfds=2, timeout=-1) at ../util/qemu-timer.c:339 #2 0x55991c67ed6c in fdmon_poll_wait (ctx=0x55991f058810, ready_list=0x7ffda8c987b0, timeout=-1) at ../util/fdmon-poll.c:79 #3 0x55991c67e6a8 in aio_poll (ctx=0x55991f058810, blocking=true) at ../util/aio-posix.c:670 #4 0x55991c50a763 in bdrv_graph_wrlock (bs=0x0) at ../block/graph-lock.c:145 Interesting, why in bdrv_close() we pass NULL to bdrv_graph_wrlock.. Shouldn't bdrv_close() be called with bs aio context locked? #5 0x55991c4daf85 in bdrv_close (bs=0x55991fff2f30) at ../block.c:5166 #6 0x55991c4dc050 in bdrv_delete (bs=0x55991fff2f30) at ../block.c:5606 #7 0x55991c4df205 in bdrv_unref (bs=0x55991fff2f30) at ../block.c:7173 #8 0x55991c4fb8ca in bdrv_cbw_drop (bs=0x55991fff2f30) at ../block/copy-before-write.c:566 #9 0x55991c4f9685 in backup_clean (job=0x55992016d0b0) at ../block/backup.c:105 -- Best regards, Vladimir
Re: [RFC] migration/block-dirty-bitmap: make loading bitmap for device with iothread future-proof
On 31.07.23 10:35, Juan Quintela wrote: Fiona Ebner wrote: The bdrv_create_dirty_bitmap() function (which is also called by bdrv_dirty_bitmap_create_successor()) uses bdrv_getlength(bs). This is a wrapper around a coroutine, and when not called in coroutine context would use bdrv_poll_co(). Such a call would trigger an assert() if the correct AioContext hasn't been acquired before, because polling would try to release the AioContext. The ingenous in me thinks: If the problem is that bdrv_poll_co() release an AioContext that it don't have acquired, perhaps we should fix bdrv_poll_co(). Ha!!! $ find . -type f -exec grep --color=auto -nH --null -e bdrv_poll_co \{\} + ./scripts/block-coroutine-wrapper.py\0173: bdrv_poll_co(_state); ./scripts/block-coroutine-wrapper.py\0198: bdrv_poll_co(_state); ./block/block-gen.h\038:static inline void bdrv_poll_co(BdrvPollCo *s) $ /me retreats The function is used in generated code. There are a lot of calls in build/block/block-gen.c if grep after make. The issue does not happen for migration, because the call happens from process_incoming_migration_co(), i.e. in coroutine context. So the bdrv_getlength() wrapper will just call bdrv_co_getlength() directly without polling. The ingenous in me wonders why bdrv_getlength() needs to use coroutines at all, but as I have been burned on the previous paragraph, I learn not to even try. Ok, I never learn, so I do a grep and I see two appearces of bdrv_getlength in include files, but grep only shows uses of the function, not a real definition. the function is generated, and after building, it's definition is in build/block/block-gen.c The information is in comment in include/block/block-common.h. The link, which lead from function declaration to the comment is "co_wrapper", but that's not obvious when just grep the function name. -- Best regards, Vladimir
Re: [PATCH 0/5] RFC: migration: check required entries and sections are loaded
On Tue, Sep 26, 2023 at 07:59:20PM +0400, marcandre.lur...@redhat.com wrote: > Marc-André Lureau (5): > block/fdc: 'phase' is not needed on load > virtio: make endian_needed() work during loading > net/slirp: use different IDs for each instance First 3 patches are bug fixes, am I right? It'll be great if they can be acked (or even picked up earlier?) by subsystem maintainers if so, and copy stable if justified proper. Thanks, -- Peter Xu
Re: [PATCH 4/5] RFC: migration: check required subsections are loaded, once
On Tue, Sep 26, 2023 at 07:59:24PM +0400, marcandre.lur...@redhat.com wrote: > @@ -484,6 +513,13 @@ static int vmstate_subsection_load(QEMUFile *f, const > VMStateDescription *vmsd, > } > } > > +for (i = 0; i < n; i++) { > +if (!visited[i] && vmstate_save_needed(vmsd->subsections[i], > opaque)) { > +trace_vmstate_subsection_load_bad(vmsd->name, > vmsd->subsections[i]->name, "(not visited)"); > +return -ENOENT; > +} > +} One thing that might be tricky to call needed() on loading side is, IMHO the needed() hooks normally was designed to be only called on a complete VM state. IOW, I think it can reference any machine/device state, or whatever variable assuming all of them contain valid data. But the load side may not yet contain everything.. we can guarantee here we loaded the full device state of this one as subsections should be the last to come, and all we've loaded so far. But what if it references something else outside what we've loaded? It looks possible in some special .needed() hook we can return something unexpected. I assume most needed() hooks are fine (and it does look like we can find bugs with this, which means this might be proved useful already at least in some form or another). I just worry on something start to break after we become strict on this. Maybe.. make the check only throw warnings, but not yet fail the migration? > + > trace_vmstate_subsection_load_good(vmsd->name); > return 0; > } > -- > 2.41.0 > -- Peter Xu
Re: [RFC] migration/block-dirty-bitmap: make loading bitmap for device with iothread future-proof
add Kevin, Paolo, Emanuele, pls take a look On 28.07.23 16:39, Fiona Ebner wrote: The bdrv_create_dirty_bitmap() function (which is also called by bdrv_dirty_bitmap_create_successor()) uses bdrv_getlength(bs). This is a wrapper around a coroutine, and when not called in coroutine context would use bdrv_poll_co(). Such a call would trigger an assert() if the correct AioContext hasn't been acquired before, because polling would try to release the AioContext. The issue does not happen for migration, because the call happens from process_incoming_migration_co(), i.e. in coroutine context. So the bdrv_getlength() wrapper will just call bdrv_co_getlength() directly without polling. The issue would happen for snapshots, but won't in practice, because saving a snapshot with a block dirty bitmap is currently not possible. The reason is that dirty_bitmap_save_iterate() returns whether it has completed the bulk phase, which only happens in postcopy, so qemu_savevm_state_iterate() will always return 0, meaning the call to iterate will be repeated over and over again without ever reaching the completion phase. Still, this would make the code more robust for the future. Signed-off-by: Fiona Ebner --- We ran into this issue downstream, because we have a custom snapshot mechanism which does support dirty bitmaps and does not run in coroutine context during load. migration/block-dirty-bitmap.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c index 032fc5f405..e1ae3b7316 100644 --- a/migration/block-dirty-bitmap.c +++ b/migration/block-dirty-bitmap.c @@ -805,8 +805,11 @@ static int dirty_bitmap_load_start(QEMUFile *f, DBMLoadState *s) "destination", bdrv_dirty_bitmap_name(s->bitmap)); return -EINVAL; } else { +AioContext *ctx = bdrv_get_aio_context(s->bs); +aio_context_acquire(ctx); s->bitmap = bdrv_create_dirty_bitmap(s->bs, granularity, s->bitmap_name, _err); +aio_context_release(ctx); if (!s->bitmap) { error_report_err(local_err); return -EINVAL; @@ -833,7 +836,10 @@ static int dirty_bitmap_load_start(QEMUFile *f, DBMLoadState *s) bdrv_disable_dirty_bitmap(s->bitmap); if (flags & DIRTY_BITMAP_MIG_START_FLAG_ENABLED) { +AioContext *ctx = bdrv_get_aio_context(s->bs); +aio_context_acquire(ctx); bdrv_dirty_bitmap_create_successor(s->bitmap, _err); +aio_context_release(ctx); Would not this deadlock in current code? When we have the only one aio context and therefore we are already in it? If as Juan said, we rework incoming migration coroutine to be a separate thread, this patch becomes more correct, I think.. If keep coroutine, I think, we should check are we already in that aio context, and if so we should not acquire it. if (local_err) { error_report_err(local_err); return -EINVAL; -- Best regards, Vladimir
[PATCH] block-jobs: add final flush
From: Vladimir Sementsov-Ogievskiy Actually block job is not completed without the final flush. It's rather unexpected to have broken target when job was successfully completed long ago and now we fail to flush or process just crashed/killed. Mirror job already has mirror_flush() for this. So, it's OK. Add similar things for other jobs: backup, stream, commit. Note, that stream has (documented) different treatment of IGNORE action: it don't retry the operation, continue execution and report error at last. We keep it for final flush too. Signed-off-by: Vladimir Sementsov-Ogievskiy --- Was: [PATCH v4] block-jobs: flush target at the end of .run() But now rewritten. Supersedes: <20230725174008.1147467-1-vsement...@yandex-team.ru> block/backup.c | 2 +- block/block-copy.c | 7 +++ block/commit.c | 16 block/stream.c | 21 + include/block/block-copy.h | 1 + 5 files changed, 38 insertions(+), 9 deletions(-) diff --git a/block/backup.c b/block/backup.c index db3791f4d1..6a1321092a 100644 --- a/block/backup.c +++ b/block/backup.c @@ -156,7 +156,7 @@ static int coroutine_fn backup_loop(BackupBlockJob *job) job->bg_bcs_call = s = block_copy_async(job->bcs, 0, QEMU_ALIGN_UP(job->len, job->cluster_size), job->perf.max_workers, job->perf.max_chunk, -backup_block_copy_callback, job); +true, backup_block_copy_callback, job); while (!block_copy_call_finished(s) && !job_is_cancelled(>common.job)) diff --git a/block/block-copy.c b/block/block-copy.c index 1c60368d72..9b8672d4c8 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -54,6 +54,7 @@ typedef struct BlockCopyCallState { int max_workers; int64_t max_chunk; bool ignore_ratelimit; +bool need_final_flush; BlockCopyAsyncCallbackFunc cb; void *cb_opaque; /* Coroutine where async block-copy is running */ @@ -880,6 +881,10 @@ block_copy_common(BlockCopyCallState *call_state) */ } while (ret > 0 && !qatomic_read(_state->cancelled)); +if (ret == 0 && call_state->need_final_flush) { +ret = bdrv_co_flush(s->target->bs); +} + qatomic_store_release(_state->finished, true); if (call_state->cb) { @@ -935,6 +940,7 @@ int coroutine_fn block_copy(BlockCopyState *s, int64_t start, int64_t bytes, BlockCopyCallState *block_copy_async(BlockCopyState *s, int64_t offset, int64_t bytes, int max_workers, int64_t max_chunk, + bool need_final_flush, BlockCopyAsyncCallbackFunc cb, void *cb_opaque) { @@ -946,6 +952,7 @@ BlockCopyCallState *block_copy_async(BlockCopyState *s, .bytes = bytes, .max_workers = max_workers, .max_chunk = max_chunk, +.need_final_flush = need_final_flush, .cb = cb, .cb_opaque = cb_opaque, diff --git a/block/commit.c b/block/commit.c index aa45beb0f0..5205c77ec9 100644 --- a/block/commit.c +++ b/block/commit.c @@ -120,6 +120,7 @@ static int coroutine_fn commit_run(Job *job, Error **errp) int64_t n = 0; /* bytes */ QEMU_AUTO_VFREE void *buf = NULL; int64_t len, base_len; +BlockErrorAction action; len = blk_co_getlength(s->top); if (len < 0) { @@ -169,9 +170,8 @@ static int coroutine_fn commit_run(Job *job, Error **errp) } } if (ret < 0) { -BlockErrorAction action = -block_job_error_action(>common, s->on_error, - error_in_source, -ret); +action = block_job_error_action(>common, s->on_error, +error_in_source, -ret); if (action == BLOCK_ERROR_ACTION_REPORT) { return ret; } else { @@ -187,7 +187,15 @@ static int coroutine_fn commit_run(Job *job, Error **errp) } } -return 0; +do { +ret = blk_co_flush(s->base); +if (ret < 0) { +action = block_job_error_action(>common, s->on_error, +false, -ret); +} +} while (ret < 0 && action != BLOCK_ERROR_ACTION_REPORT); + +return ret; } static const BlockJobDriver commit_job_driver = { diff --git a/block/stream.c b/block/stream.c index 133cb72fb4..41eb536feb 100644 --- a/block/stream.c +++ b/block/stream.c @@ -143,6 +143,8 @@ static int coroutine_fn stream_run(Job *job, Error **errp) int64_t offset = 0; int error = 0; int64_t n = 0; /* bytes */ +BlockErrorAction action; +int ret; if (unfiltered_bs == s->base_overlay) { /* Nothing to stream */ @@ -159,7 +161,6 @@ static int coroutine_fn stream_run(Job *job, Error **errp) for ( ;
[PULL 1/1] osdep: set _FORTIFY_SOURCE=2 when optimization is enabled
From: Daniel P. Berrangé Currently we set _FORTIFY_SOURCE=2 as a compiler argument when the meson 'optimization' setting is non-zero, the compiler is GCC and the target is Linux. While the default QEMU optimization level is 2, user could override this by setting CFLAGS="-O0" or --extra-cflags="-O0" when running configure and this won't be reflected in the meson 'optimization' setting. As a result we try to enable _FORTIFY_SOURCE=2 and then the user gets compile errors as it only works with optimization. Rather than trying to improve detection in meson, it is simpler to just check the __OPTIMIZE__ define from osdep.h. The comment about being incompatible with clang appears to be outdated, as compilation works fine without excluding clang. In the coroutine code we must set _FORTIFY_SOURCE=0 to stop the logic in osdep.h then enabling it. Signed-off-by: Daniel P. Berrangé Message-id: 20231003091549.223020-1-berra...@redhat.com Signed-off-by: Stefan Hajnoczi --- meson.build | 10 -- include/qemu/osdep.h | 4 util/coroutine-sigaltstack.c | 4 ++-- util/coroutine-ucontext.c| 4 ++-- 4 files changed, 8 insertions(+), 14 deletions(-) diff --git a/meson.build b/meson.build index 21a1bc03f8..20ceeb8158 100644 --- a/meson.build +++ b/meson.build @@ -479,16 +479,6 @@ if 'cpp' in all_languages qemu_cxxflags = ['-D__STDC_LIMIT_MACROS', '-D__STDC_CONSTANT_MACROS', '-D__STDC_FORMAT_MACROS'] + qemu_cflags endif -# clang does not support glibc + FORTIFY_SOURCE (is it still true?) -if get_option('optimization') != '0' and targetos == 'linux' - if cc.get_id() == 'gcc' -qemu_cflags += ['-U_FORTIFY_SOURCE', '-D_FORTIFY_SOURCE=2'] - endif - if 'cpp' in all_languages and cxx.get_id() == 'gcc' -qemu_cxxflags += ['-U_FORTIFY_SOURCE', '-D_FORTIFY_SOURCE=2'] - endif -endif - add_project_arguments(qemu_cflags, native: false, language: 'c') add_project_arguments(cc.get_supported_arguments(warn_flags), native: false, language: 'c') if 'cpp' in all_languages diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index 18b940db75..475a1c62ff 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -27,6 +27,10 @@ #ifndef QEMU_OSDEP_H #define QEMU_OSDEP_H +#if !defined _FORTIFY_SOURCE && defined __OPTIMIZE__ && __OPTIMIZE__ && defined __linux__ +# define _FORTIFY_SOURCE 2 +#endif + #include "config-host.h" #ifdef NEED_CPU_H #include CONFIG_TARGET diff --git a/util/coroutine-sigaltstack.c b/util/coroutine-sigaltstack.c index e2690c5f41..037d6416c4 100644 --- a/util/coroutine-sigaltstack.c +++ b/util/coroutine-sigaltstack.c @@ -22,9 +22,9 @@ */ /* XXX Is there a nicer way to disable glibc's stack check for longjmp? */ -#ifdef _FORTIFY_SOURCE #undef _FORTIFY_SOURCE -#endif +#define _FORTIFY_SOURCE 0 + #include "qemu/osdep.h" #include #include "qemu/coroutine_int.h" diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c index ddc98fb4f8..7b304c79d9 100644 --- a/util/coroutine-ucontext.c +++ b/util/coroutine-ucontext.c @@ -19,9 +19,9 @@ */ /* XXX Is there a nicer way to disable glibc's stack check for longjmp? */ -#ifdef _FORTIFY_SOURCE #undef _FORTIFY_SOURCE -#endif +#define _FORTIFY_SOURCE 0 + #include "qemu/osdep.h" #include #include "qemu/coroutine_int.h" -- 2.41.0
[PULL 0/1] Block patches
The following changes since commit da1034094d375afe9e3d8ec8980550ea0f06f7e0: Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging (2023-10-03 07:43:44 -0400) are available in the Git repository at: https://gitlab.com/stefanha/qemu.git tags/block-pull-request for you to fetch changes up to 9afa888ce0f816d0f2cfc95eebe4f49244c518af: osdep: set _FORTIFY_SOURCE=2 when optimization is enabled (2023-10-04 09:52:06 -0400) Pull request Daniel P. Berrangé (1): osdep: set _FORTIFY_SOURCE=2 when optimization is enabled meson.build | 10 -- include/qemu/osdep.h | 4 util/coroutine-sigaltstack.c | 4 ++-- util/coroutine-ucontext.c| 4 ++-- 4 files changed, 8 insertions(+), 14 deletions(-) -- 2.41.0
Re: [PATCH v3 15/16] sysemu/tpm: Clean up global variable shadowing
On 10/4/23 08:00, Philippe Mathieu-Daudé wrote: Fix: softmmu/tpm.c:178:59: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] int tpm_config_parse(QemuOptsList *opts_list, const char *optarg) ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here extern char *optarg;/* getopt(3) external variables */ ^ Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Stefan Berger --- include/sysemu/tpm.h | 2 +- softmmu/tpm.c| 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h index 66e3b45f30..1ee568b3b6 100644 --- a/include/sysemu/tpm.h +++ b/include/sysemu/tpm.h @@ -17,7 +17,7 @@ #ifdef CONFIG_TPM -int tpm_config_parse(QemuOptsList *opts_list, const char *optarg); +int tpm_config_parse(QemuOptsList *opts_list, const char *optstr); int tpm_init(void); void tpm_cleanup(void); diff --git a/softmmu/tpm.c b/softmmu/tpm.c index 578563f05a..7164ea7ff1 100644 --- a/softmmu/tpm.c +++ b/softmmu/tpm.c @@ -175,15 +175,15 @@ int tpm_init(void) * Parse the TPM configuration options. * To display all available TPM backends the user may use '-tpmdev help' */ -int tpm_config_parse(QemuOptsList *opts_list, const char *optarg) +int tpm_config_parse(QemuOptsList *opts_list, const char *optstr) { QemuOpts *opts; -if (!strcmp(optarg, "help")) { +if (!strcmp(optstr, "help")) { tpm_display_backend_drivers(); return -1; } -opts = qemu_opts_parse_noisily(opts_list, optarg, true); +opts = qemu_opts_parse_noisily(opts_list, optstr, true); if (!opts) { return -1; }
Re: [PATCH v3 09/16] semihosting: Clean up global variable shadowing
Alex Bennée writes: > Philippe Mathieu-Daudé writes: > >> Fix: >> >> semihosting/config.c:134:49: error: declaration shadows a variable in the >> global scope [-Werror,-Wshadow] >> int qemu_semihosting_config_options(const char *optarg) >> ^ >> >> /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: >> note: previous declaration is here >> extern char *optarg;/* getopt(3) external >> variables */ > > I'm going to assume the getopt.h is somehow swept up by osdep.h? It comes from unitstd.h via osdep.h. getopt.h provides getopt_long() & friends. > > Anyway: > > Acked-by: Alex Bennée
Re: [PATCH v3 09/16] semihosting: Clean up global variable shadowing
On 04/10/2023 14.16, Alex Bennée wrote: Philippe Mathieu-Daudé writes: Fix: semihosting/config.c:134:49: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] int qemu_semihosting_config_options(const char *optarg) ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here extern char *optarg;/* getopt(3) external variables */ I'm going to assume the getopt.h is somehow swept up by osdep.h? Anyway: Acked-by: Alex Bennée Could we maybe rather remove getopt.h from osdep.h instead of renaming this everywhere? getopt.h should only be required by some few files, so including this in osdep.h seems exaggerated, IMHO. Thomas
Re: [PATCH v3 05/16] plugins/loader: Clean up global variable shadowing
Philippe Mathieu-Daudé writes: > Fix: > > include/qemu/plugin.h:245:54: error: declaration shadows a variable in the > global scope [-Werror,-Wshadow] > static inline void qemu_plugin_opt_parse(const char *optarg, >^ > > /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: > note: previous declaration is here > extern char *optarg;/* getopt(3) external > variables */ My same raised eyebrows are how exactly getopt.h is getting included but anyway: Acked-by: Alex Bennée -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH v3 13/16] semihosting/arm-compat: Clean up local variable shadowing
Philippe Mathieu-Daudé writes: > Fix: > > semihosting/arm-compat-semi.c: In function ‘do_common_semihosting’: > semihosting/arm-compat-semi.c:379:13: warning: declaration of ‘ret’ shadows > a previous local [-Wshadow=local] > 379 | int ret, err = 0; > | ^~~ > semihosting/arm-compat-semi.c:370:14: note: shadowed declaration is here > 370 | uint32_t ret; > | ^~~ > semihosting/arm-compat-semi.c:682:27: warning: declaration of ‘ret’ > shadows a previous local [-Wshadow=local] > 682 | abi_ulong ret; > | ^~~ > semihosting/arm-compat-semi.c:370:9: note: shadowed declaration is here > 370 | int ret; > | ^~~ > > Signed-off-by: Philippe Mathieu-Daudé > --- > semihosting/arm-compat-semi.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c > index 564fe17f75..0033a1e018 100644 > --- a/semihosting/arm-compat-semi.c > +++ b/semihosting/arm-compat-semi.c > @@ -367,7 +367,6 @@ void do_common_semihosting(CPUState *cs) > target_ulong ul_ret; > char * s; > int nr; > -uint32_t ret; > int64_t elapsed; > > nr = common_semi_arg(cs, 0) & 0xU; > @@ -725,6 +724,9 @@ void do_common_semihosting(CPUState *cs) > > case TARGET_SYS_EXIT: > case TARGET_SYS_EXIT_EXTENDED: > +{ > +uint32_t ret; > + I suspect this could just as well be an int with an explicit cast for ret = arg1 because the consumers are all expecting int anyway. Otherwise: Reviewed-by: Alex Bennée -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH v3 09/16] semihosting: Clean up global variable shadowing
Philippe Mathieu-Daudé writes: > Fix: > > semihosting/config.c:134:49: error: declaration shadows a variable in the > global scope [-Werror,-Wshadow] > int qemu_semihosting_config_options(const char *optarg) > ^ > > /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: > note: previous declaration is here > extern char *optarg;/* getopt(3) external > variables */ I'm going to assume the getopt.h is somehow swept up by osdep.h? Anyway: Acked-by: Alex Bennée -- Alex Bennée Virtualisation Tech Lead @ Linaro
[PATCH v3 11/16] util/cutils: Clean up global variable shadowing in get_relocated_path()
Fix: util/cutils.c:1147:17: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] const char *exec_dir = qemu_get_exec_dir(); ^ util/cutils.c:1035:20: note: previous declaration is here static const char *exec_dir; ^ Signed-off-by: Philippe Mathieu-Daudé --- util/cutils.c | 1 - 1 file changed, 1 deletion(-) diff --git a/util/cutils.c b/util/cutils.c index 25373198ad..b44718a6a2 100644 --- a/util/cutils.c +++ b/util/cutils.c @@ -1144,7 +1144,6 @@ char *get_relocated_path(const char *dir) { size_t prefix_len = strlen(CONFIG_PREFIX); const char *bindir = CONFIG_BINDIR; -const char *exec_dir = qemu_get_exec_dir(); GString *result; int len_dir, len_bindir; -- 2.41.0
[PATCH v3 12/16] util/guest-random: Clean up global variable shadowing
Fix: util/guest-random.c:90:45: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] int qemu_guest_random_seed_main(const char *optarg, Error **errp) ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here extern char *optarg;/* getopt(3) external variables */ ^ Signed-off-by: Philippe Mathieu-Daudé --- include/qemu/guest-random.h | 8 util/guest-random.c | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/include/qemu/guest-random.h b/include/qemu/guest-random.h index 09ff9c2236..5060d49d60 100644 --- a/include/qemu/guest-random.h +++ b/include/qemu/guest-random.h @@ -13,16 +13,16 @@ #define QEMU_GUEST_RANDOM_H /** - * qemu_guest_random_seed_main(const char *optarg, Error **errp) - * @optarg: a non-NULL pointer to a C string + * qemu_guest_random_seed_main(const char *seedstr, Error **errp) + * @seedstr: a non-NULL pointer to a C string * @errp: an error indicator * - * The @optarg value is that which accompanies the -seed argument. + * The @seedstr value is that which accompanies the -seed argument. * This forces qemu_guest_getrandom into deterministic mode. * * Returns 0 on success, < 0 on failure while setting *errp. */ -int qemu_guest_random_seed_main(const char *optarg, Error **errp); +int qemu_guest_random_seed_main(const char *seedstr, Error **errp); /** * qemu_guest_random_seed_thread_part1(void) diff --git a/util/guest-random.c b/util/guest-random.c index 9465dda085..33607d5ff2 100644 --- a/util/guest-random.c +++ b/util/guest-random.c @@ -87,11 +87,11 @@ void qemu_guest_random_seed_thread_part2(uint64_t seed) } } -int qemu_guest_random_seed_main(const char *optarg, Error **errp) +int qemu_guest_random_seed_main(const char *seedstr, Error **errp) { uint64_t seed; -if (parse_uint_full(optarg, 0, )) { -error_setg(errp, "Invalid seed number: %s", optarg); +if (parse_uint_full(seedstr, 0, )) { +error_setg(errp, "Invalid seed number: %s", seedstr); return -1; } else { deterministic = true; -- 2.41.0
[PATCH v3 16/16] trace/control: Clean up global variable shadowing
Fix: trace/control.c:288:34: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] void trace_opt_parse(const char *optarg) ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here extern char *optarg;/* getopt(3) external variables */ ^ Signed-off-by: Philippe Mathieu-Daudé --- trace/control.h | 4 ++-- trace/control.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/trace/control.h b/trace/control.h index dfd209edd8..6754bfe052 100644 --- a/trace/control.h +++ b/trace/control.h @@ -197,11 +197,11 @@ extern QemuOptsList qemu_trace_opts; /** * trace_opt_parse: - * @optarg: A string argument of --trace command line argument + * @optstr: A string argument of --trace command line argument * * Initialize tracing subsystem. */ -void trace_opt_parse(const char *optarg); +void trace_opt_parse(const char *optstr); /** * trace_get_vcpu_event_count: diff --git a/trace/control.c b/trace/control.c index 1a48a7e266..ef107829ac 100644 --- a/trace/control.c +++ b/trace/control.c @@ -285,10 +285,10 @@ bool trace_init_backends(void) return true; } -void trace_opt_parse(const char *optarg) +void trace_opt_parse(const char *optstr) { QemuOpts *opts = qemu_opts_parse_noisily(qemu_find_opts("trace"), - optarg, true); + optstr, true); if (!opts) { exit(1); } -- 2.41.0
[PATCH v3 15/16] sysemu/tpm: Clean up global variable shadowing
Fix: softmmu/tpm.c:178:59: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] int tpm_config_parse(QemuOptsList *opts_list, const char *optarg) ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here extern char *optarg;/* getopt(3) external variables */ ^ Signed-off-by: Philippe Mathieu-Daudé --- include/sysemu/tpm.h | 2 +- softmmu/tpm.c| 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h index 66e3b45f30..1ee568b3b6 100644 --- a/include/sysemu/tpm.h +++ b/include/sysemu/tpm.h @@ -17,7 +17,7 @@ #ifdef CONFIG_TPM -int tpm_config_parse(QemuOptsList *opts_list, const char *optarg); +int tpm_config_parse(QemuOptsList *opts_list, const char *optstr); int tpm_init(void); void tpm_cleanup(void); diff --git a/softmmu/tpm.c b/softmmu/tpm.c index 578563f05a..7164ea7ff1 100644 --- a/softmmu/tpm.c +++ b/softmmu/tpm.c @@ -175,15 +175,15 @@ int tpm_init(void) * Parse the TPM configuration options. * To display all available TPM backends the user may use '-tpmdev help' */ -int tpm_config_parse(QemuOptsList *opts_list, const char *optarg) +int tpm_config_parse(QemuOptsList *opts_list, const char *optstr) { QemuOpts *opts; -if (!strcmp(optarg, "help")) { +if (!strcmp(optstr, "help")) { tpm_display_backend_drivers(); return -1; } -opts = qemu_opts_parse_noisily(opts_list, optarg, true); +opts = qemu_opts_parse_noisily(opts_list, optstr, true); if (!opts) { return -1; } -- 2.41.0
[PATCH v3 07/16] qemu-io: Clean up global variable shadowing
Fix: qemu-io.c:478:36: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] static void add_user_command(char *optarg) ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here extern char *optarg;/* getopt(3) external variables */ ^ Signed-off-by: Philippe Mathieu-Daudé --- qemu-io.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qemu-io.c b/qemu-io.c index 2bd7bfb650..050c70835f 100644 --- a/qemu-io.c +++ b/qemu-io.c @@ -475,10 +475,10 @@ static int command_loop(void) return last_error; } -static void add_user_command(char *optarg) +static void add_user_command(char *user_cmd) { cmdline = g_renew(char *, cmdline, ++ncmdline); -cmdline[ncmdline-1] = optarg; +cmdline[ncmdline - 1] = user_cmd; } static void reenable_tty_echo(void) -- 2.41.0
[PATCH v3 14/16] softmmu/vl: Clean up global variable shadowing
Fix: softmmu/vl.c:1069:44: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] static void parse_display_qapi(const char *optarg) ^ softmmu/vl.c:1224:39: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] static void monitor_parse(const char *optarg, const char *mode, bool pretty) ^ softmmu/vl.c:1634:17: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] const char *optarg = qdict_get_try_str(qdict, "type"); ^ softmmu/vl.c:1784:45: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] static void object_option_parse(const char *optarg) ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here extern char *optarg;/* getopt(3) external variables */ ^ Signed-off-by: Philippe Mathieu-Daudé --- softmmu/vl.c | 26 +- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/softmmu/vl.c b/softmmu/vl.c index 98e071e63b..ae1ff9887d 100644 --- a/softmmu/vl.c +++ b/softmmu/vl.c @@ -1066,12 +1066,12 @@ static void select_vgahw(const MachineClass *machine_class, const char *p) } } -static void parse_display_qapi(const char *optarg) +static void parse_display_qapi(const char *optstr) { DisplayOptions *opts; Visitor *v; -v = qobject_input_visitor_new_str(optarg, "type", _fatal); +v = qobject_input_visitor_new_str(optstr, "type", _fatal); visit_type_DisplayOptions(v, NULL, , _fatal); QAPI_CLONE_MEMBERS(DisplayOptions, , opts); @@ -1221,21 +1221,21 @@ static int mon_init_func(void *opaque, QemuOpts *opts, Error **errp) return monitor_init_opts(opts, errp); } -static void monitor_parse(const char *optarg, const char *mode, bool pretty) +static void monitor_parse(const char *str, const char *mode, bool pretty) { static int monitor_device_index = 0; QemuOpts *opts; const char *p; char label[32]; -if (strstart(optarg, "chardev:", )) { +if (strstart(str, "chardev:", )) { snprintf(label, sizeof(label), "%s", p); } else { snprintf(label, sizeof(label), "compat_monitor%d", monitor_device_index); -opts = qemu_chr_parse_compat(label, optarg, true); +opts = qemu_chr_parse_compat(label, str, true); if (!opts) { -error_report("parse error: %s", optarg); +error_report("parse error: %s", str); exit(1); } } @@ -1631,13 +1631,13 @@ static const QEMUOption *lookup_opt(int argc, char **argv, static MachineClass *select_machine(QDict *qdict, Error **errp) { -const char *optarg = qdict_get_try_str(qdict, "type"); +const char *machine_type = qdict_get_try_str(qdict, "type"); GSList *machines = object_class_get_list(TYPE_MACHINE, false); MachineClass *machine_class; Error *local_err = NULL; -if (optarg) { -machine_class = find_machine(optarg, machines); +if (machine_type) { +machine_class = find_machine(machine_type, machines); qdict_del(qdict, "type"); if (!machine_class) { error_setg(_err, "unsupported machine type"); @@ -1781,20 +1781,20 @@ static void object_option_add_visitor(Visitor *v) QTAILQ_INSERT_TAIL(_opts, opt, next); } -static void object_option_parse(const char *optarg) +static void object_option_parse(const char *optstr) { QemuOpts *opts; const char *type; Visitor *v; -if (optarg[0] == '{') { -QObject *obj = qobject_from_json(optarg, _fatal); +if (optstr[0] == '{') { +QObject *obj = qobject_from_json(optstr, _fatal); v = qobject_input_visitor_new(obj); qobject_unref(obj); } else { opts = qemu_opts_parse_noisily(qemu_find_opts("object"), - optarg, true); + optstr, true); if (!opts) { exit(1); } -- 2.41.0
[PATCH v3 10/16] ui/cocoa: Clean up global variable shadowing
Fix: ui/cocoa.m:346:20: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] QemuCocoaView *cocoaView = userInfo; ^ ui/cocoa.m:342:16: note: previous declaration is here QemuCocoaView *cocoaView; ^ Signed-off-by: Philippe Mathieu-Daudé --- ui/cocoa.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/cocoa.m b/ui/cocoa.m index df6d13be38..15477288fd 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -343,9 +343,9 @@ - (void) raiseAllKeys; static CGEventRef handleTapEvent(CGEventTapProxy proxy, CGEventType type, CGEventRef cgEvent, void *userInfo) { -QemuCocoaView *cocoaView = userInfo; +QemuCocoaView *view = userInfo; NSEvent *event = [NSEvent eventWithCGEvent:cgEvent]; -if ([cocoaView isMouseGrabbed] && [cocoaView handleEvent:event]) { +if ([view isMouseGrabbed] && [view handleEvent:event]) { COCOA_DEBUG("Global events tap: qemu handled the event, capturing!\n"); return NULL; } -- 2.41.0
[PATCH v3 13/16] semihosting/arm-compat: Clean up local variable shadowing
Fix: semihosting/arm-compat-semi.c: In function ‘do_common_semihosting’: semihosting/arm-compat-semi.c:379:13: warning: declaration of ‘ret’ shadows a previous local [-Wshadow=local] 379 | int ret, err = 0; | ^~~ semihosting/arm-compat-semi.c:370:14: note: shadowed declaration is here 370 | uint32_t ret; | ^~~ semihosting/arm-compat-semi.c:682:27: warning: declaration of ‘ret’ shadows a previous local [-Wshadow=local] 682 | abi_ulong ret; | ^~~ semihosting/arm-compat-semi.c:370:9: note: shadowed declaration is here 370 | int ret; | ^~~ Signed-off-by: Philippe Mathieu-Daudé --- semihosting/arm-compat-semi.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c index 564fe17f75..0033a1e018 100644 --- a/semihosting/arm-compat-semi.c +++ b/semihosting/arm-compat-semi.c @@ -367,7 +367,6 @@ void do_common_semihosting(CPUState *cs) target_ulong ul_ret; char * s; int nr; -uint32_t ret; int64_t elapsed; nr = common_semi_arg(cs, 0) & 0xU; @@ -725,6 +724,9 @@ void do_common_semihosting(CPUState *cs) case TARGET_SYS_EXIT: case TARGET_SYS_EXIT_EXTENDED: +{ +uint32_t ret; + if (common_semi_sys_exit_extended(cs, nr)) { /* * The A64 version of SYS_EXIT takes a parameter block, @@ -752,6 +754,7 @@ void do_common_semihosting(CPUState *cs) } gdb_exit(ret); exit(ret); +} case TARGET_SYS_ELAPSED: elapsed = get_clock() - clock_start; -- 2.41.0
[PATCH v3 09/16] semihosting: Clean up global variable shadowing
Fix: semihosting/config.c:134:49: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] int qemu_semihosting_config_options(const char *optarg) ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here extern char *optarg;/* getopt(3) external variables */ ^ Signed-off-by: Philippe Mathieu-Daudé --- include/semihosting/semihost.h | 2 +- semihosting/config.c | 8 stubs/semihost.c | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/include/semihosting/semihost.h b/include/semihosting/semihost.h index efd2efa25a..97d2a2ba99 100644 --- a/include/semihosting/semihost.h +++ b/include/semihosting/semihost.h @@ -66,7 +66,7 @@ const char *semihosting_get_cmdline(void); void semihosting_arg_fallback(const char *file, const char *cmd); /* for vl.c hooks */ void qemu_semihosting_enable(void); -int qemu_semihosting_config_options(const char *opt); +int qemu_semihosting_config_options(const char *optstr); void qemu_semihosting_chardev_init(void); void qemu_semihosting_console_init(Chardev *); #endif /* CONFIG_USER_ONLY */ diff --git a/semihosting/config.c b/semihosting/config.c index 8ca569735d..e826457733 100644 --- a/semihosting/config.c +++ b/semihosting/config.c @@ -131,10 +131,10 @@ void qemu_semihosting_enable(void) semihosting.target = SEMIHOSTING_TARGET_AUTO; } -int qemu_semihosting_config_options(const char *optarg) +int qemu_semihosting_config_options(const char *optstr) { QemuOptsList *opt_list = qemu_find_opts("semihosting-config"); -QemuOpts *opts = qemu_opts_parse_noisily(opt_list, optarg, false); +QemuOpts *opts = qemu_opts_parse_noisily(opt_list, optstr, false); semihosting.enabled = true; @@ -155,7 +155,7 @@ int qemu_semihosting_config_options(const char *optarg) semihosting.target = SEMIHOSTING_TARGET_AUTO; } else { error_report("unsupported semihosting-config %s", - optarg); + optstr); return 1; } } else { @@ -165,7 +165,7 @@ int qemu_semihosting_config_options(const char *optarg) qemu_opt_foreach(opts, add_semihosting_arg, , NULL); } else { -error_report("unsupported semihosting-config %s", optarg); +error_report("unsupported semihosting-config %s", optstr); return 1; } diff --git a/stubs/semihost.c b/stubs/semihost.c index aad7a70353..b3c61935b3 100644 --- a/stubs/semihost.c +++ b/stubs/semihost.c @@ -36,7 +36,7 @@ void qemu_semihosting_enable(void) { } -int qemu_semihosting_config_options(const char *optarg) +int qemu_semihosting_config_options(const char *optstr) { return 1; } -- 2.41.0
[PATCH v3 08/16] qom/object_interfaces: Clean up global variable shadowing
Fix: qom/object_interfaces.c:262:53: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] ObjectOptions *user_creatable_parse_str(const char *optarg, Error **errp) ^ qom/object_interfaces.c:298:46: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] bool user_creatable_add_from_str(const char *optarg, Error **errp) ^ qom/object_interfaces.c:313:49: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] void user_creatable_process_cmdline(const char *optarg) ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here extern char *optarg;/* getopt(3) external variables */ ^ Signed-off-by: Philippe Mathieu-Daudé --- include/qom/object_interfaces.h | 16 qom/object_interfaces.c | 16 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h index 81541e2080..02b11a7ef0 100644 --- a/include/qom/object_interfaces.h +++ b/include/qom/object_interfaces.h @@ -99,7 +99,7 @@ void user_creatable_add_qapi(ObjectOptions *options, Error **errp); /** * user_creatable_parse_str: - * @optarg: the object definition string as passed on the command line + * @str: the object definition string as passed on the command line * @errp: if an error occurs, a pointer to an area to store the error * * Parses the option for the user creatable object with a keyval parser and @@ -110,14 +110,14 @@ void user_creatable_add_qapi(ObjectOptions *options, Error **errp); * Returns: ObjectOptions on success, NULL when an error occurred (*errp is set * then) or help was printed (*errp is not set). */ -ObjectOptions *user_creatable_parse_str(const char *optarg, Error **errp); +ObjectOptions *user_creatable_parse_str(const char *str, Error **errp); /** * user_creatable_add_from_str: - * @optarg: the object definition string as passed on the command line + * @str: the object definition string as passed on the command line * @errp: if an error occurs, a pointer to an area to store the error * - * Create an instance of the user creatable object by parsing optarg + * Create an instance of the user creatable object by parsing @str * with a keyval parser and implicit key 'qom-type', converting the * result to ObjectOptions and calling into qmp_object_add(). * @@ -126,13 +126,13 @@ ObjectOptions *user_creatable_parse_str(const char *optarg, Error **errp); * Returns: true when an object was successfully created, false when an error * occurred (*errp is set then) or help was printed (*errp is not set). */ -bool user_creatable_add_from_str(const char *optarg, Error **errp); +bool user_creatable_add_from_str(const char *str, Error **errp); /** * user_creatable_process_cmdline: - * @optarg: the object definition string as passed on the command line + * @cmdline: the object definition string as passed on the command line * - * Create an instance of the user creatable object by parsing optarg + * Create an instance of the user creatable object by parsing @cmdline * with a keyval parser and implicit key 'qom-type', converting the * result to ObjectOptions and calling into qmp_object_add(). * @@ -141,7 +141,7 @@ bool user_creatable_add_from_str(const char *optarg, Error **errp); * This function is only meant to be called during command line parsing. * It exits the process on failure or after printing help. */ -void user_creatable_process_cmdline(const char *optarg); +void user_creatable_process_cmdline(const char *cmdline); /** * user_creatable_print_help: diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c index 7d31589b04..e0833c8bfe 100644 --- a/qom/object_interfaces.c +++ b/qom/object_interfaces.c @@ -259,7 +259,7 @@ static void user_creatable_print_help_from_qdict(QDict *args) } } -ObjectOptions *user_creatable_parse_str(const char *optarg, Error **errp) +ObjectOptions *user_creatable_parse_str(const char *str, Error **errp) { ERRP_GUARD(); QObject *obj; @@ -267,14 +267,14 @@ ObjectOptions *user_creatable_parse_str(const char *optarg, Error **errp) Visitor *v; ObjectOptions *options; -if (optarg[0] == '{') { -obj = qobject_from_json(optarg, errp); +if (str[0] == '{') { +obj = qobject_from_json(str, errp); if (!obj) { return NULL; } v = qobject_input_visitor_new(obj); } else { -QDict *args = keyval_parse(optarg, "qom-type", , errp); +QDict *args = keyval_parse(str, "qom-type", , errp); if (*errp) { return NULL; } @@ -295,12 +295,12 @@ ObjectOptions *user_creatable_parse_str(const char
[PATCH v3 03/16] net/net: Clean up global variable shadowing
Fix: net/net.c:1680:35: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] bool netdev_is_modern(const char *optarg) ^ net/net.c:1714:38: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] void netdev_parse_modern(const char *optarg) ^ net/net.c:1728:60: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] void net_client_parse(QemuOptsList *opts_list, const char *optarg) ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here extern char *optarg;/* getopt(3) external variables */ ^ Signed-off-by: Philippe Mathieu-Daudé --- include/net/net.h | 6 +++--- net/net.c | 14 +++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/include/net/net.h b/include/net/net.h index 330d285930..2fb1c9181c 100644 --- a/include/net/net.h +++ b/include/net/net.h @@ -247,9 +247,9 @@ extern const char *host_net_devices[]; /* from net.c */ extern NetClientStateList net_clients; -bool netdev_is_modern(const char *optarg); -void netdev_parse_modern(const char *optarg); -void net_client_parse(QemuOptsList *opts_list, const char *str); +bool netdev_is_modern(const char *optstr); +void netdev_parse_modern(const char *optstr); +void net_client_parse(QemuOptsList *opts_list, const char *optstr); void show_netdevs(void); void net_init_clients(void); void net_check_clients(void); diff --git a/net/net.c b/net/net.c index 1c0bfdaa6c..c0c0cbe99e 100644 --- a/net/net.c +++ b/net/net.c @@ -1677,7 +1677,7 @@ void net_init_clients(void) * Modern syntax is to be parsed with netdev_parse_modern(). * Traditional syntax is to be parsed with net_client_parse(). */ -bool netdev_is_modern(const char *optarg) +bool netdev_is_modern(const char *optstr) { QemuOpts *opts; bool is_modern; @@ -1689,13 +1689,13 @@ bool netdev_is_modern(const char *optarg) .desc = { { } }, }; -if (optarg[0] == '{') { +if (optstr[0] == '{') { /* This is JSON, which means it's modern syntax */ return true; } opts = qemu_opts_create(_opts, NULL, false, _abort); -qemu_opts_do_parse(opts, optarg, dummy_opts.implied_opt_name, +qemu_opts_do_parse(opts, optstr, dummy_opts.implied_opt_name, _abort); type = qemu_opt_get(opts, "type"); is_modern = !g_strcmp0(type, "stream") || !g_strcmp0(type, "dgram"); @@ -1711,12 +1711,12 @@ bool netdev_is_modern(const char *optarg) * netdev_parse_modern() appends to @nd_queue, whereas net_client_parse() * appends to @qemu_netdev_opts. */ -void netdev_parse_modern(const char *optarg) +void netdev_parse_modern(const char *optstr) { Visitor *v; NetdevQueueEntry *nd; -v = qobject_input_visitor_new_str(optarg, "type", _fatal); +v = qobject_input_visitor_new_str(optstr, "type", _fatal); nd = g_new(NetdevQueueEntry, 1); visit_type_Netdev(v, NULL, >nd, _fatal); visit_free(v); @@ -1725,9 +1725,9 @@ void netdev_parse_modern(const char *optarg) QSIMPLEQ_INSERT_TAIL(_queue, nd, entry); } -void net_client_parse(QemuOptsList *opts_list, const char *optarg) +void net_client_parse(QemuOptsList *opts_list, const char *optstr) { -if (!qemu_opts_parse_noisily(opts_list, optarg, true)) { +if (!qemu_opts_parse_noisily(opts_list, optstr, true)) { exit(1); } } -- 2.41.0
[PATCH v3 04/16] os-posix: Clean up global variable shadowing
Fix: os-posix.c:103:31: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] bool os_set_runas(const char *optarg) ^ os-posix.c:176:32: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] void os_set_chroot(const char *optarg) ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here extern char *optarg;/* getopt(3) external variables */ ^ Signed-off-by: Philippe Mathieu-Daudé --- include/sysemu/os-posix.h | 4 ++-- os-posix.c| 12 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h index 6dfdcbb086..dff32ae185 100644 --- a/include/sysemu/os-posix.h +++ b/include/sysemu/os-posix.h @@ -49,8 +49,8 @@ void os_setup_signal_handling(void); int os_set_daemonize(bool d); bool is_daemonized(void); void os_daemonize(void); -bool os_set_runas(const char *optarg); -void os_set_chroot(const char *optarg); +bool os_set_runas(const char *user_id); +void os_set_chroot(const char *path); void os_setup_post(void); int os_mlock(void); diff --git a/os-posix.c b/os-posix.c index f90dfda9b0..52ef6990ff 100644 --- a/os-posix.c +++ b/os-posix.c @@ -94,13 +94,13 @@ static uid_t user_uid = (uid_t)-1; /* -1 -1 >=0*/ static gid_t user_gid = (gid_t)-1; /* -1 -1>=0*/ /* - * Prepare to change user ID. optarg can be one of 3 forms: + * Prepare to change user ID. user_id can be one of 3 forms: * - a username, in which case user ID will be changed to its uid, * with primary and supplementary groups set up too; * - a numeric uid, in which case only the uid will be set; * - a pair of numeric uid:gid. */ -bool os_set_runas(const char *optarg) +bool os_set_runas(const char *user_id) { unsigned long lv; const char *ep; @@ -108,14 +108,14 @@ bool os_set_runas(const char *optarg) gid_t got_gid; int rc; -user_pwd = getpwnam(optarg); +user_pwd = getpwnam(user_id); if (user_pwd) { user_uid = -1; user_gid = -1; return true; } -rc = qemu_strtoul(optarg, , 0, ); +rc = qemu_strtoul(user_id, , 0, ); got_uid = lv; /* overflow here is ID in C99 */ if (rc || *ep != ':' || got_uid != lv || got_uid == (uid_t)-1) { return false; @@ -173,9 +173,9 @@ static void change_process_uid(void) static const char *chroot_dir; -void os_set_chroot(const char *optarg) +void os_set_chroot(const char *path) { -chroot_dir = optarg; +chroot_dir = path; } static void change_root(void) -- 2.41.0
[PATCH v3 06/16] qemu-img: Clean up global variable shadowing
Fix: qemu-img.c:247:46: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] static bool is_valid_option_list(const char *optarg) ^ qemu-img.c:265:53: error: declaration shadows a variable in the global scope [-Werror,-Wshadow] static int accumulate_options(char **options, char *optarg) ^ /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: note: previous declaration is here extern char *optarg;/* getopt(3) external variables */ ^ Signed-off-by: Philippe Mathieu-Daudé --- qemu-img.c | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index a48edb7101..6068ab0d27 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -235,25 +235,25 @@ void help(void) } /* - * Is @optarg safe for accumulate_options()? + * Is @list safe for accumulate_options()? * It is when multiple of them can be joined together separated by ','. - * To make that work, @optarg must not start with ',' (or else a + * To make that work, @list must not start with ',' (or else a * separating ',' preceding it gets escaped), and it must not end with * an odd number of ',' (or else a separating ',' following it gets * escaped), or be empty (or else a separating ',' preceding it can * escape a separating ',' following it). * */ -static bool is_valid_option_list(const char *optarg) +static bool is_valid_option_list(const char *list) { -size_t len = strlen(optarg); +size_t len = strlen(list); size_t i; -if (!optarg[0] || optarg[0] == ',') { +if (!list[0] || list[0] == ',') { return false; } -for (i = len; i > 0 && optarg[i - 1] == ','; i--) { +for (i = len; i > 0 && list[i - 1] == ','; i--) { } if ((len - i) % 2) { return false; @@ -262,19 +262,19 @@ static bool is_valid_option_list(const char *optarg) return true; } -static int accumulate_options(char **options, char *optarg) +static int accumulate_options(char **options, char *list) { char *new_options; -if (!is_valid_option_list(optarg)) { -error_report("Invalid option list: %s", optarg); +if (!is_valid_option_list(list)) { +error_report("Invalid option list: %s", list); return -1; } if (!*options) { -*options = g_strdup(optarg); +*options = g_strdup(list); } else { -new_options = g_strdup_printf("%s,%s", *options, optarg); +new_options = g_strdup_printf("%s,%s", *options, list); g_free(*options); *options = new_options; } -- 2.41.0