Re: [dm-devel] [PATCH 30/35] multipathd: uxlsnr: merge uxsock_trigger() into state machine

[Martin Wilck <mwilck@suse.com>]

On Wed, 2021-09-15 at 22:32 -0500, Benjamin Marzinski wrote:
> On Fri, Sep 10, 2021 at 01:41:15PM +0200, mwilck@suse.com wrote:
> > From: Martin Wilck <mwilck@suse.com>
> > 
> > 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@suse.com>
> > ---
> >  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@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel