Re: [RFC v2] migration: Add migrate-set-bitmap-node-mapping

[Max Reitz]

On 14.05.20 10:42, Dr. David Alan Gilbert wrote:
> * Max Reitz (address@hidden) wrote:
> 
> <snip>
> 
>> +void qmp_migrate_set_bitmap_node_mapping(MigrationBlockNodeMappingList 
>> *mapping,
>> +                                         Error **errp)
>> +{
>> +    QDict *in_mapping = qdict_new();
>> +    QDict *out_mapping = qdict_new();
>> +
>> +    for (; mapping; mapping = mapping->next) {
>> +        MigrationBlockNodeMapping *entry = mapping->value;
>> +
>> +        if (qdict_haskey(out_mapping, entry->node_name)) {
>> +            error_setg(errp, "Cannot map node name '%s' twice",
>> +                       entry->node_name);
>> +            goto fail;
>> +        }
> 
> I'm not too clear exactly which case this is protecting against;
> I think that's protecting against mapping
> 
>   'src1'->'dst1' and 'src1'->'dst2'
> which is a good check.s (or maybe it's checking against dst2 twice?)

This one is against mapping src1 twice.  Both checks together check that
it’s a one-to-one bijective mapping.

The technical reason why it needs to be one-to-one is because we base
two QDicts off of it, so the inverse mapping needs to work.

> What about cases where there is no mapping - e.g. imagine
> that we have b1/b2 on the source and b2/b3 on the dest; now
> if we add just a mapping:
> 
>   b1->b2
> 
> then we end up with:
> 
>   b1 -> b2
>   b2 -> b2  (non-mapped)
>         b3
> 
> so we have a clash there - are we protected against that?

Oh, no, we aren’t.  That wasn’t intentional.  However, I’m not sure how
we’d protect against it.  We can’t check it in
qmp_migrate_set_bitmap_node_mapping(), because we don’t know yet which
nodes will exist at the time of migration, and which of those will have
bitmaps.

So we’d need to check it as part of the migration process (by looking up
any unmapped entries that default to the identity mapping in the
respective reverse mapping, to see whether anything maps to the same name).

OTOH, Vladimir proposed adding a parameter to
migrate-set-bitmap-node-mapping that would make migration fail if any
bitmaps should be migrated off of unmapped nodes, or if any incoming
alias is unmapped (instead of defaulting to the identity mapping).  If
we just make that the only behavior, then we wouldn’t have a problem
with that at all, because all unmapped nodes would always throw an error.

(And on the third hand, I wonder whether we should actually allow
migrating bitmaps from multiple nodes to a single one, but I suppose
that would require two separate commands, one for incoming and one for
outgoing...)

Max

signature.asc
Description: OpenPGP digital signature