Re: [PATCH 4/6] migration: Add ram-only capability
On 1/14/22 14:22, Markus Armbruster wrote:
Nikita Lapshin writes:
If this capability is enabled migration stream
will have RAM section only.
Signed-off-by: Nikita Lapshin
[...]
diff --git a/qapi/migration.json b/qapi/migration.json
index d53956852c..626fc59d14 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -454,6 +454,8 @@
#
# @no-ram: If enabled, migration stream won't contain any ram in it. (since
7.0)
#
+# @ram-only: If enabled, only RAM sections will be sent. (since 7.0)
+#
What happens when I ask for 'no-ram': true, 'ram-only': true?
migrate_caps_check() will return false and print error message that these
capabilities are not compatible.
# Features:
# @unstable: Members @x-colo and @x-ignore-shared are experimental.
#
@@ -467,7 +469,7 @@
'block', 'return-path', 'pause-before-switchover', 'multifd',
'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate',
{ 'name': 'x-ignore-shared', 'features': [ 'unstable' ] },
- 'validate-uuid', 'background-snapshot', 'no-ram'] }
+ 'validate-uuid', 'background-snapshot', 'no-ram', 'ram-only'] }
##
# @MigrationCapabilityStatus:
#
@@ -521,7 +523,8 @@
# {"state": true, "capability": "events"},
# {"state": false, "capability": "postcopy-ram"},
# {"state": false, "capability": "x-colo"},
-# {"state": false, "capability": "no-ram"}
+# {"state": false, "capability": "no-ram"},
+# {"state": false, "capability": "ram-only"}
#]}
#
##
Re: [PATCH 4/6] migration: Add ram-only capability
14.01.2022 18:55, Markus Armbruster wrote:
Daniel P. Berrangé writes:
On Fri, Jan 14, 2022 at 12:22:13PM +0100, Markus Armbruster wrote:
Nikita Lapshin writes:
If this capability is enabled migration stream
will have RAM section only.
Signed-off-by: Nikita Lapshin
[...]
diff --git a/qapi/migration.json b/qapi/migration.json
index d53956852c..626fc59d14 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -454,6 +454,8 @@
#
# @no-ram: If enabled, migration stream won't contain any ram in it. (since
7.0)
#
+# @ram-only: If enabled, only RAM sections will be sent. (since 7.0)
+#
What happens when I ask for 'no-ram': true, 'ram-only': true?
So IIUC
no-ram=false, ram-only=false => RAM + vmstate
no-ram=true, ram-only=false => vmstate
no-ram=false, ram-only=true => RAM
no-ram=true, ram-only=true => nothing to send ?
I find that the fact that one flag is a negative request and
the other flag is a positive request to be confusing.
Me too.
If we must have two flags then could we at least use the same
style for both. ie either
@no-ram
@no-vmstate
Or
@ram-only
@vmstate-only
I strongly prefer "positive" names for booleans, avoiding double
negation.
Since the code enforces these flags are mutually exclusive
though, it might point towards being handled by a enum
{ 'enum': 'MigrationStreamContent',
'data': ['both', 'ram', 'vmstate'] }
Enumerating the combinations that are actually valid is often nicer than
a set of flags that look independent, but aren't.
MigrationCapability can only do flags. For an enum, we'd have to use
MigrationParameters & friends. For an example, check out
@multifd-compression there.
Remember, that we also have block-dirty-bitmaps in migration stream. And we
also may have block devices data (should it be deprecated?).
So, what about an optional migration parameter stream-content, which would be a
*list* of MigrationStreamContent enum values?
Than we can deprecate dirty-bitmaps and block capabilities in favor of new
migration parameter.
{ 'enum': 'MigrationStreamContent', 'data': ['block-dirty-bitmpas', 'block',
'ram', 'vmstate'] }
...
Migration parameters:
...
'stream-content': ['MigrationStreamContent']
--
Best regards,
Vladimir
Re: [PATCH 4/6] migration: Add ram-only capability
Daniel P. Berrangé writes:
> On Fri, Jan 14, 2022 at 12:22:13PM +0100, Markus Armbruster wrote:
>> Nikita Lapshin writes:
>>
>> > If this capability is enabled migration stream
>> > will have RAM section only.
>> >
>> > Signed-off-by: Nikita Lapshin
>>
>> [...]
>>
>> > diff --git a/qapi/migration.json b/qapi/migration.json
>> > index d53956852c..626fc59d14 100644
>> > --- a/qapi/migration.json
>> > +++ b/qapi/migration.json
>> > @@ -454,6 +454,8 @@
>> > #
>> > # @no-ram: If enabled, migration stream won't contain any ram in it.
>> > (since 7.0)
>> > #
>> > +# @ram-only: If enabled, only RAM sections will be sent. (since 7.0)
>> > +#
>>
>> What happens when I ask for 'no-ram': true, 'ram-only': true?
>
> So IIUC
>
> no-ram=false, ram-only=false => RAM + vmstate
> no-ram=true, ram-only=false => vmstate
> no-ram=false, ram-only=true => RAM
> no-ram=true, ram-only=true => nothing to send ?
>
> I find that the fact that one flag is a negative request and
> the other flag is a positive request to be confusing.
Me too.
> If we must have two flags then could we at least use the same
> style for both. ie either
>
> @no-ram
> @no-vmstate
>
> Or
>
> @ram-only
> @vmstate-only
I strongly prefer "positive" names for booleans, avoiding double
negation.
> Since the code enforces these flags are mutually exclusive
> though, it might point towards being handled by a enum
>
> { 'enum': 'MigrationStreamContent',
> 'data': ['both', 'ram', 'vmstate'] }
Enumerating the combinations that are actually valid is often nicer than
a set of flags that look independent, but aren't.
MigrationCapability can only do flags. For an enum, we'd have to use
MigrationParameters & friends. For an example, check out
@multifd-compression there.
> none of these approaches are especially future proof if we ever
> need fine grained control over sending a sub-set of the non-RAM
> vmstate. Not sure if that matters in the end.
>
>
> Regards,
> Daniel
Re: [PATCH 4/6] migration: Add ram-only capability
On Fri, Jan 14, 2022 at 12:22:13PM +0100, Markus Armbruster wrote:
> Nikita Lapshin writes:
>
> > If this capability is enabled migration stream
> > will have RAM section only.
> >
> > Signed-off-by: Nikita Lapshin
>
> [...]
>
> > diff --git a/qapi/migration.json b/qapi/migration.json
> > index d53956852c..626fc59d14 100644
> > --- a/qapi/migration.json
> > +++ b/qapi/migration.json
> > @@ -454,6 +454,8 @@
> > #
> > # @no-ram: If enabled, migration stream won't contain any ram in it.
> > (since 7.0)
> > #
> > +# @ram-only: If enabled, only RAM sections will be sent. (since 7.0)
> > +#
>
> What happens when I ask for 'no-ram': true, 'ram-only': true?
So IIUC
no-ram=false, ram-only=false => RAM + vmstate
no-ram=true, ram-only=false => vmstate
no-ram=false, ram-only=true => RAM
no-ram=true, ram-only=true => nothing to send ?
I find that the fact that one flag is a negative request and
the other flag is a positive request to be confusing.
If we must have two flags then could we at least use the same
style for both. ie either
@no-ram
@no-vmstate
Or
@ram-only
@vmstate-only
Since the code enforces these flags are mutually exclusive
though, it might point towards being handled by a enum
{ 'enum': 'MigrationStreamContent',
'data': ['both', 'ram', 'vmstate'] }
none of these approaches are especially future proof if we ever
need fine grained control over sending a sub-set of the non-RAM
vmstate. Not sure if that matters in the end.
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 :|
Re: [PATCH 4/6] migration: Add ram-only capability
Nikita Lapshin writes:
> If this capability is enabled migration stream
> will have RAM section only.
>
> Signed-off-by: Nikita Lapshin
[...]
> diff --git a/qapi/migration.json b/qapi/migration.json
> index d53956852c..626fc59d14 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -454,6 +454,8 @@
> #
> # @no-ram: If enabled, migration stream won't contain any ram in it. (since
> 7.0)
> #
> +# @ram-only: If enabled, only RAM sections will be sent. (since 7.0)
> +#
What happens when I ask for 'no-ram': true, 'ram-only': true?
> # Features:
> # @unstable: Members @x-colo and @x-ignore-shared are experimental.
> #
> @@ -467,7 +469,7 @@
> 'block', 'return-path', 'pause-before-switchover', 'multifd',
> 'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate',
> { 'name': 'x-ignore-shared', 'features': [ 'unstable' ] },
> - 'validate-uuid', 'background-snapshot', 'no-ram'] }
> + 'validate-uuid', 'background-snapshot', 'no-ram', 'ram-only'] }
> ##
> # @MigrationCapabilityStatus:
> #
> @@ -521,7 +523,8 @@
> # {"state": true, "capability": "events"},
> # {"state": false, "capability": "postcopy-ram"},
> # {"state": false, "capability": "x-colo"},
> -# {"state": false, "capability": "no-ram"}
> +# {"state": false, "capability": "no-ram"},
> +# {"state": false, "capability": "ram-only"}
> #]}
> #
> ##
Re: [PATCH 4/6] migration: Add ram-only capability
24.12.2021 14:11, Nikita Lapshin wrote:
If this capability is enabled migration stream
will have RAM section only.
Signed-off-by: Nikita Lapshin
---
migration/migration.c | 20 +++-
migration/migration.h | 1 +
migration/savevm.c| 11 ++-
qapi/migration.json | 7 +--
4 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 006447d00a..4d7ef9d8dc 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1257,6 +1257,14 @@ static bool migrate_caps_check(bool *cap_list,
return false;
}
+if (cap_list[MIGRATION_CAPABILITY_NO_RAM] &&
+cap_list[MIGRATION_CAPABILITY_RAM_ONLY]) {
+error_setg(errp, "ram-only and no-ram aren't "
+ "compatible with each other");
+
+return false;
+}
+
return true;
}
@@ -2619,6 +2627,15 @@ bool migrate_no_ram(void)
return s->enabled_capabilities[MIGRATION_CAPABILITY_NO_RAM];
}
+bool migrate_ram_only(void)
+{
+MigrationState *s;
+
+s = migrate_get_current();
+
+return s->enabled_capabilities[MIGRATION_CAPABILITY_RAM_ONLY];
+}
+
/* migration thread support */
/*
* Something bad happened to the RP stream, mark an error
@@ -3973,7 +3990,8 @@ static void *bg_migration_thread(void *opaque)
* save their state to channel-buffer along with devices.
*/
cpu_synchronize_all_states();
-if (qemu_savevm_state_complete_precopy_non_iterable(fb, false, false)) {
+if (!migrate_ram_only() &&
+qemu_savevm_state_complete_precopy_non_iterable(fb, false, false)) {
Here... [*]
goto fail;
}
/*
diff --git a/migration/migration.h b/migration/migration.h
index 43f7bf8eba..d5a077e00d 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -359,6 +359,7 @@ bool migrate_use_events(void);
bool migrate_postcopy_blocktime(void);
bool migrate_background_snapshot(void);
bool migrate_no_ram(void);
+bool migrate_ram_only(void);
/* Sending on the return path - generic and then for each message type */
void migrate_send_rp_shut(MigrationIncomingState *mis,
diff --git a/migration/savevm.c b/migration/savevm.c
index ba644083ab..e41ca76000 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -249,6 +249,7 @@ typedef struct SaveStateEntry {
void *opaque;
CompatEntry *compat;
int is_iterable;
+bool is_ram;
} SaveStateEntry;
typedef struct SaveState {
@@ -802,6 +803,10 @@ int register_savevm_live(const char *idstr,
se->is_iterable = 1;
}
+if (!strcmp("ram", idstr)) {
+se->is_ram = true;
+}
+
pstrcat(se->idstr, sizeof(se->idstr), idstr);
if (instance_id == VMSTATE_INSTANCE_ID_ANY) {
@@ -949,6 +954,10 @@ static bool should_skip(SaveStateEntry *se)
return true;
}
+if (migrate_ram_only() && !se->is_ram) {
+return true;
+}
+
return false;
}
@@ -1486,7 +1495,7 @@ int qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only,
}
}
-if (iterable_only) {
+if (iterable_only || migrate_ram_only()) {
goto flush;
}
[*] ... and here you care to avoid calling same
qemu_savevm_state_complete_precopy_non_iterable()
Seems better to check migrate_ram_only at start of
qemu_savevm_state_complete_precopy_non_iterable() and do early return from it.
diff --git a/qapi/migration.json b/qapi/migration.json
index d53956852c..626fc59d14 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -454,6 +454,8 @@
#
# @no-ram: If enabled, migration stream won't contain any ram in it. (since
7.0)
#
+# @ram-only: If enabled, only RAM sections will be sent. (since 7.0)
+#
# Features:
# @unstable: Members @x-colo and @x-ignore-shared are experimental.
#
@@ -467,7 +469,7 @@
'block', 'return-path', 'pause-before-switchover', 'multifd',
'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate',
{ 'name': 'x-ignore-shared', 'features': [ 'unstable' ] },
- 'validate-uuid', 'background-snapshot', 'no-ram'] }
+ 'validate-uuid', 'background-snapshot', 'no-ram', 'ram-only'] }
##
# @MigrationCapabilityStatus:
#
@@ -521,7 +523,8 @@
# {"state": true, "capability": "events"},
# {"state": false, "capability": "postcopy-ram"},
# {"state": false, "capability": "x-colo"},
-# {"state": false, "capability": "no-ram"}
+# {"state": false, "capability": "no-ram"},
+# {"state": false, "capability": "ram-only"}
#]}
#
##
--
Best regards,
Vladimir
[PATCH 4/6] migration: Add ram-only capability
If this capability is enabled migration stream
will have RAM section only.
Signed-off-by: Nikita Lapshin
---
migration/migration.c | 20 +++-
migration/migration.h | 1 +
migration/savevm.c| 11 ++-
qapi/migration.json | 7 +--
4 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 006447d00a..4d7ef9d8dc 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1257,6 +1257,14 @@ static bool migrate_caps_check(bool *cap_list,
return false;
}
+if (cap_list[MIGRATION_CAPABILITY_NO_RAM] &&
+cap_list[MIGRATION_CAPABILITY_RAM_ONLY]) {
+error_setg(errp, "ram-only and no-ram aren't "
+ "compatible with each other");
+
+return false;
+}
+
return true;
}
@@ -2619,6 +2627,15 @@ bool migrate_no_ram(void)
return s->enabled_capabilities[MIGRATION_CAPABILITY_NO_RAM];
}
+bool migrate_ram_only(void)
+{
+MigrationState *s;
+
+s = migrate_get_current();
+
+return s->enabled_capabilities[MIGRATION_CAPABILITY_RAM_ONLY];
+}
+
/* migration thread support */
/*
* Something bad happened to the RP stream, mark an error
@@ -3973,7 +3990,8 @@ static void *bg_migration_thread(void *opaque)
* save their state to channel-buffer along with devices.
*/
cpu_synchronize_all_states();
-if (qemu_savevm_state_complete_precopy_non_iterable(fb, false, false)) {
+if (!migrate_ram_only() &&
+qemu_savevm_state_complete_precopy_non_iterable(fb, false, false)) {
goto fail;
}
/*
diff --git a/migration/migration.h b/migration/migration.h
index 43f7bf8eba..d5a077e00d 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -359,6 +359,7 @@ bool migrate_use_events(void);
bool migrate_postcopy_blocktime(void);
bool migrate_background_snapshot(void);
bool migrate_no_ram(void);
+bool migrate_ram_only(void);
/* Sending on the return path - generic and then for each message type */
void migrate_send_rp_shut(MigrationIncomingState *mis,
diff --git a/migration/savevm.c b/migration/savevm.c
index ba644083ab..e41ca76000 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -249,6 +249,7 @@ typedef struct SaveStateEntry {
void *opaque;
CompatEntry *compat;
int is_iterable;
+bool is_ram;
} SaveStateEntry;
typedef struct SaveState {
@@ -802,6 +803,10 @@ int register_savevm_live(const char *idstr,
se->is_iterable = 1;
}
+if (!strcmp("ram", idstr)) {
+se->is_ram = true;
+}
+
pstrcat(se->idstr, sizeof(se->idstr), idstr);
if (instance_id == VMSTATE_INSTANCE_ID_ANY) {
@@ -949,6 +954,10 @@ static bool should_skip(SaveStateEntry *se)
return true;
}
+if (migrate_ram_only() && !se->is_ram) {
+return true;
+}
+
return false;
}
@@ -1486,7 +1495,7 @@ int qemu_savevm_state_complete_precopy(QEMUFile *f, bool
iterable_only,
}
}
-if (iterable_only) {
+if (iterable_only || migrate_ram_only()) {
goto flush;
}
diff --git a/qapi/migration.json b/qapi/migration.json
index d53956852c..626fc59d14 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -454,6 +454,8 @@
#
# @no-ram: If enabled, migration stream won't contain any ram in it. (since
7.0)
#
+# @ram-only: If enabled, only RAM sections will be sent. (since 7.0)
+#
# Features:
# @unstable: Members @x-colo and @x-ignore-shared are experimental.
#
@@ -467,7 +469,7 @@
'block', 'return-path', 'pause-before-switchover', 'multifd',
'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate',
{ 'name': 'x-ignore-shared', 'features': [ 'unstable' ] },
- 'validate-uuid', 'background-snapshot', 'no-ram'] }
+ 'validate-uuid', 'background-snapshot', 'no-ram', 'ram-only'] }
##
# @MigrationCapabilityStatus:
#
@@ -521,7 +523,8 @@
# {"state": true, "capability": "events"},
# {"state": false, "capability": "postcopy-ram"},
# {"state": false, "capability": "x-colo"},
-# {"state": false, "capability": "no-ram"}
+# {"state": false, "capability": "no-ram"},
+# {"state": false, "capability": "ram-only"}
#]}
#
##
--
2.27.0
