On 14.05.20 10:42, Dr. David Alan Gilbert wrote:
> * Max Reitz (mre...@redhat.com) 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
