Peter Xu <peterx(a)redhat.com> writes:
On Mon, Apr 29, 2024 at 02:18:57PM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx(a)redhat.com> 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.