replay_add_blocker() takes an Error *. All callers pass one created
like this:
error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "some feature");
Folding this into replay_add_blocker() simplifies the callers, losing
a bit of generality we haven't needed in more than six years.
Since there are no other uses of macro QERR_REPLAY_NOT_SUPPORTED,
replace the remaining one by its expansion, and drop the macro.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
include/qapi/qmp/qerror.h | 3 ---
include/sysemu/replay.h | 2 +-
replay/replay.c | 6 +++++-
replay/stubs-system.c | 2 +-
softmmu/rtc.c | 5 +----
softmmu/vl.c | 13 +++----------
6 files changed, 11 insertions(+), 20 deletions(-)
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 87ca83b155..09006e69f7 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -59,9 +59,6 @@
#define QERR_QGA_COMMAND_FAILED \
"Guest agent command failed, error was '%s'"
-#define QERR_REPLAY_NOT_SUPPORTED \
- "Record/replay feature is not supported for '%s'"
-
#define QERR_UNSUPPORTED \
"this feature or command is not currently supported"
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 7ec0882b50..6e5ab09f71 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -72,7 +72,7 @@ void replay_start(void);
/*! Closes replay log file and frees other resources. */
void replay_finish(void);
/*! Adds replay blocker with the specified error description */
-void replay_add_blocker(Error *reason);
+void replay_add_blocker(const char *feature);
/* Returns name of the replay log file */
const char *replay_get_filename(void);
/*
diff --git a/replay/replay.c b/replay/replay.c
index 9a0dc1cf44..c39156c522 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -376,8 +376,12 @@ void replay_finish(void)
replay_mode = REPLAY_MODE_NONE;
}
-void replay_add_blocker(Error *reason)
+void replay_add_blocker(const char *feature)
{
+ Error *reason = NULL;
+
+ error_setg(&reason, "Record/replay feature is not supported for '%s'",
+ feature);
replay_blockers = g_slist_prepend(replay_blockers, reason);
}
diff --git a/replay/stubs-system.c b/replay/stubs-system.c
index 5c262b08f1..50cefdb2d6 100644
--- a/replay/stubs-system.c
+++ b/replay/stubs-system.c
@@ -12,7 +12,7 @@ void replay_input_sync_event(void)
qemu_input_event_sync_impl();
}
-void replay_add_blocker(Error *reason)
+void replay_add_blocker(const char *feature)
{
}
void replay_audio_in(size_t *recorded, void *samples, size_t *wpos, size_t size)
diff --git a/softmmu/rtc.c b/softmmu/rtc.c
index f7114bed7d..4b2bf75dd6 100644
--- a/softmmu/rtc.c
+++ b/softmmu/rtc.c
@@ -152,11 +152,8 @@ void configure_rtc(QemuOpts *opts)
if (!strcmp(value, "utc")) {
rtc_base_type = RTC_BASE_UTC;
} else if (!strcmp(value, "localtime")) {
- Error *blocker = NULL;
rtc_base_type = RTC_BASE_LOCALTIME;
- error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED,
- "-rtc base=localtime");
- replay_add_blocker(blocker);
+ replay_add_blocker("-rtc base=localtime");
} else {
rtc_base_type = RTC_BASE_DATETIME;
configure_rtc_base_datetime(value);
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 9177d95d4e..9d324fc6cd 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1855,9 +1855,7 @@ static void qemu_apply_machine_options(QDict *qdict)
}
if (current_machine->smp.cpus > 1) {
- Error *blocker = NULL;
- error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
- replay_add_blocker(blocker);
+ replay_add_blocker("smp");
}
}
@@ -2770,13 +2768,8 @@ void qemu_init(int argc, char **argv)
drive_add(IF_PFLASH, -1, optarg, PFLASH_OPTS);
break;
case QEMU_OPTION_snapshot:
- {
- Error *blocker = NULL;
- snapshot = 1;
- error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED,
- "-snapshot");
- replay_add_blocker(blocker);
- }
+ snapshot = 1;
+ replay_add_blocker("-snapshot");
break;
case QEMU_OPTION_numa:
opts = qemu_opts_parse_noisily(qemu_find_opts("numa"),
--
2.39.0On 7/2/23 08:51, Markus Armbruster wrote:
> replay_add_blocker() takes an Error *. All callers pass one created
> like this:
>
> error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "some feature");
>
> Folding this into replay_add_blocker() simplifies the callers, losing
> a bit of generality we haven't needed in more than six years.
>
> Since there are no other uses of macro QERR_REPLAY_NOT_SUPPORTED,
> replace the remaining one by its expansion, and drop the macro.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> include/qapi/qmp/qerror.h | 3 ---
> include/sysemu/replay.h | 2 +-
> replay/replay.c | 6 +++++-
> replay/stubs-system.c | 2 +-
> softmmu/rtc.c | 5 +----
> softmmu/vl.c | 13 +++----------
> 6 files changed, 11 insertions(+), 20 deletions(-)
> diff --git a/replay/replay.c b/replay/replay.c
> index 9a0dc1cf44..c39156c522 100644
> --- a/replay/replay.c
> +++ b/replay/replay.c
> @@ -376,8 +376,12 @@ void replay_finish(void)
> replay_mode = REPLAY_MODE_NONE;
> }
>
> -void replay_add_blocker(Error *reason)
> +void replay_add_blocker(const char *feature)
> {
> + Error *reason = NULL;
> +
> + error_setg(&reason, "Record/replay feature is not supported for '%s'",
> + feature);
Either name 'feature' as 'cli_option' and use '-%s' in format,
> replay_blockers = g_slist_prepend(replay_blockers, reason);
> }
>
> diff --git a/replay/stubs-system.c b/replay/stubs-system.c
> index 5c262b08f1..50cefdb2d6 100644
> --- a/replay/stubs-system.c
> +++ b/replay/stubs-system.c
> @@ -12,7 +12,7 @@ void replay_input_sync_event(void)
> qemu_input_event_sync_impl();
> }
>
> -void replay_add_blocker(Error *reason)
> +void replay_add_blocker(const char *feature)
> {
> }
> void replay_audio_in(size_t *recorded, void *samples, size_t *wpos, size_t size)
> diff --git a/softmmu/rtc.c b/softmmu/rtc.c
> index f7114bed7d..4b2bf75dd6 100644
> --- a/softmmu/rtc.c
> +++ b/softmmu/rtc.c
> @@ -152,11 +152,8 @@ void configure_rtc(QemuOpts *opts)
> if (!strcmp(value, "utc")) {
> rtc_base_type = RTC_BASE_UTC;
> } else if (!strcmp(value, "localtime")) {
> - Error *blocker = NULL;
> rtc_base_type = RTC_BASE_LOCALTIME;
> - error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED,
> - "-rtc base=localtime");
> - replay_add_blocker(blocker);
> + replay_add_blocker("-rtc base=localtime");
> } else {
> rtc_base_type = RTC_BASE_DATETIME;
> configure_rtc_base_datetime(value);
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 9177d95d4e..9d324fc6cd 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -1855,9 +1855,7 @@ static void qemu_apply_machine_options(QDict *qdict)
> }
>
> if (current_machine->smp.cpus > 1) {
> - Error *blocker = NULL;
> - error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
> - replay_add_blocker(blocker);
> + replay_add_blocker("smp");
... or use "-smp" here (yes, pre-existing).
> }
> }
Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> On 7/2/23 08:51, Markus Armbruster wrote:
>> replay_add_blocker() takes an Error *. All callers pass one created
>> like this:
>>
>> error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "some feature");
>>
>> Folding this into replay_add_blocker() simplifies the callers, losing
>> a bit of generality we haven't needed in more than six years.
>>
>> Since there are no other uses of macro QERR_REPLAY_NOT_SUPPORTED,
>> replace the remaining one by its expansion, and drop the macro.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> include/qapi/qmp/qerror.h | 3 ---
>> include/sysemu/replay.h | 2 +-
>> replay/replay.c | 6 +++++-
>> replay/stubs-system.c | 2 +-
>> softmmu/rtc.c | 5 +----
>> softmmu/vl.c | 13 +++----------
>> 6 files changed, 11 insertions(+), 20 deletions(-)
>
>
>> diff --git a/replay/replay.c b/replay/replay.c
>> index 9a0dc1cf44..c39156c522 100644
>> --- a/replay/replay.c
>> +++ b/replay/replay.c
>> @@ -376,8 +376,12 @@ void replay_finish(void)
>> replay_mode = REPLAY_MODE_NONE;
>> }
>>
>> -void replay_add_blocker(Error *reason)
>> +void replay_add_blocker(const char *feature)
>> {
>> + Error *reason = NULL;
>> +
>> + error_setg(&reason, "Record/replay feature is not supported for '%s'",
>> + feature);
>
> Either name 'feature' as 'cli_option' and use '-%s' in format,
>
>> replay_blockers = g_slist_prepend(replay_blockers, reason);
>> }
>>
>> diff --git a/replay/stubs-system.c b/replay/stubs-system.c
>> index 5c262b08f1..50cefdb2d6 100644
>> --- a/replay/stubs-system.c
>> +++ b/replay/stubs-system.c
>> @@ -12,7 +12,7 @@ void replay_input_sync_event(void)
>> qemu_input_event_sync_impl();
>> }
>>
>> -void replay_add_blocker(Error *reason)
>> +void replay_add_blocker(const char *feature)
>> {
>> }
>> void replay_audio_in(size_t *recorded, void *samples, size_t *wpos, size_t size)
>> diff --git a/softmmu/rtc.c b/softmmu/rtc.c
>> index f7114bed7d..4b2bf75dd6 100644
>> --- a/softmmu/rtc.c
>> +++ b/softmmu/rtc.c
>> @@ -152,11 +152,8 @@ void configure_rtc(QemuOpts *opts)
>> if (!strcmp(value, "utc")) {
>> rtc_base_type = RTC_BASE_UTC;
>> } else if (!strcmp(value, "localtime")) {
>> - Error *blocker = NULL;
>> rtc_base_type = RTC_BASE_LOCALTIME;
>> - error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED,
>> - "-rtc base=localtime");
>> - replay_add_blocker(blocker);
>> + replay_add_blocker("-rtc base=localtime");
>> } else {
>> rtc_base_type = RTC_BASE_DATETIME;
>> configure_rtc_base_datetime(value);
>> diff --git a/softmmu/vl.c b/softmmu/vl.c
>> index 9177d95d4e..9d324fc6cd 100644
>> --- a/softmmu/vl.c
>> +++ b/softmmu/vl.c
>> @@ -1855,9 +1855,7 @@ static void qemu_apply_machine_options(QDict *qdict)
>> }
>>
>> if (current_machine->smp.cpus > 1) {
>> - Error *blocker = NULL;
>> - error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
>> - replay_add_blocker(blocker);
>> + replay_add_blocker("smp");
>
> ... or use "-smp" here (yes, pre-existing).
>
>> }
>> }
This patch doesn't change error messages. If we want to improve some,
we should do so in a separate patch.
Let's review the error messages that pass through replay_add_blocker().
0. General format
qemu-system-x86_64: Record/replay: Record/replay feature is not supported for '%s'
Pretty bad. Better:
qemu-system-x86_64: record/replay is not supported %s
Still neglects to identify the erroneous bit of configuration like we
do elsewhere, e.g.
$ qemu-system-x86_64 -device e100
qemu-system-x86_64: -device e100: 'e100' is not a valid device model name
Let's not worry about that now.
1. SMP
$ qemu-system-x86_64 -icount shift=auto,rr=record,rrfile=/dev/null -smp 2
qemu-system-x86_64: Record/replay: Record/replay feature is not supported for 'smp'
First attempt at improvement:
qemu-system-x86_64: record/replay is not supported with -smp
But that's a lie, it works just fine with -smp 1. So:
qemu-system-x86_64: record/replay is not supported with multiple CPUs
2. RTC
$ qemu-system-x86_64 -icount shift=auto,rr=record,rrfile=/dev/null -rtc base=localtime
qemu-system-x86_64: Record/replay: Record/replay feature is not supported for '-rtc base=localtime'
Obvious improvement:
qemu-system-x86_64: record/replay is not supported with -rtc base=localtime
Fine, except for the part where we assume where the configuration
comes from. Watch this:
$ cat rtc.cfg
[rtc]
base = "localtime"
$ qemu-system-x86_64 -icount shift=auto,rr=record,rrfile=/dev/null -readconfig rtc.cfg
qemu-system-x86_64: Record/replay: Record/replay feature is not supported for '-rtc base=localtime'
To make sense of this, user needs to make the connection from "-rtc
base=localtime" to what he actually wrote in the configuration file.
Let's not worry about that now, either.
3. Snapshot
$ qemu-system-x86_64 -icount shift=auto,rr=record,rrfile=/dev/null -snapshot
qemu-system-x86_64: Record/replay: Record/replay feature is not supported for '-snapshot'
Obvious improvement:
qemu-system-x86_64: record/replay is not supported with -snapshot
© 2016 - 2026 Red Hat, Inc.