On Wed, 2021-09-15 at 22:32 -0500, Benjamin Marzinski wrote: > On Fri, Sep 10, 2021 at 01:41:15PM +0200, mwilck@xxxxxxxx wrote: > > From: Martin Wilck <mwilck@xxxxxxxx> > > > > This patch sets up the bulk of the state machine. The idea is to > > fall through the case labels as long as possible (when steps > > succeed) > > and return to the caller if either an error occurs, or it becomes > > necessary to wait for some pollable condition. > > > > While doing this, switch to negative error codes for the functions > > in uxlsnr.c (e.g. parse_cmd()). Positive return codes are reserved > > for the cli_handler functions themselves. This way we can clearly > > distinguish the error source, and avoid confusion and misleading > > error messages. No cli_handler returns negative values. > > > > Note: with this patch applied, clients may hang and time out if > > the handler fails to acquire the vecs lock. This will be fixed in > > the > > follow-up patch "multipathd: uxlsnr: add idle notification". > > > > Signed-off-by: Martin Wilck <mwilck@xxxxxxxx> > > --- > > multipathd/uxlsnr.c | 145 ++++++++++++++++++++++++---------------- > > ---- > > 1 file changed, 80 insertions(+), 65 deletions(-) > > > > > > + set_client_state(c, CLT_WAIT_LOCK); > > I don't have strong feelings about this, but this state machine > doesn't > want to always fall through. sometimes, like if you get -EPERM, you > want > to swith from CLT_PARSE to CLT_SEND. If instead of fallthroughs, you > just put the switch statement in a loop, and simply returned when you > wanted to break out to uxsock_listen, you could jump from any state > to > any other state, and wouldn't need to have code to skip the actions > of > some states, to enable the follow throughs. Just a thought. Good point. I'll give it a shot and see how it plays out. The end result will probably be better readable. I'll also work on your other points. Regards Martin -- dm-devel mailing list dm-devel@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/dm-devel
