Peter Xu <peterx@xxxxxxxxxx> writes:
> On Mon, Apr 29, 2024 at 02:18:57PM -0300, Fabiano Rosas wrote:
>> Peter Xu <peterx@xxxxxxxxxx> writes:
>>
>> > On Fri, Apr 26, 2024 at 10:14:05AM -0300, Fabiano Rosas wrote:
>> >> @@ -2003,21 +1997,7 @@ static bool migrate_prepare(MigrationState *s, bool blk, bool resume,
>> >> }
>> >> }
>> >>
>> >> - if (blk) {
>> >> - if (migrate_colo()) {
>> >> - error_setg(errp, "No disk migration is required in COLO mode");
>> >> - return false;
>> >> - }
>> >> - if (migrate_block()) {
>> >> - error_setg(errp, "Command options are incompatible with "
>> >> - "current migration capabilities");
>> >> - return false;
>> >> - }
>> >> - if (!migrate_cap_set(MIGRATION_CAPABILITY_BLOCK, true, errp)) {
>> >> - return false;
>> >> - }
>> >> - s->must_remove_block_options = true;
>> >> - }
>> >> + s->must_remove_block_options = true;
>> >
>> > Can we drop this var too? Perhaps with block_cleanup_parameters()?
>>
>> Yes, Markus mentioned it in v1 already. Take a look there. There's
>> several other declarations I missed. v3 is coming soon.
>
> Right, noticed that it's removed actually in the next patch.
>
> But iiuc it can already been removed in this patch. If we want to remove
> it in the next, logically we should set must_remove_block_options=false
> here, though.. So maybe easier to just drop it here.
Ah I see what you mean. I thought you're just asking for the removal
overall.
But block_cleanup_parameters sets the block capability to false and the
whole block migration only goes away in the next patch. I think we need
to keep this as true to preserve behavior. In theory, after this patch
people could still use the block migration just fine by setting the cap.
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
