On Mon, Apr 29, 2024 at 08:55:35AM -0700, Steve Sistare wrote:
> Add the only-migratable-modes option as a generalization of only-migratable.
> Only devices that support all requested modes are allowed.
>
> Signed-off-by: Steve Sistare <steven.sist...@oracle.com>
> ---
> include/migration/misc.h | 3 +++
> include/sysemu/sysemu.h | 1 -
> migration/migration-hmp-cmds.c | 26 +++++++++++++++++++++++++-
> migration/migration.c | 22 +++++++++++++++++-----
> migration/savevm.c | 2 +-
> qemu-options.hx | 16 ++++++++++++++--
> system/globals.c | 1 -
> system/vl.c | 13 ++++++++++++-
> target/s390x/cpu_models.c | 4 +++-
> 9 files changed, 75 insertions(+), 13 deletions(-)
> diff --git a/qemu-options.hx b/qemu-options.hx
> index f0dfda5..946d731 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4807,8 +4807,20 @@ DEF("only-migratable", 0, QEMU_OPTION_only_migratable,
> \
> "-only-migratable allow only migratable devices\n", QEMU_ARCH_ALL)
> SRST
> ``-only-migratable``
> - Only allow migratable devices. Devices will not be allowed to enter
> - an unmigratable state.
> + Only allow devices that can migrate using normal mode. Devices will not
> + be allowed to enter an unmigratable state.
> +ERST
> +
> +DEF("only-migratable-modes", HAS_ARG, QEMU_OPTION_only_migratable_modes, \
> + "-only-migratable-modes mode1[,...]\n"
> + " allow only devices that are migratable using mode(s)\n",
> + QEMU_ARCH_ALL)
> +SRST
> +``-only-migratable-modes mode1[,...]``
> + Only allow devices which are migratable using all modes in the list,
> + which guarantees that migration will not fail due to a blocker.
> + If both only-migratable-modes and only-migratable are specified,
> + or are specified multiple times, then the required modes accumulate.
> ERST
Adding new top level CLI options is not something we much like doing
these days. Also its is preferrable to define args using QAPI rather
than creating hand written parsers
The pre-existing -only-migratable flag isn't ideal either, as a random
top level flag on its own.
I tend to think we should probably make both these arguments become
properties in the Machine class, and thus settable with the existing
-machine argument, since they're describing a requirement of the
machine and devices added to it.
The existing -only-migratable can be deprecated and simply made to
set the corresponding machien property
> diff --git a/system/globals.c b/system/globals.c
> index e353584..fdc263e 100644
> --- a/system/globals.c
> +++ b/system/globals.c
> @@ -48,7 +48,6 @@ const char *qemu_name;
> unsigned int nb_prom_envs;
> const char *prom_envs[MAX_PROM_ENVS];
> uint8_t *boot_splash_filedata;
> -int only_migratable; /* turn it off unless user states otherwise */
> int icount_align_option;
>
> /* The bytes in qemu_uuid are in the order specified by RFC4122, _not_ in the
> diff --git a/system/vl.c b/system/vl.c
> index b76881e..7e73be9 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -3458,7 +3458,18 @@ void qemu_init(int argc, char **argv)
> incoming = optarg;
> break;
> case QEMU_OPTION_only_migratable:
> - only_migratable = 1;
> + migration_set_required_mode(MIG_MODE_NORMAL);
> + break;
> + case QEMU_OPTION_only_migratable_modes:
> + {
> + int i, mode;
> + g_autofree char **words = g_strsplit(optarg, ",", -1);
> + for (i = 0; words[i]; i++) {
> + mode = qapi_enum_parse(&MigMode_lookup, words[i], -1,
> + &error_fatal);
> + migration_set_required_mode(mode);
> + }
> + }
> break;
> case QEMU_OPTION_nodefaults:
> has_defaults = 0;
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index 8ed3bb6..42ad160 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -16,6 +16,7 @@
> #include "kvm/kvm_s390x.h"
> #include "sysemu/kvm.h"
> #include "sysemu/tcg.h"
> +#include "migration/misc.h"
> #include "qapi/error.h"
> #include "qemu/error-report.h"
> #include "qapi/visitor.h"
> @@ -526,7 +527,8 @@ static void check_compatibility(const S390CPUModel
> *max_model,
> }
>
> #ifndef CONFIG_USER_ONLY
> - if (only_migratable && test_bit(S390_FEAT_UNPACK, model->features)) {
> + if (migration_mode_required(MIG_MODE_NORMAL) &&
> + test_bit(S390_FEAT_UNPACK, model->features)) {
> error_setg(errp, "The unpack facility is not compatible with "
> "the --only-migratable option. You must remove either "
> "the 'unpack' facility or the --only-migratable option");
> --
> 1.8.3.1
>
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 :|
