| Message ID | 20211118225840.19810-30-mwilck@suse.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | christophe varoqui |
| Headers | show |
| Series | multipathd: uxlsnr overhaul | expand |
On Thu, Nov 18, 2021 at 11:58:21PM +0100, mwilck@suse.com wrote: > From: Martin Wilck <mwilck@suse.com> > > This patch sets up the bulk of the state machine. client_state_machine() > is called in a loop, proceeding from state to state until it needs > to poll for input or wait for a lock, in which case it returns > STM_BREAK. > > 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". > Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com> > Signed-off-by: Martin Wilck <mwilck@suse.com> > --- > multipathd/uxlsnr.c | 160 ++++++++++++++++++++++++-------------------- > 1 file changed, 89 insertions(+), 71 deletions(-) > > diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c > index ff9604f..87134d5 100644 > --- a/multipathd/uxlsnr.c > +++ b/multipathd/uxlsnr.c > @@ -299,22 +299,13 @@ static int parse_cmd(struct client *c) > > r = get_cmdvec(c->cmd, &c->cmdvec); > > - if (r) { > - genhelp_handler(c->cmd, r, &c->reply); > - if (get_strbuf_len(&c->reply) == 0) > - return EINVAL; > - return 0; > - } > + if (r) > + return -r; > > c->handler = find_handler_for_cmdvec(c->cmdvec); > > - if (!c->handler || !c->handler->fn) { > - genhelp_handler(c->cmd, EINVAL, &c->reply); > - if (get_strbuf_len(&c->reply) == 0) > - r = EINVAL; > - else > - r = 0; > - } > + if (!c->handler || !c->handler->fn) > + return -EINVAL; > > return r; > } > @@ -325,7 +316,7 @@ static int execute_handler(struct client *c, struct vectors *vecs, int timeout) > struct timespec tmo; > > if (!c->handler) > - return EINVAL; > + return -EINVAL; > > if (clock_gettime(CLOCK_REALTIME, &tmo) == 0) { > tmo.tv_sec += timeout; > @@ -355,50 +346,30 @@ static int execute_handler(struct client *c, struct vectors *vecs, int timeout) > return r; > } > > -static int uxsock_trigger(struct client *c, void *trigger_data) > +void default_reply(struct client *c, int r) > { > - struct vectors * vecs; > - int r = 1; > - > - vecs = (struct vectors *)trigger_data; > - > - r = parse_cmd(c); > - > - if (r == 0 && c->cmdvec && VECTOR_SIZE(c->cmdvec) > 0) { > - struct key *kw = VECTOR_SLOT(c->cmdvec, 0); > - > - if (!c->is_root && kw->code != LIST) > - r = EPERM; > - } > - > - if (r == 0 && c->handler) > - r = execute_handler(c, vecs, uxsock_timeout / 1000); > - > - if (c->cmdvec) { > - free_keys(c->cmdvec); > - c->cmdvec = NULL; > - } > - > - if (r > 0) { > - switch(r) { > - case ETIMEDOUT: > - append_strbuf_str(&c->reply, "timeout\n"); > - break; > - case EPERM: > - append_strbuf_str(&c->reply, > - "permission deny: need to be root\n"); > - break; > - default: > - append_strbuf_str(&c->reply, "fail\n"); > - break; > - } > - } > - else if (!r && get_strbuf_len(&c->reply) == 0) { > + switch(r) { > + case -EINVAL: > + case -ESRCH: > + case -ENOMEM: > + /* return codes from get_cmdvec() */ > + genhelp_handler(c->cmd, -r, &c->reply); > + break; > + case -EPERM: > + append_strbuf_str(&c->reply, > + "permission deny: need to be root\n"); > + break; > + case -ETIMEDOUT: > + append_strbuf_str(&c->reply, "timeout\n"); > + break; > + case 0: > append_strbuf_str(&c->reply, "ok\n"); > - r = 0; > + break; > + default: > + /* cli_handler functions return 1 on unspecified error */ > + append_strbuf_str(&c->reply, "fail\n"); > + break; > } > - /* else if (r < 0) leave *reply alone */ > - return r; > } > > static void set_client_state(struct client *c, int state) > @@ -409,6 +380,7 @@ static void set_client_state(struct client *c, int state) > reset_strbuf(&c->reply); > memset(c->cmd, '\0', sizeof(c->cmd)); > c->expires = ts_zero; > + c->error = 0; > /* fallthrough */ > case CLT_SEND: > /* reuse these fields for next data transfer */ > @@ -420,11 +392,20 @@ static void set_client_state(struct client *c, int state) > c->state = state; > } > > -static void handle_client(struct client *c, void *trigger_data) > +enum { > + STM_CONT, > + STM_BREAK, > +}; > + > +static int client_state_machine(struct client *c, struct vectors *vecs) > { > ssize_t n; > + const char *buf; > > - switch (c->state) { > + condlog(4, "%s: cli[%d] state=%d cmd=\"%s\" repl \"%s\"", __func__, > + c->fd, c->state, c->cmd, get_strbuf_str(&c->reply)); > + > + switch (c->state) { > case CLT_RECV: > if (c->cmd_len == 0) { > /* > @@ -449,31 +430,59 @@ static void handle_client(struct client *c, void *trigger_data) > condlog(4, "%s: cli[%d]: connected", __func__, c->fd); > } > /* poll for data */ > - return; > + return STM_BREAK; > } else if (c->len < c->cmd_len) { > n = recv(c->fd, c->cmd + c->len, c->cmd_len - c->len, 0); > if (n <= 0 && errno != EINTR && errno != EAGAIN) { > condlog(1, "%s: cli[%d]: error in recv: %m", > __func__, c->fd); > c->error = -ECONNRESET; > - return; > + return STM_BREAK; > } > c->len += n; > if (c->len < c->cmd_len) > /* continue polling */ > - return; > - set_client_state(c, CLT_PARSE); > + return STM_BREAK; > } > - break; > - default: > - break; > - } > + condlog(4, "cli[%d]: Got request [%s]", c->fd, c->cmd); > + set_client_state(c, CLT_PARSE); > + return STM_CONT; > > - condlog(4, "cli[%d]: Got request [%s]", c->fd, c->cmd); > - uxsock_trigger(c, trigger_data); > + case CLT_PARSE: > + c->error = parse_cmd(c); > + if (!c->error) { > + /* Permission check */ > + struct key *kw = VECTOR_SLOT(c->cmdvec, 0); > > - if (get_strbuf_len(&c->reply) > 0) { > - const char *buf = get_strbuf_str(&c->reply); > + if (!c->is_root && kw->code != LIST) { > + c->error = -EPERM; > + condlog(0, "%s: cli[%d]: unauthorized cmd \"%s\"", > + __func__, c->fd, c->cmd); > + } > + } > + if (c->error) > + set_client_state(c, CLT_SEND); > + else > + set_client_state(c, CLT_WORK); > + return STM_CONT; > + > + case CLT_WAIT_LOCK: > + /* tbd */ > + set_client_state(c, CLT_WORK); > + return STM_CONT; > + > + case CLT_WORK: > + c->error = execute_handler(c, vecs, uxsock_timeout / 1000); > + free_keys(c->cmdvec); > + c->cmdvec = NULL; > + set_client_state(c, CLT_SEND); > + return STM_CONT; > + > + case CLT_SEND: > + if (get_strbuf_len(&c->reply) == 0) > + default_reply(c, c->error); > + > + buf = get_strbuf_str(&c->reply); > > if (send_packet(c->fd, buf) != 0) > dead_client(c); > @@ -481,9 +490,18 @@ static void handle_client(struct client *c, void *trigger_data) > condlog(4, "cli[%d]: Reply [%zu bytes]", c->fd, > get_strbuf_len(&c->reply) + 1); > reset_strbuf(&c->reply); > - } > > - set_client_state(c, CLT_RECV); > + set_client_state(c, CLT_RECV); > + return STM_BREAK; > + > + default: > + return STM_BREAK; > + } > +} > + > +static void handle_client(struct client *c, struct vectors *vecs) > +{ > + while (client_state_machine(c, vecs) == STM_CONT); > } > > /* > -- > 2.33.1 -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
On Thu, Nov 18, 2021 at 11:58:21PM +0100, mwilck@suse.com wrote: > From: Martin Wilck <mwilck@suse.com> > > This patch sets up the bulk of the state machine. client_state_machine() > is called in a loop, proceeding from state to state until it needs > to poll for input or wait for a lock, in which case it returns > STM_BREAK. > > 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". > Actually, one nitpick. See below > Signed-off-by: Martin Wilck <mwilck@suse.com> > --- > multipathd/uxlsnr.c | 160 ++++++++++++++++++++++++-------------------- > 1 file changed, 89 insertions(+), 71 deletions(-) > > diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c > index ff9604f..87134d5 100644 > --- a/multipathd/uxlsnr.c > +++ b/multipathd/uxlsnr.c > @@ -299,22 +299,13 @@ static int parse_cmd(struct client *c) > > r = get_cmdvec(c->cmd, &c->cmdvec); > > - if (r) { > - genhelp_handler(c->cmd, r, &c->reply); > - if (get_strbuf_len(&c->reply) == 0) > - return EINVAL; > - return 0; > - } > + if (r) > + return -r; > > c->handler = find_handler_for_cmdvec(c->cmdvec); > > - if (!c->handler || !c->handler->fn) { > - genhelp_handler(c->cmd, EINVAL, &c->reply); > - if (get_strbuf_len(&c->reply) == 0) > - r = EINVAL; > - else > - r = 0; > - } > + if (!c->handler || !c->handler->fn) > + return -EINVAL; > > return r; > } > @@ -325,7 +316,7 @@ static int execute_handler(struct client *c, struct vectors *vecs, int timeout) > struct timespec tmo; > > if (!c->handler) > - return EINVAL; > + return -EINVAL; > > if (clock_gettime(CLOCK_REALTIME, &tmo) == 0) { > tmo.tv_sec += timeout; > @@ -355,50 +346,30 @@ static int execute_handler(struct client *c, struct vectors *vecs, int timeout) > return r; > } > > -static int uxsock_trigger(struct client *c, void *trigger_data) > +void default_reply(struct client *c, int r) > { > - struct vectors * vecs; > - int r = 1; > - > - vecs = (struct vectors *)trigger_data; > - > - r = parse_cmd(c); > - > - if (r == 0 && c->cmdvec && VECTOR_SIZE(c->cmdvec) > 0) { > - struct key *kw = VECTOR_SLOT(c->cmdvec, 0); > - > - if (!c->is_root && kw->code != LIST) > - r = EPERM; > - } > - > - if (r == 0 && c->handler) > - r = execute_handler(c, vecs, uxsock_timeout / 1000); > - > - if (c->cmdvec) { > - free_keys(c->cmdvec); > - c->cmdvec = NULL; > - } > - > - if (r > 0) { > - switch(r) { > - case ETIMEDOUT: > - append_strbuf_str(&c->reply, "timeout\n"); > - break; > - case EPERM: > - append_strbuf_str(&c->reply, > - "permission deny: need to be root\n"); > - break; > - default: > - append_strbuf_str(&c->reply, "fail\n"); > - break; > - } > - } > - else if (!r && get_strbuf_len(&c->reply) == 0) { > + switch(r) { > + case -EINVAL: > + case -ESRCH: > + case -ENOMEM: > + /* return codes from get_cmdvec() */ > + genhelp_handler(c->cmd, -r, &c->reply); > + break; > + case -EPERM: > + append_strbuf_str(&c->reply, > + "permission deny: need to be root\n"); > + break; > + case -ETIMEDOUT: > + append_strbuf_str(&c->reply, "timeout\n"); > + break; > + case 0: > append_strbuf_str(&c->reply, "ok\n"); > - r = 0; > + break; > + default: > + /* cli_handler functions return 1 on unspecified error */ > + append_strbuf_str(&c->reply, "fail\n"); > + break; > } > - /* else if (r < 0) leave *reply alone */ > - return r; > } > > static void set_client_state(struct client *c, int state) > @@ -409,6 +380,7 @@ static void set_client_state(struct client *c, int state) > reset_strbuf(&c->reply); > memset(c->cmd, '\0', sizeof(c->cmd)); > c->expires = ts_zero; > + c->error = 0; > /* fallthrough */ > case CLT_SEND: > /* reuse these fields for next data transfer */ > @@ -420,11 +392,20 @@ static void set_client_state(struct client *c, int state) > c->state = state; > } > > -static void handle_client(struct client *c, void *trigger_data) > +enum { > + STM_CONT, > + STM_BREAK, > +}; > + > +static int client_state_machine(struct client *c, struct vectors *vecs) > { > ssize_t n; > + const char *buf; > > - switch (c->state) { > + condlog(4, "%s: cli[%d] state=%d cmd=\"%s\" repl \"%s\"", __func__, > + c->fd, c->state, c->cmd, get_strbuf_str(&c->reply)); > + This switch statement is indented with 8 spaces, instead of a tab -Ben > + switch (c->state) { > case CLT_RECV: > if (c->cmd_len == 0) { > /* > @@ -449,31 +430,59 @@ static void handle_client(struct client *c, void *trigger_data) > condlog(4, "%s: cli[%d]: connected", __func__, c->fd); > } > /* poll for data */ > - return; > + return STM_BREAK; > } else if (c->len < c->cmd_len) { > n = recv(c->fd, c->cmd + c->len, c->cmd_len - c->len, 0); > if (n <= 0 && errno != EINTR && errno != EAGAIN) { > condlog(1, "%s: cli[%d]: error in recv: %m", > __func__, c->fd); > c->error = -ECONNRESET; > - return; > + return STM_BREAK; > } > c->len += n; > if (c->len < c->cmd_len) > /* continue polling */ > - return; > - set_client_state(c, CLT_PARSE); > + return STM_BREAK; > } > - break; > - default: > - break; > - } > + condlog(4, "cli[%d]: Got request [%s]", c->fd, c->cmd); > + set_client_state(c, CLT_PARSE); > + return STM_CONT; > > - condlog(4, "cli[%d]: Got request [%s]", c->fd, c->cmd); > - uxsock_trigger(c, trigger_data); > + case CLT_PARSE: > + c->error = parse_cmd(c); > + if (!c->error) { > + /* Permission check */ > + struct key *kw = VECTOR_SLOT(c->cmdvec, 0); > > - if (get_strbuf_len(&c->reply) > 0) { > - const char *buf = get_strbuf_str(&c->reply); > + if (!c->is_root && kw->code != LIST) { > + c->error = -EPERM; > + condlog(0, "%s: cli[%d]: unauthorized cmd \"%s\"", > + __func__, c->fd, c->cmd); > + } > + } > + if (c->error) > + set_client_state(c, CLT_SEND); > + else > + set_client_state(c, CLT_WORK); > + return STM_CONT; > + > + case CLT_WAIT_LOCK: > + /* tbd */ > + set_client_state(c, CLT_WORK); > + return STM_CONT; > + > + case CLT_WORK: > + c->error = execute_handler(c, vecs, uxsock_timeout / 1000); > + free_keys(c->cmdvec); > + c->cmdvec = NULL; > + set_client_state(c, CLT_SEND); > + return STM_CONT; > + > + case CLT_SEND: > + if (get_strbuf_len(&c->reply) == 0) > + default_reply(c, c->error); > + > + buf = get_strbuf_str(&c->reply); > > if (send_packet(c->fd, buf) != 0) > dead_client(c); > @@ -481,9 +490,18 @@ static void handle_client(struct client *c, void *trigger_data) > condlog(4, "cli[%d]: Reply [%zu bytes]", c->fd, > get_strbuf_len(&c->reply) + 1); > reset_strbuf(&c->reply); > - } > > - set_client_state(c, CLT_RECV); > + set_client_state(c, CLT_RECV); > + return STM_BREAK; > + > + default: > + return STM_BREAK; > + } > +} > + > +static void handle_client(struct client *c, struct vectors *vecs) > +{ > + while (client_state_machine(c, vecs) == STM_CONT); > } > > /* > -- > 2.33.1 -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
On Wed, 2021-11-24 at 18:38 -0600, Benjamin Marzinski wrote: > On Thu, Nov 18, 2021 at 11:58:21PM +0100, mwilck@suse.com wrote: > > From: Martin Wilck <mwilck@suse.com> > > > > This patch sets up the bulk of the state machine. > > client_state_machine() > > is called in a loop, proceeding from state to state until it needs > > to poll for input or wait for a lock, in which case it returns > > STM_BREAK. > > > > 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". > > > > Actually, one nitpick. See below > > > + > > This switch statement is indented with 8 spaces, instead of a tab I'm going to fix that, but I assume you're aware that our code is far from being consistent in this respect. This holds also for other patches in this series. Do you want me to re-format all of them? Martin -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
On Fri, Nov 26, 2021 at 03:34:59PM +0100, Martin Wilck wrote: > On Wed, 2021-11-24 at 18:38 -0600, Benjamin Marzinski wrote: > > On Thu, Nov 18, 2021 at 11:58:21PM +0100, mwilck@suse.com wrote: > > > From: Martin Wilck <mwilck@suse.com> > > > > > > This patch sets up the bulk of the state machine. > > > client_state_machine() > > > is called in a loop, proceeding from state to state until it needs > > > to poll for input or wait for a lock, in which case it returns > > > STM_BREAK. > > > > > > 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". > > > > > > > Actually, one nitpick. See below > > > > > + > > > > This switch statement is indented with 8 spaces, instead of a tab > > I'm going to fix that, but I assume you're aware that our code is far > from being consistent in this respect. This holds also for other > patches in this series. Do you want me to re-format all of them? Huh? I mean that the line doesn't start with a tab, but instead has 8 spaces. A quick grep through the source code in your queue branch only turns that up in some of the files in the tests directory and in files we've just imported # grep -l "^ " `find ./ -name "*.[ch]"` ./libmultipath/nvme/nvme-ioctl.c ./tests/pgpolicy.c ./tests/util.c ./tests/directio.c ./tests/mpathvalid.c ./tests/dmevents.c ./third-party/valgrind/drd.h ./third-party/valgrind/valgrind.h -Ben > > Martin -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
On Mon, 2021-11-29 at 10:19 -0600, Benjamin Marzinski wrote: > On Fri, Nov 26, 2021 at 03:34:59PM +0100, Martin Wilck wrote: > > > > I'm going to fix that, but I assume you're aware that our code is > > far > > from being consistent in this respect. This holds also for other > > patches in this series. Do you want me to re-format all of them? > > Huh? I mean that the line doesn't start with a tab, but instead has 8 > spaces. A quick grep through the source code in your queue branch > only > turns that up in some of the files in the tests directory and in > files > we've just imported > > # grep -l "^ " `find ./ -name "*.[ch]"` > ./libmultipath/nvme/nvme-ioctl.c > ./tests/pgpolicy.c > ./tests/util.c > ./tests/directio.c > ./tests/mpathvalid.c > ./tests/dmevents.c > ./third-party/valgrind/drd.h > ./third-party/valgrind/valgrind.h Right. It must be some change in the way emacs handles indentation, then. I'm not aware that I'm doing anything different than before. Unfortunately, there are still some space-indented lines in my v3 submission. I'll prepare a v4, and try to figure out what changed in my environment. Sorry. Martin -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c index ff9604f..87134d5 100644 --- a/multipathd/uxlsnr.c +++ b/multipathd/uxlsnr.c @@ -299,22 +299,13 @@ static int parse_cmd(struct client *c) r = get_cmdvec(c->cmd, &c->cmdvec); - if (r) { - genhelp_handler(c->cmd, r, &c->reply); - if (get_strbuf_len(&c->reply) == 0) - return EINVAL; - return 0; - } + if (r) + return -r; c->handler = find_handler_for_cmdvec(c->cmdvec); - if (!c->handler || !c->handler->fn) { - genhelp_handler(c->cmd, EINVAL, &c->reply); - if (get_strbuf_len(&c->reply) == 0) - r = EINVAL; - else - r = 0; - } + if (!c->handler || !c->handler->fn) + return -EINVAL; return r; } @@ -325,7 +316,7 @@ static int execute_handler(struct client *c, struct vectors *vecs, int timeout) struct timespec tmo; if (!c->handler) - return EINVAL; + return -EINVAL; if (clock_gettime(CLOCK_REALTIME, &tmo) == 0) { tmo.tv_sec += timeout; @@ -355,50 +346,30 @@ static int execute_handler(struct client *c, struct vectors *vecs, int timeout) return r; } -static int uxsock_trigger(struct client *c, void *trigger_data) +void default_reply(struct client *c, int r) { - struct vectors * vecs; - int r = 1; - - vecs = (struct vectors *)trigger_data; - - r = parse_cmd(c); - - if (r == 0 && c->cmdvec && VECTOR_SIZE(c->cmdvec) > 0) { - struct key *kw = VECTOR_SLOT(c->cmdvec, 0); - - if (!c->is_root && kw->code != LIST) - r = EPERM; - } - - if (r == 0 && c->handler) - r = execute_handler(c, vecs, uxsock_timeout / 1000); - - if (c->cmdvec) { - free_keys(c->cmdvec); - c->cmdvec = NULL; - } - - if (r > 0) { - switch(r) { - case ETIMEDOUT: - append_strbuf_str(&c->reply, "timeout\n"); - break; - case EPERM: - append_strbuf_str(&c->reply, - "permission deny: need to be root\n"); - break; - default: - append_strbuf_str(&c->reply, "fail\n"); - break; - } - } - else if (!r && get_strbuf_len(&c->reply) == 0) { + switch(r) { + case -EINVAL: + case -ESRCH: + case -ENOMEM: + /* return codes from get_cmdvec() */ + genhelp_handler(c->cmd, -r, &c->reply); + break; + case -EPERM: + append_strbuf_str(&c->reply, + "permission deny: need to be root\n"); + break; + case -ETIMEDOUT: + append_strbuf_str(&c->reply, "timeout\n"); + break; + case 0: append_strbuf_str(&c->reply, "ok\n"); - r = 0; + break; + default: + /* cli_handler functions return 1 on unspecified error */ + append_strbuf_str(&c->reply, "fail\n"); + break; } - /* else if (r < 0) leave *reply alone */ - return r; } static void set_client_state(struct client *c, int state) @@ -409,6 +380,7 @@ static void set_client_state(struct client *c, int state) reset_strbuf(&c->reply); memset(c->cmd, '\0', sizeof(c->cmd)); c->expires = ts_zero; + c->error = 0; /* fallthrough */ case CLT_SEND: /* reuse these fields for next data transfer */ @@ -420,11 +392,20 @@ static void set_client_state(struct client *c, int state) c->state = state; } -static void handle_client(struct client *c, void *trigger_data) +enum { + STM_CONT, + STM_BREAK, +}; + +static int client_state_machine(struct client *c, struct vectors *vecs) { ssize_t n; + const char *buf; - switch (c->state) { + condlog(4, "%s: cli[%d] state=%d cmd=\"%s\" repl \"%s\"", __func__, + c->fd, c->state, c->cmd, get_strbuf_str(&c->reply)); + + switch (c->state) { case CLT_RECV: if (c->cmd_len == 0) { /* @@ -449,31 +430,59 @@ static void handle_client(struct client *c, void *trigger_data) condlog(4, "%s: cli[%d]: connected", __func__, c->fd); } /* poll for data */ - return; + return STM_BREAK; } else if (c->len < c->cmd_len) { n = recv(c->fd, c->cmd + c->len, c->cmd_len - c->len, 0); if (n <= 0 && errno != EINTR && errno != EAGAIN) { condlog(1, "%s: cli[%d]: error in recv: %m", __func__, c->fd); c->error = -ECONNRESET; - return; + return STM_BREAK; } c->len += n; if (c->len < c->cmd_len) /* continue polling */ - return; - set_client_state(c, CLT_PARSE); + return STM_BREAK; } - break; - default: - break; - } + condlog(4, "cli[%d]: Got request [%s]", c->fd, c->cmd); + set_client_state(c, CLT_PARSE); + return STM_CONT; - condlog(4, "cli[%d]: Got request [%s]", c->fd, c->cmd); - uxsock_trigger(c, trigger_data); + case CLT_PARSE: + c->error = parse_cmd(c); + if (!c->error) { + /* Permission check */ + struct key *kw = VECTOR_SLOT(c->cmdvec, 0); - if (get_strbuf_len(&c->reply) > 0) { - const char *buf = get_strbuf_str(&c->reply); + if (!c->is_root && kw->code != LIST) { + c->error = -EPERM; + condlog(0, "%s: cli[%d]: unauthorized cmd \"%s\"", + __func__, c->fd, c->cmd); + } + } + if (c->error) + set_client_state(c, CLT_SEND); + else + set_client_state(c, CLT_WORK); + return STM_CONT; + + case CLT_WAIT_LOCK: + /* tbd */ + set_client_state(c, CLT_WORK); + return STM_CONT; + + case CLT_WORK: + c->error = execute_handler(c, vecs, uxsock_timeout / 1000); + free_keys(c->cmdvec); + c->cmdvec = NULL; + set_client_state(c, CLT_SEND); + return STM_CONT; + + case CLT_SEND: + if (get_strbuf_len(&c->reply) == 0) + default_reply(c, c->error); + + buf = get_strbuf_str(&c->reply); if (send_packet(c->fd, buf) != 0) dead_client(c); @@ -481,9 +490,18 @@ static void handle_client(struct client *c, void *trigger_data) condlog(4, "cli[%d]: Reply [%zu bytes]", c->fd, get_strbuf_len(&c->reply) + 1); reset_strbuf(&c->reply); - } - set_client_state(c, CLT_RECV); + set_client_state(c, CLT_RECV); + return STM_BREAK; + + default: + return STM_BREAK; + } +} + +static void handle_client(struct client *c, struct vectors *vecs) +{ + while (client_state_machine(c, vecs) == STM_CONT); } /*