Page MenuHomeFreeBSD
Differential D9299 Diff 24404 usr.sbin/ctladm/ctladm.c

Changeset View

Standalone View

View Options

usr.sbin/ctladm/ctladm.c

Context not available.
#include <sys/queue.h> #include <sys/queue.h>
#include <sys/callout.h> #include <sys/callout.h>
#include <sys/sbuf.h> #include <sys/sbuf.h>
#include <sys/nv.h>
#include <stdint.h> #include <stdint.h>
#include <stdio.h> #include <stdio.h>
#include <stdlib.h> #include <stdlib.h>
Context not available.
const char *subopt; const char *subopt;
}; };
struct cctl_req_option {
char *name;
int namelen;
char *value;
int vallen;
STAILQ_ENTRY(cctl_req_option) links;
};
typedef enum { typedef enum {
CC_OR_NOT_FOUND, CC_OR_NOT_FOUND,
CC_OR_AMBIGUOUS, CC_OR_AMBIGUOUS,
Context not available.
{"lunmap", CTLADM_CMD_LUNMAP, CTLADM_ARG_NONE, "p:l:L:"}, {"lunmap", CTLADM_CMD_LUNMAP, CTLADM_ARG_NONE, "p:l:L:"},
{"modesense", CTLADM_CMD_MODESENSE, CTLADM_ARG_NEED_TL, "P:S:dlm:c:"}, {"modesense", CTLADM_CMD_MODESENSE, CTLADM_ARG_NEED_TL, "P:S:dlm:c:"},
{"modify", CTLADM_CMD_MODIFY, CTLADM_ARG_NONE, "b:l:o:s:"}, {"modify", CTLADM_CMD_MODIFY, CTLADM_ARG_NONE, "b:l:o:s:"},
{"port", CTLADM_CMD_PORT, CTLADM_ARG_NONE, "lo:p:qt:w:W:x"}, {"port", CTLADM_CMD_PORT, CTLADM_ARG_NONE, "li:o:crp:qt:w:W:x"},
{"portlist", CTLADM_CMD_PORTLIST, CTLADM_ARG_NONE, "f:ilp:qvx"}, {"portlist", CTLADM_CMD_PORTLIST, CTLADM_ARG_NONE, "f:ilp:qvx"},
{"prin", CTLADM_CMD_PRES_IN, CTLADM_ARG_NEED_TL, "a:"}, {"prin", CTLADM_CMD_PRES_IN, CTLADM_ARG_NEED_TL, "a:"},
{"prout", CTLADM_CMD_PRES_OUT, CTLADM_ARG_NEED_TL, "a:k:r:s:"}, {"prout", CTLADM_CMD_PRES_OUT, CTLADM_ARG_NEED_TL, "a:k:r:s:"},
Context not available.
CCTL_PORT_MODE_LIST, CCTL_PORT_MODE_LIST,
CCTL_PORT_MODE_SET, CCTL_PORT_MODE_SET,
CCTL_PORT_MODE_ON, CCTL_PORT_MODE_ON,
CCTL_PORT_MODE_OFF CCTL_PORT_MODE_OFF,
CCTL_PORT_MODE_CREATE,
CCTL_PORT_MODE_REMOVE
} cctl_port_mode; } cctl_port_mode;
static struct ctladm_opts cctl_fe_table[] = { static struct ctladm_opts cctl_fe_table[] = {
Context not available.
uint64_t wwnn = 0, wwpn = 0; uint64_t wwnn = 0, wwpn = 0;
cctl_port_mode port_mode = CCTL_PORT_MODE_NONE; cctl_port_mode port_mode = CCTL_PORT_MODE_NONE;
struct ctl_port_entry entry; struct ctl_port_entry entry;
struct ctl_req req;
ctl_port_type port_type = CTL_PORT_NONE; ctl_port_type port_type = CTL_PORT_NONE;
STAILQ_HEAD(, cctl_req_option) option_list;
int quiet = 0, xml = 0; int quiet = 0, xml = 0;
STAILQ_INIT(&option_list);
while ((c = getopt(argc, argv, combinedopt)) != -1) { while ((c = getopt(argc, argv, combinedopt)) != -1) {
switch (c) { switch (c) {
case 'l': case 'l':
Context not available.
port_mode = CCTL_PORT_MODE_LIST; port_mode = CCTL_PORT_MODE_LIST;
break; break;
case 'o': case 'c':
port_mode = CCTL_PORT_MODE_CREATE;
break;
case 'r':
port_mode = CCTL_PORT_MODE_REMOVE;
break;
case 'i':
mavUnsubmitted
Not Done
Inline Actions

I am not sure what to do, but I am not exactly happy about changing option name here. Why -i actually?

mav: I am not sure what to do, but I am not exactly happy about changing option name here. Why -i…
if (port_mode != CCTL_PORT_MODE_NONE) if (port_mode != CCTL_PORT_MODE_NONE)
goto bailout_badarg; goto bailout_badarg;
Context not available.
else if (strcasecmp(optarg, "off") == 0) else if (strcasecmp(optarg, "off") == 0)
port_mode = CCTL_PORT_MODE_OFF; port_mode = CCTL_PORT_MODE_OFF;
else { else {
warnx("Invalid -o argument %s, \"on\" or " warnx("Invalid -i argument %s, \"on\" or "
"\"off\" are the only valid args", "\"off\" are the only valid args",
optarg); optarg);
retval = 1; retval = 1;
Context not available.
goto bailout; goto bailout;
} }
break; break;
case 'o': {
struct cctl_req_option *option;
char *tmpstr;
char *name, *value;
tmpstr = strdup(optarg);
name = strsep(&tmpstr, "=");
if (name == NULL) {
mavUnsubmitted
Not Done
Inline Actions

Why do you need second strsep() here? tmpstr should already be the value.

mav: Why do you need second strsep() here? tmpstr should already be the value.
warnx("%s: option -o takes \"name=value\""
"argument", __func__);
retval = 1;
goto bailout;
}
value = strsep(&tmpstr, "=");
if (value == NULL) {
warnx("%s: option -o takes \"name=value\""
"argument", __func__);
retval = 1;
goto bailout;
}
option = malloc(sizeof(*option));
mavUnsubmitted
Done
Inline Actions

Here should be free(driver) before new assignment in case -d specified twice.

mav: Here should be free(driver) before new assignment in case -d specified twice.
if (option == NULL) {
warn("%s: error allocating %zd bytes",
__func__, sizeof(*option));
retval = 1;
goto bailout;
}
option->name = strdup(name);
option->value = strdup(value);
free(tmpstr);
STAILQ_INSERT_TAIL(&option_list, option, links);
mavUnsubmitted
Not Done
Inline Actions

I've already told that double conversion looks unneeded to me. Why not write directly into the nvlist?

mav: I've already told that double conversion looks unneeded to me. Why not write directly into the…
break;
}
case 'p': case 'p':
targ_port = strtol(optarg, NULL, 0); targ_port = strtol(optarg, NULL, 0);
break; break;
mavUnsubmitted
Done
Inline Actions

You've probably missed my previous comment about this. There should be command line option to specify the driver.

mav: You've probably missed my previous comment about this. There should be command line option to…
Context not available.
cctl_portlist(fd, argcx, argvx, opts); cctl_portlist(fd, argcx, argvx, opts);
break; break;
} }
case CCTL_PORT_MODE_REMOVE:
if (targ_port == -1) {
warnx("%s: -r require -p", __func__);
retval = 1;
goto bailout;
}
case CCTL_PORT_MODE_CREATE: {
struct cctl_req_option *option;
int64_t val;
bzero(&req, sizeof(req));
strlcpy(req.driver, "ioctl", sizeof(req.driver));
mavUnsubmitted
Not Done
Inline Actions

I don't think this is a good assumption. There should be an option, alike to -b for backends, to specify what port to create. While there is only ioctl ports are supported now, that may be not for long.

mav: I don't think this is a good assumption. There should be an option, alike to -b for backends…
req.args_nvl = nvlist_create(0);
if (port_mode == CCTL_PORT_MODE_REMOVE) {
req.reqtype = CTL_REQ_REMOVE;
nvlist_add_number(req.args_nvl, "port_id", targ_port);
} else {
req.reqtype = CTL_REQ_CREATE;
STAILQ_FOREACH(option, &option_list, links) {
if (strcmp(option->name, "pp") == 0 ||
strcmp(option->name, "vp") == 0) {
mavUnsubmitted
Not Done
Inline Actions

This type hard coding does not look good to me. I'd just pass everything as strings and let kernel decide.

mav: This type hard coding does not look good to me. I'd just pass everything as strings and let…
val = strtoq(option->value, NULL, 0);
nvlist_add_number(req.args_nvl,
mavUnsubmitted
Done
Inline Actions

Why?

mav: Why?
option->name, val);
} else {
nvlist_add_string(req.args_nvl,
option->name, option->value);
}
}
}
STAILQ_FOREACH(option, &option_list, links) {
STAILQ_REMOVE(&option_list, option, cctl_req_option,
links);
free(option->name);
free(option->value);
free(option);
}
req.args = nvlist_pack(req.args_nvl, &req.args_len);
retval = ioctl(fd, CTL_PORT_REQ, &req);
nvlist_destroy(req.args_nvl);
if (retval == -1) {
warn("%s: CTL_PORT_REQ ioctl failed", __func__);
retval = 1;
goto bailout;
}
switch (req.status) {
case CTL_LUN_ERROR:
warnx("error: %s", req.error_str);
retval = 1;
goto bailout;
case CTL_LUN_WARNING:
warnx("warning: %s", req.error_str);
break;
case CTL_LUN_OK:
break;
default:
warnx("unknown status: %d", req.status);
retval = 1;
goto bailout;
}
break;
}
case CCTL_PORT_MODE_SET: case CCTL_PORT_MODE_SET:
if (targ_port == -1) { if (targ_port == -1) {
warnx("%s: -w and -W require -n", __func__); warnx("%s: -w and -W require -n", __func__);
Context not available.
return (retval); return (retval);
} }
struct cctl_req_option {
char *name;
int namelen;
char *value;
int vallen;
STAILQ_ENTRY(cctl_req_option) links;
};
static int static int
cctl_create_lun(int fd, int argc, char **argv, char *combinedopt) cctl_create_lun(int fd, int argc, char **argv, char *combinedopt)
{ {
Context not available.
req.reqdata.create.flags |= CTL_LUN_FLAG_DEVID; req.reqdata.create.flags |= CTL_LUN_FLAG_DEVID;
} }
req.num_be_args = num_options; if (!STAILQ_EMPTY(&option_list)) {
if (num_options > 0) {
struct cctl_req_option *option, *next_option; struct cctl_req_option *option, *next_option;
int i; int i;
req.be_args = malloc(num_options * sizeof(*req.be_args)); req.args_nvl = nvlist_create(0);
if (req.be_args == NULL) { if (req.args_nvl == NULL) {
warn("%s: error allocating %zd bytes", __func__, warn("%s: error allocating nvlist", __func__);
num_options * sizeof(*req.be_args));
retval = 1; retval = 1;
goto bailout; goto bailout;
} }
Context not available.
for (i = 0, option = STAILQ_FIRST(&option_list); for (i = 0, option = STAILQ_FIRST(&option_list);
i < num_options; i++, option = next_option) { i < num_options; i++, option = next_option) {
next_option = STAILQ_NEXT(option, links); next_option = STAILQ_NEXT(option, links);
nvlist_add_string(req.args_nvl, option->name,
req.be_args[i].namelen = option->namelen; option->value);
req.be_args[i].name = strdup(option->name);
req.be_args[i].vallen = option->vallen;
req.be_args[i].value = strdup(option->value);
/*
* XXX KDM do we want a way to specify a writeable
* flag of some sort? Do we want a way to specify
* binary data?
*/
req.be_args[i].flags = CTL_BEARG_ASCII | CTL_BEARG_RD;
STAILQ_REMOVE(&option_list, option, cctl_req_option, STAILQ_REMOVE(&option_list, option, cctl_req_option,
links); links);
free(option->name); free(option->name);
free(option->value); free(option->value);
free(option); free(option);
} }
req.args = nvlist_pack(req.args_nvl, &req.args_len);
if (req.args == NULL) {
warn("%s: error packing nvlist", __func__);
retval = 1;
goto bailout;
}
} }
if (ioctl(fd, CTL_LUN_REQ, &req) == -1) { if (ioctl(fd, CTL_LUN_REQ, &req) == -1) {
Context not available.
req.reqdata.create.blocksize_bytes); req.reqdata.create.blocksize_bytes);
fprintf(stdout, "LUN ID: %d\n", req.reqdata.create.req_lun_id); fprintf(stdout, "LUN ID: %d\n", req.reqdata.create.req_lun_id);
fprintf(stdout, "Serial Number: %s\n", req.reqdata.create.serial_num); fprintf(stdout, "Serial Number: %s\n", req.reqdata.create.serial_num);
fprintf(stdout, "Device ID; %s\n", req.reqdata.create.device_id); fprintf(stdout, "Device ID: %s\n", req.reqdata.create.device_id);
bailout: bailout:
return (retval); return (retval);
mavUnsubmitted
Done
Inline Actions

I suppose nvlist_destroy() should be moved up just after nvlist_pack() to not leak memory if one fail. Same time here should probably be freed req.args.

mav: I suppose nvlist_destroy() should be moved up just after nvlist_pack() to not leak memory if…
Context not available.
req.reqdata.rm.lun_id = lun_id; req.reqdata.rm.lun_id = lun_id;
req.num_be_args = num_options; if (!STAILQ_EMPTY(&option_list)) {
if (num_options > 0) {
struct cctl_req_option *option, *next_option; struct cctl_req_option *option, *next_option;
int i; int i;
req.be_args = malloc(num_options * sizeof(*req.be_args)); req.args_nvl = nvlist_create(0);
if (req.be_args == NULL) {
warn("%s: error allocating %zd bytes", __func__, if (req.args_nvl == NULL) {
num_options * sizeof(*req.be_args)); warn("%s: error allocating nvlist", __func__);
retval = 1; retval = 1;
goto bailout; goto bailout;
} }
Context not available.
for (i = 0, option = STAILQ_FIRST(&option_list); for (i = 0, option = STAILQ_FIRST(&option_list);
i < num_options; i++, option = next_option) { i < num_options; i++, option = next_option) {
next_option = STAILQ_NEXT(option, links); next_option = STAILQ_NEXT(option, links);
nvlist_add_string(req.args_nvl, option->name,
req.be_args[i].namelen = option->namelen; option->value);
req.be_args[i].name = strdup(option->name);
req.be_args[i].vallen = option->vallen;
req.be_args[i].value = strdup(option->value);
/*
* XXX KDM do we want a way to specify a writeable
* flag of some sort? Do we want a way to specify
* binary data?
*/
req.be_args[i].flags = CTL_BEARG_ASCII | CTL_BEARG_RD;
STAILQ_REMOVE(&option_list, option, cctl_req_option, STAILQ_REMOVE(&option_list, option, cctl_req_option,
links); links);
free(option->name); free(option->name);
free(option->value); free(option->value);
free(option); free(option);
} }
req.args = nvlist_pack(req.args_nvl, &req.args_len);
if (req.args == NULL) {
warn("%s: error packing nvlist", __func__);
retval = 1;
goto bailout;
}
} }
if (ioctl(fd, CTL_LUN_REQ, &req) == -1) { if (ioctl(fd, CTL_LUN_REQ, &req) == -1) {
mavUnsubmitted
Done
Inline Actions

Same as above.

mav: Same as above.
Context not available.
req.reqdata.modify.lun_id = lun_id; req.reqdata.modify.lun_id = lun_id;
req.reqdata.modify.lun_size_bytes = lun_size; req.reqdata.modify.lun_size_bytes = lun_size;
req.num_be_args = num_options; if (!STAILQ_EMPTY(&option_list)) {
if (num_options > 0) {
struct cctl_req_option *option, *next_option; struct cctl_req_option *option, *next_option;
int i; int i;
req.be_args = malloc(num_options * sizeof(*req.be_args)); req.args_nvl = nvlist_create(0);
if (req.be_args == NULL) {
warn("%s: error allocating %zd bytes", __func__, if (req.args_nvl == NULL) {
num_options * sizeof(*req.be_args)); warn("%s: error allocating nvlist", __func__);
retval = 1; retval = 1;
goto bailout; goto bailout;
} }
Context not available.
for (i = 0, option = STAILQ_FIRST(&option_list); for (i = 0, option = STAILQ_FIRST(&option_list);
i < num_options; i++, option = next_option) { i < num_options; i++, option = next_option) {
next_option = STAILQ_NEXT(option, links); next_option = STAILQ_NEXT(option, links);
nvlist_add_string(req.args_nvl, option->name,
req.be_args[i].namelen = option->namelen; option->value);
req.be_args[i].name = strdup(option->name);
req.be_args[i].vallen = option->vallen;
req.be_args[i].value = strdup(option->value);
/*
* XXX KDM do we want a way to specify a writeable
* flag of some sort? Do we want a way to specify
* binary data?
*/
req.be_args[i].flags = CTL_BEARG_ASCII | CTL_BEARG_RD;
STAILQ_REMOVE(&option_list, option, cctl_req_option, STAILQ_REMOVE(&option_list, option, cctl_req_option,
links); links);
free(option->name); free(option->name);
free(option->value); free(option->value);
free(option); free(option);
} }
req.args = nvlist_pack(req.args_nvl, &req.args_len);
if (req.args == NULL) {
warn("%s: error packing nvlist", __func__);
retval = 1;
goto bailout;
}
} }
if (ioctl(fd, CTL_LUN_REQ, &req) == -1) { if (ioctl(fd, CTL_LUN_REQ, &req) == -1) {
Context not available.
mavUnsubmitted
Done
Inline Actions

Why was that new-line removed?

mav: Why was that new-line removed?
mavUnsubmitted
Not Done
Inline Actions

Better, but still seems like a pointless whitespace change.

mav: Better, but still seems like a pointless whitespace change.
mavUnsubmitted
Done
Inline Actions

Why not port_str?

mav: Why not port_str?
mavUnsubmitted
Not Done
Inline Actions

Why -d is twice here, while -r is missing?

mav: Why -d is twice here, while -r is missing?
mavUnsubmitted
Not Done
Inline Actions

Here missing -c -d and -r options.

mav: Here missing -c -d and -r options.