On Tue, 2023-12-12 at 18:53 -0500, Benjamin Marzinski wrote:
> When multipathd interactive commands fail for certain reasons, like
> timing out or incorrect permissions, they do not reply with "fail\n".
> Currently, multipath and multipathc expect that a reply other than
> "fail\n" means success. Instead, prefix all failure replies with
> "fail\n", and adapt multipath and multipathc to return failure for
> any
> reply starting with "fail\n".
>
> Signed-off-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx>
> ---
> libdmmp/libdmmp.c | 6 +++---
> multipath/main.c | 2 +-
> multipathd/uxclnt.c | 2 +-
> multipathd/uxlsnr.c | 12 +++++-------
> 4 files changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/libdmmp/libdmmp.c b/libdmmp/libdmmp.c
> index 0025e66d..77f8a6a0 100644
> --- a/libdmmp/libdmmp.c
> +++ b/libdmmp/libdmmp.c
> @@ -321,7 +321,7 @@ invoke:
> }
> }
> if ((*output != NULL) &&
> - (strncmp(*output, "timeout", strlen("timeout")) == 0))
> + (strncmp(*output, "fail\ntimeout",
> strlen("fail\ntimeout")) == 0))
> flag_check_tmo = true;
>
> if (flag_check_tmo == true) {
> @@ -364,8 +364,8 @@ invoke:
> }
>
> if ((*output != NULL) &&
> - strncmp(*output, "permission deny",
> - strlen("permission deny")) == 0) {
> + strncmp(*output, "fail\npermission deny",
> + strlen("fail\npermission deny")) == 0) {
> _error(ctx, "Permission deny, need to be root");
> rc = DMMP_ERR_PERMISSION_DENY;
> goto out;
> diff --git a/multipath/main.c b/multipath/main.c
> index c37befa5..4b399e21 100644
> --- a/multipath/main.c
> +++ b/multipath/main.c
> @@ -804,7 +804,7 @@ int delegate_to_multipathd(enum mpath_cmds cmd,
> }
>
> if (reply != NULL && *reply != '\0') {
> - if (strcmp(reply, "fail\n"))
> + if (strncmp(reply, "fail\n", 5))
> r = DELEGATE_OK;
> if (r != NOT_DELEGATED && strcmp(reply, "ok\n"))
> printf("%s", reply);
Perhaps here we should check if the reply length is > 5, and if yes,
strip the "fail\n" part. That would make the output consistent with
what it used to be.
> diff --git a/multipathd/uxclnt.c b/multipathd/uxclnt.c
> index c3ae5db6..2401199d 100644
> --- a/multipathd/uxclnt.c
> +++ b/multipathd/uxclnt.c
> @@ -31,7 +31,7 @@ static int process_req(int fd, char * inbuf,
> unsigned int timeout)
> return 1;
> } else {
> printf("%s", reply);
Same here.
> - ret = (strcmp(reply, "fail\n") == 0);
> + ret = (strncmp(reply, "fail\n", 5) == 0);
> free(reply);
> return ret;
> }
> diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c
> index 4d6f258c..4df01666 100644
> --- a/multipathd/uxlsnr.c
> +++ b/multipathd/uxlsnr.c
> @@ -392,6 +392,11 @@ static void drain_idle_fd(int fd)
>
> void default_reply(struct client *c, int r)
> {
> + if (r == 0) {
> + append_strbuf_str(&c->reply, "ok\n");
> + return;
> + }
> + append_strbuf_str(&c->reply, "fail\n");
> switch(r) {
> case -EINVAL:
> case -ESRCH:
> @@ -406,13 +411,6 @@ void default_reply(struct client *c, int r)
> case -ETIMEDOUT:
> append_strbuf_str(&c->reply, "timeout\n");
> break;
> - case 0:
> - append_strbuf_str(&c->reply, "ok\n");
> - break;
> - default:
> - /* cli_handler functions return 1 on unspecified
> error */
> - append_strbuf_str(&c->reply, "fail\n");
> - break;
> }
> }
>
