On Oct 11, 2006, at 5:39 PM, Alasdair G Kergon wrote:
On Wed, Oct 11, 2006 at 10:20:40AM -0500, Jon Brassow wrote:I view this as a bug in the 'deactivate_lv' code, since it should fail if it is unable to deactivate the lv on all nodes.Correct, if that's true. But the code doesn't show any deactivation attempt! (see below)
Good spot. I was incorrectly understanding the lv_info... line to identify active lv's on other machines.
+Enforce resynching the mirrored logical volumes.Needs explaining better here for someone who doesn't know what that means/involves:-)
Explanation expanded.
+ if (!(lv->status & MIRRORED)) + return 1;Are you sure this shouldn't be an error?
I haven't changed this. So, if tried on a linear volume, there would be no error (but nothing would be done). This is easily changed if you like.
+ if (!lv_info(cmd, lv, &info, 0) || !info.exists) + active = 0;This is currently defined as telling you if the device is active on the local node. (Patrick's looking at extending it to the cluster.)
right.
+ if (active) { + if (!deactivate_lv(cmd, lv)) {+ log_error("Unable to deactivate, %s, for sync status change", lv->name);+ log_error("Hint: %s may be in-use", lv->name); + return 0; + }On a cluster, you need to call deactivate_lv *even if* it's not active on thelocal node. So test for the VG clustered flag and use that to decidewhether to do the lv_info/active test or not. (That's only a local optimisationnormally to assist with the error messages.)
right
+ /* Must activate log, or we can't zero it */ + if (!activate_lv(cmd, first_seg(lv)->log_lv)) { + log_error("Unable to activate, %s, for sync status change", + first_seg(lv)->log_lv->name); + activate_lv(cmd, lv); + return 0; + }Interesting. That extension to the interface needs more comment: you need to demonstrate that the operation is either atomic (which it doesn't appear to be at first sight), or else if it's not atomic, there must be no failure modes that lead to corruption, or people thinking a complete resync got forced whenit actually didn't.Normally you're not allowed to activate only part of a device tree - you can't activate a mirror log except as part of the mirror. Notice how lvcreate and lvconvert manipulate the log independently before attaching it to the mirror. However the recent changes to prepare for stacked devices should be sufficient for that particular activation to work, so that's probably acceptablenow. A breakdown into atomic operations might be:Detach the log (leaving the [inactive] mirror with a core log), wipe the log,then reattach it. Or you could introduce an additional metadata state to indicate the log needs to be wiped, wipe it, then revert to the previous metadata state.Or have a combination of both (so that a future lvchange -ay can completethe process if interrupted).+ /* + * Note: there is no need to deactivate the log before + * we activate the mirror + */Because of the stacking preparation now, yes, so long as set_lv() syncs theblock device correctly.
Basically, I only added more to the man page and a check for CLUSTERED in tools/lvchange.c. I've also added some comments in the patch as to why I believe the operation is atomic - posing no inconsistencies should the operation/machine fail while part way through.
brassow
Attachment:
forcesync_mirror_option.patch
Description: Binary data
_______________________________________________ linux-lvm mailing list linux-lvm@redhat.com https://www.redhat.com/mailman/listinfo/linux-lvm read the LVM HOW-TO at http://tldp.org/HOWTO/LVM-HOWTO/
