[Sheepdog] [PATCH, RFC] collie: use exit codes to distinguish between errors
MORITA Kazutaka
morita.kazutaka at lab.ntt.co.jp
Thu Jun 16 17:48:46 CEST 2011
At Thu, 16 Jun 2011 12:46:12 +0100,
Chris Webb wrote:
>
> When integrating collie into cluster management scripts, it is useful to
> be able to tell the difference between different types of error without
> needing to parse human-readable error text. In addition to the standard
> EXIT_SUCCESS (0) and EXIT_FAILURE (1) exit conditions, we introduce
>
> EXIT_SYSFAIL (2) - something is wrong with the cluster or local host
> EXIT_BUSY (3) - an object is alredy in use, blocking the operation
> EXIT_FULL (4) - no more space is left in the cluster
> EXIT_MISSING (5) - the specified object is not found
> EXIT_USAGE (64) - standard EUSAGE error exit code
>
> and attempt to return these consistently for all collie commands.
Nice work, thank you!
I have some comments below.
>
> Signed-off-by: Chris Webb <chris at arachsys.com>
>
> ---
> collie/collie.c | 94 ++++++++++++++++++++++++++++--------------------------
> include/exits.h | 12 +++++++
> 2 files changed, 61 insertions(+), 45 deletions(-)
> create mode 100644 include/exits.h
>
> diff --git a/collie/collie.c b/collie/collie.c
> index a0ac7f6..44eacd3 100644
> --- a/collie/collie.c
> +++ b/collie/collie.c
> @@ -29,6 +29,7 @@
> #include "sheep.h"
> #include "net.h"
> #include "treeview.h"
> +#include "exits.h"
>
> static char program_name[] = "collie";
> static const char *sdhost = "localhost";
> @@ -183,7 +184,7 @@ static int cluster_format(int argc, char **argv)
>
> fd = connect_to(sdhost, sdport);
> if (fd < 0)
> - return -1;
> + return EXIT_SYSFAIL;
>
> gettimeofday(&tv, NULL);
>
> @@ -201,15 +202,15 @@ static int cluster_format(int argc, char **argv)
>
> if (ret) {
> fprintf(stderr, "failed to connect\n");
> - return ret;
> + return EXIT_SYSFAIL;
> }
>
> if (rsp->result != SD_RES_SUCCESS) {
> fprintf(stderr, "%s\n", sd_strerror(rsp->result));
> - return 1;
> + return EXIT_FAILURE;
> }
>
> - return 0;
> + return EXIT_SUCCESS;
> }
>
> static int shutdown_sheepdog(void)
> @@ -235,15 +236,15 @@ static int shutdown_sheepdog(void)
>
> if (ret) {
> fprintf(stderr, "failed to connect\n");
> - return ret;
> + return EXIT_SYSFAIL;
> }
>
> if (rsp->result != SD_RES_SUCCESS) {
> fprintf(stderr, "%s\n", sd_strerror(rsp->result));
> - return 1;
> + return EXIT_FAILURE;
> }
>
> - return 0;
> + return EXIT_SUCCESS;
> }
>
> typedef void (*vdi_parser_func_t)(uint32_t vid, char *name, char *tag,
> @@ -615,7 +616,7 @@ static int node_list(int argc, char **argv)
> printf(" %4d - %-20s\t%d\n", i, data, node_list_entries[i].nr_vnodes);
> }
>
> - return 0;
> + return EXIT_SUCCESS;
> }
>
> static int node_info(int argc, char **argv)
> @@ -667,7 +668,7 @@ static int node_info(int argc, char **argv)
>
> if (success == 0) {
> fprintf(stderr, "cannot get information from any nodes\n");
> - return 1;
> + return EXIT_SYSFAIL;
> }
>
> parse_vdi(cal_total_vdi_size, SD_INODE_HEADER_SIZE, &total_vdi_size);
> @@ -680,7 +681,7 @@ static int node_info(int argc, char **argv)
> (int)(((double)(total_size - total_avail) / total_size) * 100),
> vdi_size_str);
>
> - return 0;
> + return EXIT_SUCCESS;
> }
>
> static struct subcommand node_cmd[] = {
> @@ -695,7 +696,7 @@ static int vdi_list(int argc, char **argv)
> printf("------------------------------------------------------------------\n");
>
> parse_vdi(print_vdi_list, SD_INODE_SIZE, NULL);
> - return 0;
> + return EXIT_SUCCESS;
> }
>
> static int vdi_tree(int argc, char **argv)
> @@ -704,7 +705,7 @@ static int vdi_tree(int argc, char **argv)
> parse_vdi(print_vdi_tree, SD_INODE_HEADER_SIZE, NULL);
> dump_tree();
>
> - return 0;
> + return EXIT_SUCCESS;
> }
>
> static int vdi_graph(int argc, char **argv)
> @@ -719,7 +720,7 @@ static int vdi_graph(int argc, char **argv)
> /* print a footer */
> printf("}\n");
>
> - return 0;
> + return EXIT_SUCCESS;
> }
>
> static int vdi_delete(int argc, char **argv)
> @@ -733,7 +734,7 @@ static int vdi_delete(int argc, char **argv)
>
> fd = connect_to(sdhost, sdport);
> if (fd < 0)
> - return -1;
> + return EXIT_SYSFAIL;
>
> memset(&hdr, 0, sizeof(hdr));
>
> @@ -755,15 +756,18 @@ static int vdi_delete(int argc, char **argv)
>
> if (ret) {
> fprintf(stderr, "failed to connect\n");
> - return ret;
> + return EXIT_SYSFAIL;
> }
>
> if (rsp->result != SD_RES_SUCCESS) {
> fprintf(stderr, "%s: %s\n", vdiname, sd_strerror(rsp->result));
> - return 1;
> + if (rsp->result == SD_RES_NO_VDI)
> + return EXIT_MISSING;
> + else
> + return EXIT_FAILURE;
> }
>
> - return 0;
> + return EXIT_SUCCESS;
> }
>
> static int vdi_object(int argc, char **argv)
> @@ -785,7 +789,7 @@ static int vdi_object(int argc, char **argv)
> vid = info.vid;
> if (vid == 0) {
> printf("No such vdi\n");
> - return 1;
> + return EXIT_MISSING;
> }
>
> if (idx == ~0) {
> @@ -800,7 +804,7 @@ static int vdi_object(int argc, char **argv)
>
> if (idx >= MAX_DATA_OBJS) {
> printf("The offset is too large!\n");
> - exit(1);
> + exit(EXIT_FAILURE);
> }
>
> parse_objs(vid_to_vdi_oid(vid), get_data_oid, &old_info);
> @@ -819,7 +823,7 @@ static int vdi_object(int argc, char **argv)
> printf("failed to read the inode object 0x%" PRIx32 "\n", vid);
> }
>
> - return 0;
> + return EXIT_SUCCESS;
> }
>
> static int find_vdi_attr_oid(char *vdiname, char *tag, uint32_t snapid,
> @@ -896,7 +900,7 @@ static int vdi_setattr(int argc, char **argv)
> key = argv[optind++];
> if (!key) {
> fprintf(stderr, "please specify the name of key\n");
> - return 1;
> + return EXIT_USAGE;
> }
>
> value = argv[optind++];
> @@ -904,7 +908,7 @@ static int vdi_setattr(int argc, char **argv)
> value = malloc(SD_MAX_VDI_ATTR_VALUE_LEN);
> if (!value) {
> fprintf(stderr, "failed to allocate memory\n");
> - return 1;
> + return EXIT_SYSFAIL;
> }
>
> offset = 0;
> @@ -913,7 +917,7 @@ reread:
> SD_MAX_VDI_ATTR_VALUE_LEN - offset);
> if (ret < 0) {
> fprintf(stderr, "failed to read from stdin, %m\n");
> - return 1;
> + return EXIT_FAILURE;
> }
> if (ret > 0) {
> offset += ret;
> @@ -928,12 +932,13 @@ reread:
> if (ret) {
> if (ret == SD_RES_VDI_EXIST) {
> fprintf(stderr, "the attribute already exists, %s\n", key);
> + return EXIT_BUSY;
Is EXIT_BUSY suitable for this? We can use vdi attributes for other
purposes than locking vdi, so I think it is better to create other
status like EXIT_EXIST.
> } else if (ret == SD_RES_NO_OBJ) {
> fprintf(stderr, "no such attribute, %s\n", key);
> } else
> fprintf(stderr, "failed to find attr oid, %s\n",
> sd_strerror(ret));
> - return 1;
> + return EXIT_MISSING;
Should we return EXIT_FAILURE if (ret != SD_RES_NO_OBJ)?
> }
>
> oid = attr_oid;
> @@ -976,16 +981,16 @@ reread:
>
> if (ret) {
> fprintf(stderr, "failed to set attribute\n");
> - return 1;
> + return EXIT_FAILURE;
Should be EXIT_SYSFAIL?
> }
> if (rsp->result != SD_RES_SUCCESS) {
> fprintf(stderr, "failed to set attribute, %s\n",
> sd_strerror(rsp->result));
> - return 1;
> + return EXIT_FAILURE;
> }
> }
>
> - return 0;
> + return EXIT_SUCCESS;
> }
>
> static int vdi_getattr(int argc, char **argv)
> @@ -1002,7 +1007,7 @@ static int vdi_getattr(int argc, char **argv)
> key = argv[optind++];
> if (!key) {
> fprintf(stderr, "please specify the name of key\n");
> - return 1;
> + return EXIT_USAGE;
> }
>
> ret = find_vdi_attr_oid(vdiname, vdi_cmd_data.snapshot_tag,
> @@ -1010,18 +1015,18 @@ static int vdi_getattr(int argc, char **argv)
> &nr_copies, 0, 0);
> if (ret == SD_RES_NO_OBJ) {
> fprintf(stderr, "no such attribute, %s\n", key);
> - return 1;
> + return EXIT_MISSING;
> } else if (ret) {
> fprintf(stderr, "failed to find attr oid, %s\n",
> sd_strerror(ret));
> - return 1;
> + return EXIT_MISSING;
> }
>
> oid = attr_oid;
> value = malloc(SD_MAX_VDI_ATTR_VALUE_LEN);
> if (!value) {
> fprintf(stderr, "failed to allocate memory\n");
> - return 1;
> + return EXIT_SYSFAIL;
> }
> for (i = 0; i < nr_copies; i++) {
> rlen = SD_MAX_VDI_ATTR_VALUE_LEN;
> @@ -1054,13 +1059,13 @@ static int vdi_getattr(int argc, char **argv)
> if (rsp->result == SD_RES_SUCCESS) {
> printf("%s", value);
> free(value);
> - return 0;
> + return EXIT_SUCCESS;
> }
> }
> }
> out:
> free(value);
> - return 1;
> + return EXIT_FAILURE;
> }
>
> static struct subcommand vdi_cmd[] = {
> @@ -1121,7 +1126,7 @@ static int cluster_info(int argc, char **argv)
>
> fd = connect_to(sdhost, sdport);
> if (fd < 0)
> - return 1;
> + return EXIT_SYSFAIL;
>
> memset(&hdr, 0, sizeof(hdr));
>
> @@ -1135,7 +1140,7 @@ static int cluster_info(int argc, char **argv)
> close(fd);
>
> if (ret != 0)
> - return 1;
> + return EXIT_SYSFAIL;
>
> if (rsp->result == SD_RES_SUCCESS)
> printf("running\n");
> @@ -1166,7 +1171,7 @@ static int cluster_info(int argc, char **argv)
> printf("]\n");
> }
>
> - return 0;
> + return EXIT_SUCCESS;
> }
>
> static int cluster_parser(int ch, char *opt)
> @@ -1182,8 +1187,7 @@ static int cluster_parser(int ch, char *opt)
>
> static int cluster_shutdown(int argc, char **argv)
> {
> - shutdown_sheepdog();
> - return 0;
> + return shutdown_sheepdog();
If we use the return value of shutdown_sheepdog() here,
shutdown_sheepdog() should return EXIT_* in all cases.
> }
>
> static struct subcommand cluster_cmd[] = {
> @@ -1251,7 +1255,7 @@ static unsigned long setup_command(char *cmd, char *subcmd)
>
> if (!found) {
> fprintf(stderr, "'%s' is not a valid command\n", cmd);
> - usage(1);
> + usage(EXIT_USAGE);
> }
>
> for (s = commands[i].sub; s->name; s++) {
> @@ -1264,10 +1268,10 @@ static unsigned long setup_command(char *cmd, char *subcmd)
>
> if (!command_fn) {
> fprintf(stderr, "'%s' is not a valid subcommand\n", subcmd);
> - fprintf(stderr, "'%s' supports the following subcommand:\n", cmd);
> + fprintf(stderr, "'%s' supports the following subcommands:\n", cmd);
> for (s = commands[i].sub; s->name; s++)
> fprintf(stderr, "%s\n", s->name);
> - exit(1);
> + exit(EXIT_USAGE);
> }
>
> return flags;
> @@ -1300,13 +1304,13 @@ int main(int argc, char **argv)
> command_help();
> break;
> case '?':
> - usage(1);
> + usage(EXIT_USAGE);
> break;
> default:
> if (command_parser)
> command_parser(ch, optarg);
> else
> - usage(1);
> + usage(EXIT_USAGE);
> break;
> }
> }
> @@ -1318,13 +1322,13 @@ int main(int argc, char **argv)
> ret = update_node_list(SD_MAX_NODES, 0);
> if (ret < 0) {
> fprintf(stderr, "failed to get node list\n");
> - exit(1);
> + exit(EXIT_SYSFAIL);
> }
> }
>
> if (flags & SUBCMD_FLAG_NEED_THIRD_ARG && argc == optind) {
> fprintf(stderr, "'%s %s' needs the third argument\n", argv[1], argv[2]);
> - exit(1);
> + exit(EXIT_USAGE);
> }
>
> return command_fn(argc, argv);
> diff --git a/include/exits.h b/include/exits.h
> new file mode 100644
> index 0000000..2ad8327
> --- /dev/null
> +++ b/include/exits.h
> @@ -0,0 +1,12 @@
> +#ifndef __EXITS_H__
> +#define __EXITS_H__
> +
> +#define EXIT_SUCCESS 0
> +#define EXIT_FAILURE 1
> +#define EXIT_SYSFAIL 2
> +#define EXIT_BUSY 3
> +#define EXIT_FULL 4
> +#define EXIT_MISSING 5
> +#define EXIT_USAGE 64
> +
> +#endif
It would be nice if a description is added for each status. :)
Thanks,
Kazutaka
More information about the sheepdog
mailing list