Re: [PATCH 09/12] replay: Simplify setting replay blockers

[Philippe Mathieu-Daudé]

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).

>       }
>   }