On Tue, 1 Nov 2011 18:26:14 +0200 Alexander Lyakas <alex.bolshoy@xxxxxxxxx> wrote: > Hi Neil, > I hope you don't find my questions too pesky; I hope as time goes, I > will be able to contribute as well, and not just ask for information. Enquiring minds should always be encouraged! > > What you suggested works great, but only if the drive already has a > valid superblock. This is according to md_import_device(), which calls > super_1_load(). Although it calls it with NULL refdev, but it still > performs basic validity checks of the superblock. So it is needed to > have functionality of write_init_super() of mdadm (perhaps can be an > mdadm operation, not sure how dangerous this is to expose such > functionality). Correct. With v1.x metadata - and even more so with DDF and IMSM - the kernel does not know everything about the metadata and cannot create new superlocks. At best it can edit what is there. As for adding things to mdadm - you just need a genuine use-case. I can see it could be valid to add a '--role' option to --add so you can add a device to a specific slot. I doubt it would be used much, but if there was a concrete need I wouldn't object. > > Another question I was digging for, is to find a spot where kernel > determines whether it is going to do a bitmap-based reconstruction or > a full reconstruction. Can you verify that I found it correctly, or I > am way off? > > In super_1_validate(): > if (ev1 < mddev->bitmap->events_cleared) > return 0; > This means that the array bitmap was cleared after the drive was > detached, so we bail right out, and do not set rdev->raid_disk, and > leave all flags off. In that case, eventually, we will need full > reconstruction. > Otherwise, after we decide that this drive is an active device (not > failed or spare): > if ((le32_to_cpu(sb->feature_map) & MD_FEATURE_RECOVERY_OFFSET)) > rdev->recovery_offset = le64_to_cpu(sb->recovery_offset); > else > set_bit(In_sync, &rdev->flags); > Here we might or might not set the In_sync flag. > Yes. 'In_sync' effectively means that recovery_offset == MAX. > Then in slot_store() and in add_new_disk(), we set the important flag > for that matter: > if (test_bit(In_sync, &rdev->flags)) > rdev->saved_raid_disk = slot; > else > rdev->saved_raid_disk = -1; > > And later, in hot_add_disk (like raid5_add_disk, raid1_add_disk), we > check this flag and set/not set the fullsync flag. And in > raid1_add_disk, like you said, the slot doesn't matter, any valid > saved_raid_disk is good. > > Is this correct? If yes, then in the sequence you suggested, we will > always do full reconstruction. Because new_dev_store() and > slot_store() sequence do not call validate_super(), so In_sync is > never set, so saved_raid_disk remains -1. This is perfectly fine. Yes, this is all correct. In the sequence I suggested you could add echo insync > dev-$DEVNAME/state before echo $slot > dev-$DEVNAME/slot and it would result in saved_raid_disk being set. NeilBrown
Attachment:
signature.asc
Description: PGP signature
