include/qapi/qmp/qerror.h | 3 +++ include/sysemu/sysemu.h | 1 + migration/migration.c | 18 +++++++++++++++++- qemu-options.hx | 7 +++++++ softmmu/globals.c | 1 + softmmu/vl.c | 3 +++ 6 files changed, 32 insertions(+), 1 deletion(-)
When a migration blocker is added nothing is reported to the user,
inability to migrate such guest may come as a late surprise. As a bare
minimum, we can print a warning. To not pollute the output for those, who
have no intention to migrate their guests, introduce '--no-migration'
option which both block the migration and eliminates warning from
Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
include/qapi/qmp/qerror.h | 3 +++
include/sysemu/sysemu.h | 1 +
migration/migration.c | 18 +++++++++++++++++-
qemu-options.hx | 7 +++++++
softmmu/globals.c | 1 +
softmmu/vl.c | 3 +++
6 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 596fce0c54e7..2e1563c72f83 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -50,6 +50,9 @@
#define QERR_MISSING_PARAMETER \
"Parameter '%s' is missing"
+#define QERR_NO_MIGRATION \
+ "Guest is not migratable ('--no-migration' used)"
+
#define QERR_PERMISSION_DENIED \
"Insufficient permission to perform this operation"
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 8fae667172ac..c65cd5d5a336 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -9,6 +9,7 @@
/* vl.c */
extern int only_migratable;
+extern int no_migration;
extern const char *qemu_name;
extern QemuUUID qemu_uuid;
extern bool qemu_uuid_set;
diff --git a/migration/migration.c b/migration/migration.c
index ca8b97baa5ac..29a8480ced54 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1077,7 +1077,9 @@ static void fill_source_migration_info(MigrationInfo *info)
info->blocked = migration_is_blocked(NULL);
info->has_blocked_reasons = info->blocked;
info->blocked_reasons = NULL;
- if (info->blocked) {
+ if (no_migration) {
+ QAPI_LIST_PREPEND(info->blocked_reasons, g_strdup(QERR_NO_MIGRATION));
+ } else if (info->blocked) {
GSList *cur_blocker = migration_blockers;
/*
@@ -2048,6 +2050,10 @@ void migrate_init(MigrationState *s)
int migrate_add_blocker(Error *reason, Error **errp)
{
+ if (!no_migration) {
+ warn_report("Guest won't be migratable: %s", error_get_pretty(reason));
+ }
+
if (only_migratable) {
error_propagate_prepend(errp, error_copy(reason),
"disallowing migration blocker "
@@ -2155,6 +2161,11 @@ bool migration_is_blocked(Error **errp)
return true;
}
+ if (no_migration) {
+ error_setg(errp, QERR_NO_MIGRATION);
+ return true;
+ }
+
if (migration_blockers) {
error_propagate(errp, error_copy(migration_blockers->data));
return true;
@@ -2198,6 +2209,11 @@ static bool migrate_prepare(MigrationState *s, bool blk, bool blk_inc,
return true;
}
+ if (no_migration) {
+ error_setg(errp, QERR_NO_MIGRATION);
+ return false;
+ }
+
if (migration_is_running(s->state)) {
error_setg(errp, QERR_MIGRATION_ACTIVE);
return false;
diff --git a/qemu-options.hx b/qemu-options.hx
index fd21002bd61d..3443130273e9 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4234,6 +4234,13 @@ SRST
an unmigratable state.
ERST
+DEF("no-migration", 0, QEMU_OPTION_no_migration, \
+ "-no-migration disallow migration\n", QEMU_ARCH_ALL)
+SRST
+``-no-migration``
+ Disallow migration. Don't warn about non-migratable configurations.
+ERST
+
DEF("nodefaults", 0, QEMU_OPTION_nodefaults, \
"-nodefaults don't create default devices\n", QEMU_ARCH_ALL)
SRST
diff --git a/softmmu/globals.c b/softmmu/globals.c
index 7d0fc811835a..bb0d892df307 100644
--- a/softmmu/globals.c
+++ b/softmmu/globals.c
@@ -59,6 +59,7 @@ int boot_menu;
bool boot_strict;
uint8_t *boot_splash_filedata;
int only_migratable; /* turn it off unless user states otherwise */
+int no_migration;
int icount_align_option;
/* The bytes in qemu_uuid are in the order specified by RFC4122, _not_ in the
diff --git a/softmmu/vl.c b/softmmu/vl.c
index aadb52613888..9a6535e594c3 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -3339,6 +3339,9 @@ void qemu_init(int argc, char **argv, char **envp)
case QEMU_OPTION_only_migratable:
only_migratable = 1;
break;
+ case QEMU_OPTION_no_migration:
+ no_migration = 1;
+ break;
case QEMU_OPTION_nodefaults:
has_defaults = 0;
break;
--
2.30.2
Vitaly Kuznetsov <vkuznets@redhat.com> writes:
> When a migration blocker is added nothing is reported to the user,
> inability to migrate such guest may come as a late surprise. As a bare
> minimum, we can print a warning. To not pollute the output for those, who
> have no intention to migrate their guests, introduce '--no-migration'
> option which both block the migration and eliminates warning from
Sure this justifies its own command line option? Can we make it a
parameter of an existing option? We have too many options, and options
starting "no-" are often awkward.
> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
> include/qapi/qmp/qerror.h | 3 +++
> include/sysemu/sysemu.h | 1 +
> migration/migration.c | 18 +++++++++++++++++-
> qemu-options.hx | 7 +++++++
> softmmu/globals.c | 1 +
> softmmu/vl.c | 3 +++
> 6 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
> index 596fce0c54e7..2e1563c72f83 100644
> --- a/include/qapi/qmp/qerror.h
> +++ b/include/qapi/qmp/qerror.h
> @@ -50,6 +50,9 @@
/*
* These macros will go away, please don't use in new code, and do not
* add new ones!
*/
...
> #define QERR_MISSING_PARAMETER \
> "Parameter '%s' is missing"
>
> +#define QERR_NO_MIGRATION \
> + "Guest is not migratable ('--no-migration' used)"
> +
> #define QERR_PERMISSION_DENIED \
> "Insufficient permission to perform this operation"
>
Please don't add new macros here.
Looks like you use QERR_NO_MIGRATION only in migration/migration.c. You
can add a macro there. Or simply duplicate the error string, which I'd
find easier to read. Up to you.
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 8fae667172ac..c65cd5d5a336 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -9,6 +9,7 @@
> /* vl.c */
>
> extern int only_migratable;
> +extern int no_migration;
> extern const char *qemu_name;
> extern QemuUUID qemu_uuid;
> extern bool qemu_uuid_set;
> diff --git a/migration/migration.c b/migration/migration.c
> index ca8b97baa5ac..29a8480ced54 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1077,7 +1077,9 @@ static void fill_source_migration_info(MigrationInfo *info)
> info->blocked = migration_is_blocked(NULL);
> info->has_blocked_reasons = info->blocked;
> info->blocked_reasons = NULL;
> - if (info->blocked) {
> + if (no_migration) {
> + QAPI_LIST_PREPEND(info->blocked_reasons, g_strdup(QERR_NO_MIGRATION));
> + } else if (info->blocked) {
> GSList *cur_blocker = migration_blockers;
>
> /*
Apropos blocked-reasons. migration.json has:
# @blocked: True if outgoing migration is blocked (since 6.0)
#
# @blocked-reasons: A list of reasons an outgoing migration is blocked (since 6.0)
[...]
'blocked': 'bool',
'*blocked-reasons': ['str'],
Can "blocked-reasons" be absent or empty when "blocked" is true?
If not, then "blocked" is redundant, and should be dropped before we
release 6.0.
Else, the documentation should spell it out. No need to rush that.
The patch was not cc'ed to me. I might have caught it earlier...
[...]
Markus Armbruster <armbru@redhat.com> writes:
[...]
> Apropos blocked-reasons. migration.json has:
>
> # @blocked: True if outgoing migration is blocked (since 6.0)
> #
> # @blocked-reasons: A list of reasons an outgoing migration is blocked (since 6.0)
> [...]
> 'blocked': 'bool',
> '*blocked-reasons': ['str'],
>
> Can "blocked-reasons" be absent or empty when "blocked" is true?
No.
From fill_source_migration_info():
info->blocked = migration_is_blocked(NULL);
info->has_blocked_reasons = info->blocked;
info->blocked_reasons = NULL;
if (info->blocked) {
GSList *cur_blocker = migration_blockers;
/*
* There are two types of reasons a migration might be blocked;
* a) devices marked in VMState as non-migratable, and
* b) Explicit migration blockers
* We need to add both of them here.
*/
qemu_savevm_non_migratable_list(&info->blocked_reasons);
while (cur_blocker) {
QAPI_LIST_PREPEND(info->blocked_reasons,
g_strdup(error_get_pretty(cur_blocker->data)));
cur_blocker = g_slist_next(cur_blocker);
}
}
where
bool migration_is_blocked(Error **errp)
{
if (qemu_savevm_state_blocked(errp)) {
return true;
}
if (migration_blockers) {
error_propagate(errp, error_copy(migration_blockers->data));
return true;
}
return false;
}
and
bool qemu_savevm_state_blocked(Error **errp)
{
SaveStateEntry *se;
QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
if (se->vmsd && se->vmsd->unmigratable) {
error_setg(errp, "State blocked by non-migratable device '%s'",
se->idstr);
return true;
}
}
return false;
}
void qemu_savevm_non_migratable_list(strList **reasons)
{
SaveStateEntry *se;
QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
if (se->vmsd && se->vmsd->unmigratable) {
QAPI_LIST_PREPEND(*reasons,
g_strdup_printf("non-migratable device: %s",
se->idstr));
}
}
}
info->blocked is "non-migratable devices exist, or migration blockers
exist".
info->blocked_reasons has one entry per non-migratable device, and one
entry per migration blocker.
> If not, then "blocked" is redundant, and should be dropped before we
> release 6.0.
It is, and it should.
> Else, the documentation should spell it out. No need to rush that.
>
> The patch was not cc'ed to me. I might have caught it earlier...
"The patch" is commit 3af8554bd0 "migration: Add blocker information"
>
> [...]
Markus Armbruster <armbru@redhat.com> writes:
> Vitaly Kuznetsov <vkuznets@redhat.com> writes:
>
>> When a migration blocker is added nothing is reported to the user,
>> inability to migrate such guest may come as a late surprise. As a bare
>> minimum, we can print a warning. To not pollute the output for those, who
>> have no intention to migrate their guests, introduce '--no-migration'
>> option which both block the migration and eliminates warning from
>
> Sure this justifies its own command line option? Can we make it a
> parameter of an existing option? We have too many options, and options
> starting "no-" are often awkward.
We already have
-only-migratable allow only migratable devices
which fails any migration blocker, not just devices.
On Thu, 15 Apr 2021 at 16:46, Vitaly Kuznetsov <vkuznets@redhat.com> wrote: > When a migration blocker is added nothing is reported to the user, > inability to migrate such guest may come as a late surprise. As a bare > minimum, we can print a warning. To not pollute the output for those, who > have no intention to migrate their guests, introduce '--no-migration' > option which both block the migration and eliminates warning from I'm not a fan. For a lot of people and configurations this is going to be "add an extra complaint from QEMU to a previously working configuration". We add too many of those already. thanks -- PMM
On Sun, Apr 18, 2021 at 11:54 AM Peter Maydell <peter.maydell@linaro.org> wrote: > > On Thu, 15 Apr 2021 at 16:46, Vitaly Kuznetsov <vkuznets@redhat.com> wrote: > > When a migration blocker is added nothing is reported to the user, > > inability to migrate such guest may come as a late surprise. As a bare > > minimum, we can print a warning. To not pollute the output for those, who > > have no intention to migrate their guests, introduce '--no-migration' > > option which both block the migration and eliminates warning from > > I'm not a fan. For a lot of people and configurations this > is going to be "add an extra complaint from QEMU to a previously > working configuration". We add too many of those already. I agree that warning with machine types that never supported live migration would be useless noise, but warning if using an explicit versioned machine type sounds like a reasonable default (as long as the warnings includes clear instructions on how to silence them). -- Eduardo
On Mon, Apr 19, 2021 at 12:28:04PM -0400, Eduardo Habkost wrote: > On Sun, Apr 18, 2021 at 11:54 AM Peter Maydell <peter.maydell@linaro.org> wrote: > > > > On Thu, 15 Apr 2021 at 16:46, Vitaly Kuznetsov <vkuznets@redhat.com> wrote: > > > When a migration blocker is added nothing is reported to the user, > > > inability to migrate such guest may come as a late surprise. As a bare > > > minimum, we can print a warning. To not pollute the output for those, who > > > have no intention to migrate their guests, introduce '--no-migration' > > > option which both block the migration and eliminates warning from > > > > I'm not a fan. For a lot of people and configurations this > > is going to be "add an extra complaint from QEMU to a previously > > working configuration". We add too many of those already. > > I agree that warning with machine types that never supported live > migration would be useless noise, but warning if using an explicit > versioned machine type sounds like a reasonable default (as long as > the warnings includes clear instructions on how to silence them). Libvirt will always expand a machine type alias into a versioned machine type, because stable ABI is useful even if never migrating, because it ensures the guest OS doesn't see hardware changes that may trigger license re-activation At the same time a large portion of users will never care about live migration/save/restore, or they do care but will hotunplug the problems devices before attempting a migration. IMHO tieing messages to versioned machine types is not desirable as a strategy by default. Warning about migration compatibility should be an explicit opt-in 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 :|
© 2016 - 2025 Red Hat, Inc.