Date: Tue, 9 Jul 2013 21:14:16 +0000 (UTC) From: Jim Harris <jimharris@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r253109 - head/sbin/nvmecontrol Message-ID: <201307092114.r69LEGhr044445@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: jimharris Date: Tue Jul 9 21:14:15 2013 New Revision: 253109 URL: http://svnweb.freebsd.org/changeset/base/253109 Log: Incorporate feedback from bde@ based on r252672 changes: * Use 0/1 instead of sysexits. Man pages are confusing on this topic, but 0/1 is sufficient for nvmecontrol. * Use err function family where possible instead of fprintf/exit. * Fix some typing errors. * Clean up some error message inconsistencies. Sponsored by: Intel Submitted by: bde (parts of firmware.c changes) MFC after: 3 days Modified: head/sbin/nvmecontrol/devlist.c head/sbin/nvmecontrol/firmware.c head/sbin/nvmecontrol/identify.c head/sbin/nvmecontrol/logpage.c head/sbin/nvmecontrol/nvmecontrol.c head/sbin/nvmecontrol/perftest.c head/sbin/nvmecontrol/reset.c Modified: head/sbin/nvmecontrol/devlist.c ============================================================================== --- head/sbin/nvmecontrol/devlist.c Tue Jul 9 21:03:39 2013 (r253108) +++ head/sbin/nvmecontrol/devlist.c Tue Jul 9 21:14:15 2013 (r253109) @@ -29,13 +29,12 @@ __FBSDID("$FreeBSD$"); #include <sys/param.h> -#include <errno.h> +#include <err.h> #include <fcntl.h> #include <stddef.h> #include <stdio.h> #include <stdlib.h> #include <string.h> -#include <sysexits.h> #include <unistd.h> #include "nvmecontrol.h" @@ -45,7 +44,7 @@ devlist_usage(void) { fprintf(stderr, "usage:\n"); fprintf(stderr, DEVLIST_USAGE); - exit(EX_USAGE); + exit(1); } static inline uint32_t @@ -62,9 +61,7 @@ devlist(int argc, char *argv[]) struct nvme_namespace_data nsdata; char name[64]; uint32_t i; - int ch, ctrlr, exit_code, fd, found; - - exit_code = EX_OK; + int ch, ctrlr, fd, found, ret; while ((ch = getopt(argc, argv, "")) != -1) { switch ((char)ch) { @@ -80,14 +77,14 @@ devlist(int argc, char *argv[]) ctrlr++; sprintf(name, "%s%d", NVME_CTRLR_PREFIX, ctrlr); - exit_code = open_dev(name, &fd, 0, 0); + ret = open_dev(name, &fd, 0, 0); - if (exit_code == EX_NOINPUT) - break; - else if (exit_code == EX_NOPERM) { - printf("Could not open /dev/%s, errno = %d (%s)\n", - name, errno, strerror(errno)); - continue; + if (ret != 0) { + if (fd < 0) { + warnx("could not open /dev/%s\n", name); + continue; + } else + break; } found++; @@ -111,5 +108,5 @@ devlist(int argc, char *argv[]) if (found == 0) printf("No NVMe controllers found.\n"); - exit(EX_OK); + exit(1); } Modified: head/sbin/nvmecontrol/firmware.c ============================================================================== --- head/sbin/nvmecontrol/firmware.c Tue Jul 9 21:03:39 2013 (r253108) +++ head/sbin/nvmecontrol/firmware.c Tue Jul 9 21:14:15 2013 (r253109) @@ -36,14 +36,14 @@ __FBSDID("$FreeBSD$"); #include <sys/types.h> #include <ctype.h> -#include <errno.h> +#include <err.h> #include <fcntl.h> +#include <inttypes.h> #include <stdbool.h> #include <stddef.h> #include <stdio.h> #include <stdlib.h> #include <string.h> -#include <sysexits.h> #include <unistd.h> #include "nvmecontrol.h" @@ -64,60 +64,58 @@ slot_has_valid_firmware(int fd, int slot } static void -read_image_file(char *path, void **buf, ssize_t *size) +read_image_file(char *path, void **buf, int32_t *size) { struct stat sb; + int32_t filesize; int fd; *size = 0; *buf = NULL; - if ((fd = open(path, O_RDONLY)) < 0) { - fprintf(stderr, "Unable to open '%s'.\n", path); - exit(EX_IOERR); - } - if (fstat(fd, &sb) < 0) { - fprintf(stderr, "Unable to stat '%s'.\n", path); - close(fd); - exit(EX_IOERR); - } - if ((*buf = malloc(sb.st_size)) == NULL) { - fprintf(stderr, "Unable to malloc %jd bytes.\n", - sb.st_size); - close(fd); - exit(EX_IOERR); - } - if ((*size = read(fd, *buf, sb.st_size)) < 0) { - fprintf(stderr, "Error reading '%s', errno=%d (%s)\n", - path, errno, strerror(errno)); - close(fd); - exit(EX_IOERR); - } - if (*size != sb.st_size) { - fprintf(stderr, "Error reading '%s', " - "read %zd bytes, requested %jd bytes\n", - path, *size, sb.st_size); - close(fd); - exit(EX_IOERR); - } + if ((fd = open(path, O_RDONLY)) < 0) + err(1, "unable to open '%s'", path); + if (fstat(fd, &sb) < 0) + err(1, "unable to stat '%s'", path); + + /* + * The NVMe spec does not explicitly state a maximum firmware image + * size, although one can be inferred from the dword size limitation + * for the size and offset fields in the Firmware Image Download + * command. + * + * Technically, the max is UINT32_MAX * sizeof(uint32_t), since the + * size and offsets are specified in terms of dwords (not bytes), but + * realistically INT32_MAX is sufficient here and simplifies matters + * a bit. + */ + if (sb.st_size > INT32_MAX) + errx(1, "size of file '%s' is too large (%jd bytes)", + path, (intmax_t)sb.st_size); + filesize = (int32_t)sb.st_size; + if ((*buf = malloc(filesize)) == NULL) + errx(1, "unable to malloc %zd bytes", filesize); + if ((*size = read(fd, *buf, filesize)) < 0) + err(1, "error reading '%s'", path); + /* XXX assuming no short reads */ + if (*size != filesize) + errx(1, + "error reading '%s' (read %d bytes, requested %d bytes)", + path, *size, filesize); } static void -update_firmware(int fd, uint8_t *payload, uint32_t payload_size) +update_firmware(int fd, uint8_t *payload, int32_t payload_size) { struct nvme_pt_command pt; - size_t size; + int32_t off, resid, size; void *chunk; - uint32_t off, resid; - int exit_code = EX_OK; off = 0; resid = payload_size; - if ((chunk = malloc((size_t)NVME_MAX_XFER_SIZE)) == NULL) { - printf("Unable to malloc %d bytes.\n", NVME_MAX_XFER_SIZE); - exit(EX_IOERR); - } + if ((chunk = malloc(NVME_MAX_XFER_SIZE)) == NULL) + errx(1, "unable to malloc %d bytes", NVME_MAX_XFER_SIZE); while (resid > 0) { size = (resid >= NVME_MAX_XFER_SIZE) ? @@ -132,26 +130,15 @@ update_firmware(int fd, uint8_t *payload pt.len = size; pt.is_read = 0; - if (ioctl(fd, NVME_PASSTHROUGH_CMD, &pt) < 0) { - printf("Firmware image download request failed. " - "errno=%d (%s)\n", - errno, strerror(errno)); - exit_code = EX_IOERR; - break; - } + if (ioctl(fd, NVME_PASSTHROUGH_CMD, &pt) < 0) + err(1, "firmware download request failed"); - if (nvme_completion_is_error(&pt.cpl)) { - printf("Passthrough command returned error.\n"); - exit_code = EX_IOERR; - break; - } + if (nvme_completion_is_error(&pt.cpl)) + errx(1, "firmware download request returned error"); resid -= size; off += size; } - - if (exit_code != EX_OK) - exit(exit_code); } static void @@ -164,16 +151,11 @@ activate_firmware(int fd, int slot, int pt.cmd.cdw10 = (activate_action << 3) | slot; pt.is_read = 0; - if (ioctl(fd, NVME_PASSTHROUGH_CMD, &pt) < 0) { - printf("Firmware activate request failed. errno=%d (%s)\n", - errno, strerror(errno)); - exit(EX_IOERR); - } + if (ioctl(fd, NVME_PASSTHROUGH_CMD, &pt) < 0) + err(1, "firmware activate request failed"); - if (nvme_completion_is_error(&pt.cpl)) { - printf("Passthrough command returned error.\n"); - exit(EX_IOERR); - } + if (nvme_completion_is_error(&pt.cpl)) + errx(1, "firmware activate request returned error"); } static void @@ -181,7 +163,7 @@ firmware_usage(void) { fprintf(stderr, "usage:\n"); fprintf(stderr, FIRMWARE_USAGE); - exit(EX_USAGE); + exit(1); } void @@ -192,7 +174,7 @@ firmware(int argc, char *argv[]) char ch, *p, *image = NULL; char *controller = NULL, prompt[64]; void *buf = NULL; - ssize_t size; + int32_t size = 0; struct nvme_controller_data cdata; a_flag = s_flag = f_flag = false; @@ -252,34 +234,24 @@ firmware(int argc, char *argv[]) open_dev(controller, &fd, 1, 1); read_controller_data(fd, &cdata); - if (cdata.oacs.firmware == 0) { - fprintf(stderr, - "Controller does not support firmware " - "activate/download.\n"); - exit(EX_IOERR); - } - - if (f_flag && slot == 1 && cdata.frmw.slot1_ro) { - fprintf(stderr, "Slot %d is marked as read only.\n", slot); - exit(EX_IOERR); - } - - if (slot > cdata.frmw.num_slots) { - fprintf(stderr, - "Slot %d was specified but controller only " - "supports %d firmware slots.\n", + if (cdata.oacs.firmware == 0) + errx(1, + "controller does not support firmware activate/download"); + + if (f_flag && slot == 1 && cdata.frmw.slot1_ro) + errx(1, "slot %d is marked as read only", slot); + + if (slot > cdata.frmw.num_slots) + errx(1, + "slot %d specified but controller only supports %d slots", slot, cdata.frmw.num_slots); - exit(EX_IOERR); - } - if (!slot_has_valid_firmware(fd, slot)) { - fprintf(stderr, - "Slot %d does not contain valid firmware.\n" - "Try 'nvmecontrol logpage -p 3 %s' to get a list " - "of available firmware images.\n", + if (!slot_has_valid_firmware(fd, slot)) + errx(1, + "slot %d does not contain valid firmware,\n" + "try 'nvmecontrol logpage -p 3 %s' to get a list " + "of available images\n", slot, controller); - exit(EX_IOERR); - } if (f_flag && a_flag) printf("You are about to download and activate " @@ -305,7 +277,7 @@ firmware(int argc, char *argv[]) if (strncasecmp(prompt, "yes", 3) == 0) break; if (strncasecmp(prompt, "no", 2) == 0) - exit(EX_OK); + exit(1); printf("Please answer \"yes\" or \"no\". "); } @@ -331,5 +303,5 @@ firmware(int argc, char *argv[]) } close(fd); - exit(EX_OK); + exit(0); } Modified: head/sbin/nvmecontrol/identify.c ============================================================================== --- head/sbin/nvmecontrol/identify.c Tue Jul 9 21:03:39 2013 (r253108) +++ head/sbin/nvmecontrol/identify.c Tue Jul 9 21:14:15 2013 (r253109) @@ -30,13 +30,13 @@ __FBSDID("$FreeBSD$"); #include <sys/param.h> #include <ctype.h> +#include <err.h> #include <errno.h> #include <fcntl.h> #include <stddef.h> #include <stdio.h> #include <stdlib.h> #include <string.h> -#include <sysexits.h> #include <unistd.h> #include "nvmecontrol.h" @@ -139,7 +139,7 @@ identify_usage(void) { fprintf(stderr, "usage:\n"); fprintf(stderr, IDENTIFY_USAGE); - exit(EX_USAGE); + exit(1); } static void @@ -177,16 +177,16 @@ identify_ctrlr(int argc, char *argv[]) hexlength = offsetof(struct nvme_controller_data, reserved5); print_hex(&cdata, hexlength); - exit(EX_OK); + exit(0); } if (verboseflag == 1) { - printf("-v not currently supported without -x.\n"); + fprintf(stderr, "-v not currently supported without -x\n"); identify_usage(); } print_controller(&cdata); - exit(EX_OK); + exit(0); } static void @@ -231,10 +231,8 @@ identify_ns(int argc, char *argv[]) nsloc = strnstr(argv[optind], NVME_NS_PREFIX, 10); if (nsloc != NULL) nsid = strtol(nsloc + 2, NULL, 10); - if (nsloc == NULL || (nsid == 0 && errno != 0)) { - printf("Invalid namespace ID %s.\n", argv[optind]); - exit(EX_IOERR); - } + if (nsloc == NULL || (nsid == 0 && errno != 0)) + errx(1, "invalid namespace ID '%s'", argv[optind]); /* * We send IDENTIFY commands to the controller, not the namespace, @@ -253,16 +251,16 @@ identify_ns(int argc, char *argv[]) hexlength = offsetof(struct nvme_namespace_data, reserved6); print_hex(&nsdata, hexlength); - exit(EX_OK); + exit(0); } if (verboseflag == 1) { - printf("-v not currently supported without -x.\n"); + fprintf(stderr, "-v not currently supported without -x\n"); identify_usage(); } print_namespace(&nsdata); - exit(EX_OK); + exit(0); } void Modified: head/sbin/nvmecontrol/logpage.c ============================================================================== --- head/sbin/nvmecontrol/logpage.c Tue Jul 9 21:03:39 2013 (r253108) +++ head/sbin/nvmecontrol/logpage.c Tue Jul 9 21:14:15 2013 (r253109) @@ -34,6 +34,7 @@ __FBSDID("$FreeBSD$"); #include <sys/ioccom.h> #include <ctype.h> +#include <err.h> #include <errno.h> #include <fcntl.h> #include <stdbool.h> @@ -41,7 +42,6 @@ __FBSDID("$FreeBSD$"); #include <stdio.h> #include <stdlib.h> #include <string.h> -#include <sysexits.h> #include <unistd.h> #include "nvmecontrol.h" @@ -52,14 +52,13 @@ __FBSDID("$FreeBSD$"); typedef void (*print_fn_t)(void *buf, uint32_t size); static void * -get_log_buffer(size_t size) +get_log_buffer(uint32_t size) { void *buf; - if ((buf = malloc(size)) == NULL) { - fprintf(stderr, "Unable to malloc %zd bytes\n", size); - exit(EX_IOERR); - } + if ((buf = malloc(size)) == NULL) + errx(1, "unable to malloc %u bytes", size); + memset(buf, 0, size); return (buf); } @@ -79,16 +78,11 @@ read_logpage(int fd, uint8_t log_page, i pt.len = payload_size; pt.is_read = 1; - if (ioctl(fd, NVME_PASSTHROUGH_CMD, &pt) < 0) { - printf("Get log page request failed. errno=%d (%s)\n", - errno, strerror(errno)); - exit(EX_IOERR); - } + if (ioctl(fd, NVME_PASSTHROUGH_CMD, &pt) < 0) + err(1, "get log page request failed"); - if (nvme_completion_is_error(&pt.cpl)) { - printf("Passthrough command returned error.\n"); - exit(EX_IOERR); - } + if (nvme_completion_is_error(&pt.cpl)) + errx(1, "get log page request returned error"); } static void @@ -242,7 +236,7 @@ logpage_usage(void) { fprintf(stderr, "usage:\n"); fprintf(stderr, LOGPAGE_USAGE); - exit(EX_USAGE); + exit(1); } void @@ -254,7 +248,7 @@ logpage(int argc, char *argv[]) int allow_ns = false; char ch, *p, *nsloc = NULL; char *cname = NULL; - size_t size; + uint32_t size; void *buf; struct logpage_function *f; struct nvme_controller_data cdata; @@ -313,28 +307,20 @@ logpage(int argc, char *argv[]) /* If a namespace id was specified, validate it's use */ if (strstr(argv[optind], NVME_NS_PREFIX) != NULL) { if (!allow_ns) { - if (log_page != NVME_LOG_HEALTH_INFORMATION) { - fprintf(stderr, - "Namespace ID not valid for log page %d.\n", + if (log_page != NVME_LOG_HEALTH_INFORMATION) + errx(1, + "log page %d valid only at controller level", log_page); - } else if (cdata.lpa.ns_smart == 0) { - fprintf(stderr, - "Controller does not support per " - "namespace SMART/Health information.\n"); - } - close(fd); - exit(EX_IOERR); + else if (cdata.lpa.ns_smart == 0) + errx(1, + "controller does not support per " + "namespace smart/health information"); } nsloc = strnstr(argv[optind], NVME_NS_PREFIX, 10); if (nsloc != NULL) nsid = strtol(nsloc + 2, NULL, 10); - if (nsloc == NULL || (nsid == 0 && errno != 0)) { - fprintf(stderr, - "Invalid namespace ID %s.\n", - argv[optind]); - close(fd); - exit(EX_IOERR); - } + if (nsloc == NULL || (nsid == 0 && errno != 0)) + errx(1, "invalid namespace id '%s'", argv[optind]); /* * User is asking for per namespace log page information @@ -384,5 +370,5 @@ logpage(int argc, char *argv[]) print_fn(buf, size); close(fd); - exit(EX_OK); + exit(0); } Modified: head/sbin/nvmecontrol/nvmecontrol.c ============================================================================== --- head/sbin/nvmecontrol/nvmecontrol.c Tue Jul 9 21:03:39 2013 (r253108) +++ head/sbin/nvmecontrol/nvmecontrol.c Tue Jul 9 21:14:15 2013 (r253109) @@ -32,14 +32,13 @@ __FBSDID("$FreeBSD$"); #include <sys/stat.h> #include <ctype.h> -#include <errno.h> +#include <err.h> #include <fcntl.h> #include <stdbool.h> #include <stddef.h> #include <stdio.h> #include <stdlib.h> #include <string.h> -#include <sysexits.h> #include <unistd.h> #include "nvmecontrol.h" @@ -71,7 +70,7 @@ usage(void) fprintf(stderr, "%s", f->usage); f++; } - exit(EX_USAGE); + exit(1); } static void @@ -134,16 +133,11 @@ read_controller_data(int fd, struct nvme pt.len = sizeof(*cdata); pt.is_read = 1; - if (ioctl(fd, NVME_PASSTHROUGH_CMD, &pt) < 0) { - printf("Identify request failed. errno=%d (%s)\n", - errno, strerror(errno)); - exit(EX_IOERR); - } + if (ioctl(fd, NVME_PASSTHROUGH_CMD, &pt) < 0) + err(1, "identify request failed"); - if (nvme_completion_is_error(&pt.cpl)) { - printf("Passthrough command returned error.\n"); - exit(EX_IOERR); - } + if (nvme_completion_is_error(&pt.cpl)) + errx(1, "identify request returned error"); } void @@ -158,16 +152,11 @@ read_namespace_data(int fd, int nsid, st pt.len = sizeof(*nsdata); pt.is_read = 1; - if (ioctl(fd, NVME_PASSTHROUGH_CMD, &pt) < 0) { - printf("Identify request failed. errno=%d (%s)\n", - errno, strerror(errno)); - exit(EX_IOERR); - } + if (ioctl(fd, NVME_PASSTHROUGH_CMD, &pt) < 0) + err(1, "identify request failed"); - if (nvme_completion_is_error(&pt.cpl)) { - printf("Passthrough command returned error.\n"); - exit(EX_IOERR); - } + if (nvme_completion_is_error(&pt.cpl)) + errx(1, "identify request returned error"); } int @@ -178,38 +167,35 @@ open_dev(const char *str, int *fd, int s if (!strnstr(str, NVME_CTRLR_PREFIX, strlen(NVME_CTRLR_PREFIX))) { if (show_error) - fprintf(stderr, - "Controller/namespace IDs must begin with '%s'.\n", + warnx("controller/namespace ids must begin with '%s'", NVME_CTRLR_PREFIX); if (exit_on_error) - exit(EX_USAGE); + exit(1); else - return (EX_USAGE); + return (1); } snprintf(full_path, sizeof(full_path), "/dev/%s", str); if (stat(full_path, &devstat) != 0) { if (show_error) - fprintf(stderr, "Could not stat %s. errno=%d (%s)\n", - full_path, errno, strerror(errno)); + warn("could not stat %s", full_path); if (exit_on_error) - exit(EX_NOINPUT); + exit(1); else - return (EX_NOINPUT); + return (1); } *fd = open(full_path, O_RDWR); if (*fd < 0) { if (show_error) - fprintf(stderr, "Could not open %s. errno=%d (%s)\n", - full_path, errno, strerror(errno)); + warn("could not open %s", full_path); if (exit_on_error) - exit(EX_NOPERM); + exit(1); else - return (EX_NOPERM); + return (1); } - return (EX_OK); + return (0); } int Modified: head/sbin/nvmecontrol/perftest.c ============================================================================== --- head/sbin/nvmecontrol/perftest.c Tue Jul 9 21:03:39 2013 (r253108) +++ head/sbin/nvmecontrol/perftest.c Tue Jul 9 21:14:15 2013 (r253109) @@ -31,14 +31,13 @@ __FBSDID("$FreeBSD$"); #include <sys/ioccom.h> #include <ctype.h> -#include <errno.h> +#include <err.h> #include <fcntl.h> #include <stdbool.h> #include <stddef.h> #include <stdio.h> #include <stdlib.h> #include <string.h> -#include <sysexits.h> #include <unistd.h> #include "nvmecontrol.h" @@ -72,7 +71,7 @@ perftest_usage(void) { fprintf(stderr, "usage:\n"); fprintf(stderr, PERFTEST_USAGE); - exit(EX_USAGE); + exit(1); } void @@ -168,14 +167,10 @@ perftest(int argc, char *argv[]) perftest_usage(); open_dev(argv[optind], &fd, 1, 1); - if (ioctl(fd, ioctl_cmd, &io_test) < 0) { - fprintf(stderr, "NVME_IO_TEST failed. errno=%d (%s)\n", errno, - strerror(errno)); - close(fd); - exit(EX_IOERR); - } + if (ioctl(fd, ioctl_cmd, &io_test) < 0) + err(1, "ioctl NVME_IO_TEST failed"); close(fd); print_perftest(&io_test, perthread); - exit(EX_OK); + exit(0); } Modified: head/sbin/nvmecontrol/reset.c ============================================================================== --- head/sbin/nvmecontrol/reset.c Tue Jul 9 21:03:39 2013 (r253108) +++ head/sbin/nvmecontrol/reset.c Tue Jul 9 21:14:15 2013 (r253109) @@ -30,12 +30,11 @@ __FBSDID("$FreeBSD$"); #include <sys/param.h> #include <sys/ioccom.h> -#include <errno.h> +#include <err.h> #include <fcntl.h> #include <stdio.h> #include <stdlib.h> #include <string.h> -#include <sysexits.h> #include <unistd.h> #include "nvmecontrol.h" @@ -45,7 +44,7 @@ reset_usage(void) { fprintf(stderr, "usage:\n"); fprintf(stderr, RESET_USAGE); - exit(EX_USAGE); + exit(1); } void @@ -65,11 +64,8 @@ reset(int argc, char *argv[]) reset_usage(); open_dev(argv[optind], &fd, 1, 1); - if (ioctl(fd, NVME_RESET_CONTROLLER) < 0) { - printf("Reset request to %s failed. errno=%d (%s)\n", - argv[optind], errno, strerror(errno)); - exit(EX_IOERR); - } + if (ioctl(fd, NVME_RESET_CONTROLLER) < 0) + err(1, "reset request to %s failed", argv[optind]); - exit(EX_OK); + exit(0); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201307092114.r69LEGhr044445>