On Fri, Apr 26, 2024 at 11:20:37AM -0300, Fabiano Rosas wrote:
> Add the direct-io migration parameter that tells the migration code to
> use O_DIRECT when opening the migration stream file whenever possible.
>
> This is currently only used with the mapped-ram migration that has a
> clear window guaranteed to perform aligned writes.
>
> Acked-by: Markus Armbruster <arm...@redhat.com>
> Signed-off-by: Fabiano Rosas <faro...@suse.de>
> ---
> include/qemu/osdep.h | 2 ++
> migration/migration-hmp-cmds.c | 11 +++++++++++
> migration/options.c | 30 ++++++++++++++++++++++++++++++
> migration/options.h | 1 +
> qapi/migration.json | 18 +++++++++++++++---
> util/osdep.c | 9 +++++++++
> 6 files changed, 68 insertions(+), 3 deletions(-)
>
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index c7053cdc2b..645c14a65d 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -612,6 +612,8 @@ int qemu_lock_fd_test(int fd, int64_t start, int64_t len,
> bool exclusive);
> bool qemu_has_ofd_lock(void);
> #endif
>
> +bool qemu_has_direct_io(void);
> +
> #if defined(__HAIKU__) && defined(__i386__)
> #define FMT_pid "%ld"
> #elif defined(WIN64)
> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> index 7e96ae6ffd..8496a2b34e 100644
> --- a/migration/migration-hmp-cmds.c
> +++ b/migration/migration-hmp-cmds.c
> @@ -397,6 +397,13 @@ void hmp_info_migrate_parameters(Monitor *mon, const
> QDict *qdict)
> monitor_printf(mon, "%s: %s\n",
> MigrationParameter_str(MIGRATION_PARAMETER_MODE),
> qapi_enum_lookup(&MigMode_lookup, params->mode));
> +
> + if (params->has_direct_io) {
> + monitor_printf(mon, "%s: %s\n",
> + MigrationParameter_str(
> + MIGRATION_PARAMETER_DIRECT_IO),
> + params->direct_io ? "on" : "off");
> + }
> }
>
> qapi_free_MigrationParameters(params);
> @@ -690,6 +697,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict
> *qdict)
> p->has_mode = true;
> visit_type_MigMode(v, param, &p->mode, &err);
> break;
> + case MIGRATION_PARAMETER_DIRECT_IO:
> + p->has_direct_io = true;
> + visit_type_bool(v, param, &p->direct_io, &err);
> + break;
> default:
> assert(0);
> }
> diff --git a/migration/options.c b/migration/options.c
> index 239f5ecfb4..ae464aa4f2 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -826,6 +826,22 @@ int migrate_decompress_threads(void)
> return s->parameters.decompress_threads;
> }
>
> +bool migrate_direct_io(void)
> +{
> + MigrationState *s = migrate_get_current();
> +
> + /* For now O_DIRECT is only supported with mapped-ram */
> + if (!s->capabilities[MIGRATION_CAPABILITY_MAPPED_RAM]) {
> + return false;
> + }
> +
> + if (s->parameters.has_direct_io) {
> + return s->parameters.direct_io;
> + }
> +
> + return false;
> +}
> +
> uint64_t migrate_downtime_limit(void)
> {
> MigrationState *s = migrate_get_current();
> @@ -1061,6 +1077,11 @@ MigrationParameters
> *qmp_query_migrate_parameters(Error **errp)
> params->has_zero_page_detection = true;
> params->zero_page_detection = s->parameters.zero_page_detection;
>
> + if (s->parameters.has_direct_io) {
> + params->has_direct_io = true;
> + params->direct_io = s->parameters.direct_io;
> + }
> +
> return params;
> }
>
> @@ -1097,6 +1118,7 @@ void migrate_params_init(MigrationParameters *params)
> params->has_vcpu_dirty_limit = true;
> params->has_mode = true;
> params->has_zero_page_detection = true;
> + params->has_direct_io = qemu_has_direct_io();
> }
>
> /*
> @@ -1416,6 +1438,10 @@ static void
> migrate_params_test_apply(MigrateSetParameters *params,
> if (params->has_zero_page_detection) {
> dest->zero_page_detection = params->zero_page_detection;
> }
> +
> + if (params->has_direct_io) {
> + dest->direct_io = params->direct_io;
> + }
> }
>
> static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
> @@ -1570,6 +1596,10 @@ static void migrate_params_apply(MigrateSetParameters
> *params, Error **errp)
> if (params->has_zero_page_detection) {
> s->parameters.zero_page_detection = params->zero_page_detection;
> }
> +
> + if (params->has_direct_io) {
> + s->parameters.direct_io = params->direct_io;
> + }
> }
I would expect to see something added to migrat_params_check() that
calls qemu_has_direct_io() and reports an error if the platform
lacks O_DIRECT, so mgmt apps see when they're trying to use O_DIRECT
on a bad platform straightaway, rather than only when migration
starts later.
Alternatively, and perhaps better would be for use to have a meson.build
check for O_DIRECT, and then make all the QAPI features have a condition
on CONFIG_O_DIRECT, so QAPI rejects any use of 'direct-io' feature at
input time.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
