On Thu, Jun 16, 2011 at 02:41:03PM +0900, Simon Horman wrote: > > One question though : what flags are emitted in the logs when a > > session is closed that way ? I'm wondering if we should not add a new flag > > such as "D" (for "down") and/or "K" (for "killed") on the first character > > to indicate why the session was terminated. It would also make room for the > > ability to kill sessions from the CLI later. But this new flag could impact > > more code, we have to check this. > > Good point, I'll take a look into implementing that.
OK. Be careful, I remember about a bitfield and a mask in the session which is set by set_term_flags(). From what I recall, we currently have 8 values (3 full bits) so it might not be that obvious to implement. > > One comment about the "struct list server" in the session. I'm afraid this > > name could cause confusion with the "srv" field, especially since it appears > > at the beginning. Maybe we should find another name such as "by_srv" or > > something like this to more clearly indicate it's the node used to chain > > the session from the server's list. > > I'm not entirely sure what a good name is, but I would like to avoid > confusion. I'll change it to by_srv and repost. It should be easy enough > to change it again later if we come up with a better name. OK, and I agree we can change it later if needed. > > Last point, I'm amazed by the number of functions you managed to turn to > > static. A few years ago this would not have been possible at all, and when > > I read your changes, for each of them I remember what we changed which made > > that possible. This means to me that the code is becoming much more modular > > and that's a very good news ! I'll try to be careful in the future about > > what functions can be turned to static after changes (provided it makes > > sense of course). > > Great. > > I did restrict those changes to a few files. Mainly because I wanted > to see if the changes were acceptable or not. I assume there is quite > a lot of scope for other files to have symbols made static. The idea should be that we don't want to jump on this and change all the code, because sometimes I have to do the opposite. But doing so in the context of related changes makes sense of course. Regards, Willy
