Fail the migration request if options are set that are incompatible
with cpr.
Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
migration/migration.c | 17 +++++++++++++++++
qapi/migration.json | 2 ++
2 files changed, 19 insertions(+)
diff --git a/migration/migration.c b/migration/migration.c
index 90a9094..7652fd4 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1953,6 +1953,23 @@ static bool migrate_prepare(MigrationState *s, bool blk, bool blk_inc,
return false;
}
+ if (migrate_mode_is_cpr(s)) {
+ const char *conflict = NULL;
+
+ if (migrate_postcopy()) {
+ conflict = "postcopy";
+ } else if (migrate_background_snapshot()) {
+ conflict = "background snapshot";
+ } else if (migrate_colo()) {
+ conflict = "COLO";
+ }
+
+ if (conflict) {
+ error_setg(errp, "Cannot use %s with CPR", conflict);
+ return false;
+ }
+ }
+
if (blk || blk_inc) {
if (migrate_colo()) {
error_setg(errp, "No disk migration is required in COLO mode");
diff --git a/qapi/migration.json b/qapi/migration.json
index 0990297..c6bfe2e 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -657,6 +657,8 @@
# shared backend must be be non-volatile across reboot, such as by backing
# it with a dax device.
#
+# cpr-reboot may not be used with postcopy, colo, or background-snapshot.
+#
# (since 8.2)
##
{ 'enum': 'MigMode',
--
1.8.3.1Steve Sistare <steven.sistare@oracle.com> writes:
> Fail the migration request if options are set that are incompatible
> with cpr.
>
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
> migration/migration.c | 17 +++++++++++++++++
> qapi/migration.json | 2 ++
> 2 files changed, 19 insertions(+)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 90a9094..7652fd4 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1953,6 +1953,23 @@ static bool migrate_prepare(MigrationState *s, bool blk, bool blk_inc,
> return false;
> }
>
> + if (migrate_mode_is_cpr(s)) {
> + const char *conflict = NULL;
> +
> + if (migrate_postcopy()) {
> + conflict = "postcopy";
> + } else if (migrate_background_snapshot()) {
> + conflict = "background snapshot";
> + } else if (migrate_colo()) {
> + conflict = "COLO";
> + }
> +
> + if (conflict) {
> + error_setg(errp, "Cannot use %s with CPR", conflict);
> + return false;
> + }
> + }
> +
> if (blk || blk_inc) {
> if (migrate_colo()) {
> error_setg(errp, "No disk migration is required in COLO mode");
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 0990297..c6bfe2e 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -657,6 +657,8 @@
> # shared backend must be be non-volatile across reboot, such as by backing
> # it with a dax device.
> #
> +# cpr-reboot may not be used with postcopy, colo, or background-snapshot.
> +#
@cpr-reboot
COLO
Wrap the line:
# @cpr-reboot may not be used with postcopy, COLO, or
# background-snapshot.
This doesn't tell the reader what settings exactly do not work with
@cpr-reboot.
For instance "background-snapshot" is about enabling migration
capability @background-snapshot. We could write something like "is
incompatible with enabling migration capability @background-snapshot".
Same for the other two. Worthwhile?
> # (since 8.2)
> ##
> { 'enum': 'MigMode',
On 2/28/2024 2:21 AM, Markus Armbruster wrote:
> Steve Sistare <steven.sistare@oracle.com> writes:
>
>> Fail the migration request if options are set that are incompatible
>> with cpr.
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> ---
>> migration/migration.c | 17 +++++++++++++++++
>> qapi/migration.json | 2 ++
>> 2 files changed, 19 insertions(+)
>>
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 90a9094..7652fd4 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -1953,6 +1953,23 @@ static bool migrate_prepare(MigrationState *s, bool blk, bool blk_inc,
>> return false;
>> }
>>
>> + if (migrate_mode_is_cpr(s)) {
>> + const char *conflict = NULL;
>> +
>> + if (migrate_postcopy()) {
>> + conflict = "postcopy";
>> + } else if (migrate_background_snapshot()) {
>> + conflict = "background snapshot";
>> + } else if (migrate_colo()) {
>> + conflict = "COLO";
>> + }
>> +
>> + if (conflict) {
>> + error_setg(errp, "Cannot use %s with CPR", conflict);
>> + return false;
>> + }
>> + }
>> +
>> if (blk || blk_inc) {
>> if (migrate_colo()) {
>> error_setg(errp, "No disk migration is required in COLO mode");
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index 0990297..c6bfe2e 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -657,6 +657,8 @@
>> # shared backend must be be non-volatile across reboot, such as by backing
>> # it with a dax device.
>> #
>> +# cpr-reboot may not be used with postcopy, colo, or background-snapshot.
>> +#
>
> @cpr-reboot
>
> COLO
>
> Wrap the line:
>
> # @cpr-reboot may not be used with postcopy, COLO, or
> # background-snapshot.
>
> This doesn't tell the reader what settings exactly do not work with
> @cpr-reboot.
>
> For instance "background-snapshot" is about enabling migration
> capability @background-snapshot. We could write something like "is
> incompatible with enabling migration capability @background-snapshot".
>
> Same for the other two. Worthwhile?
I initially was more precise exactly as you suggest, but I realized that
postcopy encompasses multiple capabilities, and I did not want to enumerate
them, nor require new capabilities related to these 3 to be listed here
if/when they are created, so I generalized. A keyword search in this file
gives unambiguous matches.
Peter pushed the patch a few hours before you sent this.
- Steve
Steven Sistare <steven.sistare@oracle.com> writes: > On 2/28/2024 2:21 AM, Markus Armbruster wrote: >> Steve Sistare <steven.sistare@oracle.com> writes: >> >>> Fail the migration request if options are set that are incompatible >>> with cpr. >>> >>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com> [...] >>> diff --git a/qapi/migration.json b/qapi/migration.json >>> index 0990297..c6bfe2e 100644 >>> --- a/qapi/migration.json >>> +++ b/qapi/migration.json >>> @@ -657,6 +657,8 @@ >>> # shared backend must be be non-volatile across reboot, such as by backing >>> # it with a dax device. >>> # >>> +# cpr-reboot may not be used with postcopy, colo, or background-snapshot. >>> +# >> >> @cpr-reboot >> >> COLO >> >> Wrap the line: >> >> # @cpr-reboot may not be used with postcopy, COLO, or >> # background-snapshot. >> >> This doesn't tell the reader what settings exactly do not work with >> @cpr-reboot. >> >> For instance "background-snapshot" is about enabling migration >> capability @background-snapshot. We could write something like "is >> incompatible with enabling migration capability @background-snapshot". >> >> Same for the other two. Worthwhile? > > I initially was more precise exactly as you suggest, but I realized that > postcopy encompasses multiple capabilities, and I did not want to enumerate > them, nor require new capabilities related to these 3 to be listed here > if/when they are created, so I generalized. A keyword search in this file > gives unambiguous matches. > > Peter pushed the patch a few hours before you sent this. Okay. A followup patch to correct @cpr-reboot, COLO and line wrapping would be nice.
On 2/28/2024 11:05 AM, Markus Armbruster wrote: > Steven Sistare <steven.sistare@oracle.com> writes: > >> On 2/28/2024 2:21 AM, Markus Armbruster wrote: >>> Steve Sistare <steven.sistare@oracle.com> writes: >>> >>>> Fail the migration request if options are set that are incompatible >>>> with cpr. >>>> >>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com> > > [...] > >>>> diff --git a/qapi/migration.json b/qapi/migration.json >>>> index 0990297..c6bfe2e 100644 >>>> --- a/qapi/migration.json >>>> +++ b/qapi/migration.json >>>> @@ -657,6 +657,8 @@ >>>> # shared backend must be be non-volatile across reboot, such as by backing >>>> # it with a dax device. >>>> # >>>> +# cpr-reboot may not be used with postcopy, colo, or background-snapshot. >>>> +# >>> >>> @cpr-reboot >>> >>> COLO >>> >>> Wrap the line: >>> >>> # @cpr-reboot may not be used with postcopy, COLO, or >>> # background-snapshot. >>> >>> This doesn't tell the reader what settings exactly do not work with >>> @cpr-reboot. >>> >>> For instance "background-snapshot" is about enabling migration >>> capability @background-snapshot. We could write something like "is >>> incompatible with enabling migration capability @background-snapshot". >>> >>> Same for the other two. Worthwhile? >> >> I initially was more precise exactly as you suggest, but I realized that >> postcopy encompasses multiple capabilities, and I did not want to enumerate >> them, nor require new capabilities related to these 3 to be listed here >> if/when they are created, so I generalized. A keyword search in this file >> gives unambiguous matches. >> >> Peter pushed the patch a few hours before you sent this. > > Okay. > > A followup patch to correct @cpr-reboot, COLO and line wrapping would be > nice. Will do - steve
Steven Sistare <steven.sistare@oracle.com> writes: > On 2/28/2024 11:05 AM, Markus Armbruster wrote: >> Steven Sistare <steven.sistare@oracle.com> writes: >> >>> On 2/28/2024 2:21 AM, Markus Armbruster wrote: >>>> Steve Sistare <steven.sistare@oracle.com> writes: >>>> >>>>> Fail the migration request if options are set that are incompatible >>>>> with cpr. >>>>> >>>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com> >> >> [...] >> >>>>> diff --git a/qapi/migration.json b/qapi/migration.json >>>>> index 0990297..c6bfe2e 100644 >>>>> --- a/qapi/migration.json >>>>> +++ b/qapi/migration.json >>>>> @@ -657,6 +657,8 @@ >>>>> # shared backend must be be non-volatile across reboot, such as by backing >>>>> # it with a dax device. >>>>> # >>>>> +# cpr-reboot may not be used with postcopy, colo, or background-snapshot. >>>>> +# >>>> >>>> @cpr-reboot >>>> >>>> COLO >>>> >>>> Wrap the line: >>>> >>>> # @cpr-reboot may not be used with postcopy, COLO, or >>>> # background-snapshot. >>>> >>>> This doesn't tell the reader what settings exactly do not work with >>>> @cpr-reboot. >>>> >>>> For instance "background-snapshot" is about enabling migration >>>> capability @background-snapshot. We could write something like "is >>>> incompatible with enabling migration capability @background-snapshot". >>>> >>>> Same for the other two. Worthwhile? >>> >>> I initially was more precise exactly as you suggest, but I realized that >>> postcopy encompasses multiple capabilities, and I did not want to enumerate >>> them, nor require new capabilities related to these 3 to be listed here >>> if/when they are created, so I generalized. A keyword search in this file >>> gives unambiguous matches. >>> >>> Peter pushed the patch a few hours before you sent this. >> >> Okay. >> >> A followup patch to correct @cpr-reboot, COLO and line wrapping would be >> nice. > > Will do - steve Hmm, perhaps Peter can still squash in the corrections before posting his PR. Peter?
On Thu, Feb 29, 2024 at 06:31:26AM +0100, Markus Armbruster wrote: > Hmm, perhaps Peter can still squash in the corrections before posting > his PR. Peter? The PR was sent yesterday, it's already in PeterM's -staging. I worry it's a bit late to call a stop now. https://lore.kernel.org/qemu-devel/20240228051315.400759-23-peterx@redhat.com Steve, could you provide a standalone patch for the update? Then I'll do the best accordingly (e.g. if the PR failed to apply I'll squash when resend v2; or I'll pick it up for the next). Thanks, -- Peter Xu
On 2/29/2024 12:40 AM, Peter Xu wrote: > On Thu, Feb 29, 2024 at 06:31:26AM +0100, Markus Armbruster wrote: >> Hmm, perhaps Peter can still squash in the corrections before posting >> his PR. Peter? > > The PR was sent yesterday, it's already in PeterM's -staging. I worry it's > a bit late to call a stop now. > > https://lore.kernel.org/qemu-devel/20240228051315.400759-23-peterx@redhat.com > > Steve, could you provide a standalone patch for the update? Then I'll do > the best accordingly (e.g. if the PR failed to apply I'll squash when > resend v2; or I'll pick it up for the next). I sent the patch. I also re-wrapped all cpr-reboot paragraphs to 70 columns and addressed Markus' other comments on "migration: update cpr-reboot description". Peter, if you squash it, the last sentence "cpr-reboot may not be used ..." squashes into migration: options incompatible with cpr and everything else squashes into migration: update cpr-reboot description - Steve
On Thu, Feb 22, 2024 at 09:28:40AM -0800, Steve Sistare wrote: > Fail the migration request if options are set that are incompatible > with cpr. > > Signed-off-by: Steve Sistare <steven.sistare@oracle.com> Reviewed-by: Peter Xu <peterx@redhat.com> -- Peter Xu
© 2016 - 2026 Red Hat, Inc.