On Thu, Oct 11, 2018 at 06:03:20PM +0300, Nikolay Borisov wrote: > Here is the second posting of the fsid change support for the kernel. For > background information you can refer to v1 [0]. The main changes in this > version > are around the handling of possible split-brain scenarios. I've changed a bit > how the userspace code works and now the process is split among 2 > transactions. > The first one flagging "we are about to change fsid" and once it's persisted > on > all disks a second one does the actual change. This of course is not enough > to guarantee full consistency so I had to extend the device scanning to > gracefully handle such cases. I believe I have covered everything but more > review will be appreciated.
All the cases seem to be covered. Do you intend to add the design document somewhere? The references in the code seem stale and puzzling. > So patch 1 implements the basic functionality with no split-brain handling > whatsoever. Patch 2 is a minor cleanup. Patch 3 deals with a split-brain that > can occur if power loss happens during the initial transaction (the one > setting > the beginning flag). Patch 4 adds some information that is needed in the last > 2 > patches. Patch 5 handles failure between transaction 1 and transaction 2 and > finally patch 6 handles the case of power loss during transaction 1 but for > an > fs which has already undergone at least one successful fsid change. More > details about the exact failure modes are in each respective patch. > > One thing which could be improved but I ran out of ideas is the naming of the > ancillary functions - find_fsid_inprogress and find_fsid_changed. Hm, no better ideas so it can stay and be changed later. > I've actually tested the split-brain handing code with specially crafted > images. > They will be part of the user-space submissions and I believe I have full > coverage for that. Perfect, thanks. Now the bad news from me :) There are several coding style and style issues all over the patches so I'll list them here. * BTRFS_SUPER_FLAG_CHANGING_FSID_v2 -- V2 with uppercase V, as other versioned symbols * all references in changelogs or comments should refer to the super flag as CHANGING_FSID_v2, not FSID_CHANGING, not just CHANGING_FSID, nor FSID_CHANGE. This is because we want to be able to search for it and find all occurences * error messages should stick to the preferred format, https://btrfs.wiki.kernel.org/index.php/Development_notes#Error_messages * comments referring to UUID should use "UUID", there's a mixt of both ways added by the patches, error messages should use the uppercase form too * find_fsid - type and name should be on one line, parameters on the next if they don't fit * device_list_add - missing space before = As these are not functional problems, I'll add the patchset to for-next for testing.
