Re: [RFC v2 0/4] Allow shutdown of sessions when a server becomes unavailable

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 Received on 2011/06/16 07:53