dm-devel-bounces@xxxxxxxxxx wrote on 20.10.2006 21:36:54: > > > "disk" is format2. > > or "clustered_disk" or "clustered_core" or any other logging > implementation that we may develop in the future, like "multi_disk". > The problem is, then we need to either encode all those in the > mirror_ctr or wait for 'create_dirty_log' to return NULL - then pass on > to the second format, which may also return an error. > > And if we again decide to change the format in the future... > > With the format argument, we don't need the extra logic. We simply > pass on the rest to the correct constructor function for that format. > Well, I guess this is just a minor issue. It is a simple approach to to it that way. My point was just that "core" and "disk" (if these were not used as section label) would indicate the old format. The new parser would get used for all formats, simply skipping sections that it doesn't understand. But "formatX" is ok. > For simplicity, I've kept the offset (pvmove uses it), removed the > status arg, and made the section argument count consistent. I now > think it should look something like: > devices 4 253:3 0 253:4 0 > (The example patch expects this.) > > > devices 2 2 253:3 rr_min=1000 3 253:4 mstate=m rr_min=1000 > > be done more than once... > > I agree that having to state a device multiple times is ugly. However, > I don't like parsing sections multiple times for different things. I'd > really like to have a 'section to function' mapping if possible. > Additionally, we can't be sure what extra information (sections) we may > add in the future. (Think device status, or other). That could be > solved by your solution above and would simply change the per device > argument count; but again, I'd like to avoid parsing the same section > multiple times - or mangling the order of parameters to much just to > pass them to say, the path selector constructor for read balancing.) > Yes, it makes it simpler to break up parsing. The other approach, while it might be simpler to read would mean every place interrested in device options would have to parse them again. And it would require all parsers to change. And that is a too big change. What makes me a bit uneasy about having devices in several places is that this allows configuration mistakes that could be fatal. For example give different devices to device section and round-robin section... > > Thank-you for your comments Stefan. Below is a quick patch of how I > would see the code changes for the constructor. (I didn't put in the > per-section functions. Nor did I put in the read balancing stuff yet. > This is just an example of how to make the constructor more flexible... > paving the way for read balancing.) > Well, besides my "bad feelings" this looks good to me. But at the moment I can't think of a better solution that wouldn't require changes to other parsers. :( Regards, Stefan -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
