Philippe Mathieu-Daudé <phi...@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 <arm...@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
