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

[Dr. David Alan Gilbert]

* Max Reitz (address@hidden) wrote:
> 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).

Yeh a once through check of all the nodes at the start of the migration
would probably fix it.

> 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.

Yeh that would force you to put a full mapping table in place.

> (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...)

Wouldn't that get very messy?

Dave

> Max
> 



--
Dr. David Alan Gilbert / address@hidden / Manchester, UK