Re: [PATCH v2 3/6] migration: Remove 'blk/-b' option from migrate commands

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