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