[dm-devel] [PATCH 11/14] dm-zoned: support arbitrary number of devices

[Hannes Reinecke]

On 6/1/20 1:54 AM, Damien Le Moal wrote:
> On 2020/05/31 22:06, Hannes Reinecke wrote:
>> On 5/31/20 11:10 AM, Damien Le Moal wrote:
[ .. ]
>>>
>>> Looks all good to me, but thinking more about it, don't we need to add
>>> a device index in the super blocks ? The reason is that if the drive
>>> configuration changes between stopt/start (drives removed, added or
>>> changed slots), the drive names will change and while the userspace
>>> will still be able to find the group of drives constituting the target
>>> (using UUID9, there is no obvious way to find out what the original
>>> drive order was. Since the kernel side relies on the drive being passed
>>> to the ctr function in the order of the mapping, we need to preserve
>>> that. Or change also the kernel side to use the index in the super
>>> block to put each drive in its correct dmz->dev[] slot.
>>>
>> Already taken care of; here's where the tertiary superblocks come in.
>> Each superblock carries its own position (in the 'sb_block' field).
>> This is the _absolute_ position within the entire setup, not the
>> relative per-device block number.
>> And it also has the absolute number of blocks in the 'nr_chunks' field.
>>
>> Hence we know exactly where this superblock (and, by implication, the
>> zones following this superblock) should end up. And we know how large
>> the entire setup will be. So can insert the superblock at the right
>> position and then can check if we have enough zones for the entire
>> device.
> 
> I do not get it though. Where is that checked ? At least in this patch, drives
> are initialized in the order of the ctr arguments, and this loop:
> 
> +		for (i = 1; i < dmz->nr_ddevs; i++) {
> +			dmz->dev[i].zone_offset = zone_offset;
> +			zone_offset += dmz->dev[i].nr_zones;
> +		}
> 
> in dmz_fixup_devices() sets the zone offset for each device in the same order.
> So for a given chunk mapped to a zone identified by its ID, if the device order
> changes, zone ID will change and the chunk will not be mapped to the correct
> zone. What am I missing here ?
> 
Well, I _did_ state that we're missing support for it; all I did was 
pointing out that the metadata already has the capability for detecting 
a mismatch. And I do think we're getting a warning when loading 
superblocks, and the setup would be rejected.

But then I just checked, and we're indeed missing the sb_block 
validation. Will be adding it such that we're rejecting out-of-order 
devices.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke            Teamlead Storage & Networking
hare at suse.de                               +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer