From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Teigland Date: Tue, 25 Nov 2008 09:47:20 -0600 Subject: [Cluster-devel] Re: Groupd uevent clean up In-Reply-To: <1227622122.9571.99.camel@quoit> References: <1227622122.9571.99.camel@quoit> Message-ID: <20081125154720.GA24769@redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On Tue, Nov 25, 2008 at 02:08:42PM +0000, Steven Whitehouse wrote: > > The following patch is designed to clean up a number of items relating to > gfs_controld's handling of uevent notifications. When a uevent is received > is consists of a number of strings whose total length is bounded by the > buffer size and returned by the recv call. > > Originally only the initial string was being used to select the various > actions. That was wrong for various reasons, but mostly because its much > easier to simply look for the individual fields, broken out from the > message. The other reason that this is wrong, and also the reason that > I started looking into this area initially is that the initial event > string provides a "physical" view of the event. By using the SUBSYSTEM= > field, we get the same information, but it can also be overridden by > the kernel using the kset_uevent_ops which will become important when > we merge the lock_dlm module into GFS2. > > The other problem (unsolved here) is that we have to parse the fs name > from the DEVPATH= variable. Its trivial to add a feature to the kernel > to provide the LOCKTABLE as well, so I plan to do that in the future > so we don't have to jump through hoops to get that information. > > At the same time I've marked a bunch of variables const. Mainly > because they started to throw up warnings after I'd added the > new event parsing code. > > I've tested the parsing code on its own, but since I don't have the > right packages installed here, I've not had a chance to build & > test a complete new groupd yet. OK, thanks, I dislike the const sprinkling, and I don't care about string parsing. I'll take your word that this string parsing is an improvement over what's there (I really dislike string parsing, previously get_args() was used for splitting args from other strings, too). All I know is that what's there works, so if you can verify that the new parsing really works I'll add it (or I can do a quick test if you resend a patch with just that bit). > Also, I've fixed up some broken error handling in process_uevent > at the same time. Yep, good catch, I'll add that small bit right now. > +/* FIXME: All of the below functions call find_mg as pretty much > + * the first thing that they do. This ought to be done here and > + * the found mg (if any) passed to the routines. > + */ It didn't work out well last time I tried that, one of the main sticking points was that the "old" leave code needs to first look for the mg in a different list... once we get rid of the old code (post cluster3 split) we can try again.