Page MenuHomeFreeBSD
Differential D9299 Diff 24636 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.
{"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;
nvlist_t *option_list;
ctl_port_type port_type = CTL_PORT_NONE; ctl_port_type port_type = CTL_PORT_NONE;
int quiet = 0, xml = 0; int quiet = 0, xml = 0;
option_list = nvlist_create(0);
if (option_list == NULL)
err(1, "%s: unable to allocate nvlist", __func__);
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':
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)
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…
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': {
char *tmpstr;
char *name, *value;
tmpstr = strdup(optarg);
name = strsep(&tmpstr, "=");
if (name == NULL) {
warnx("%s: option -o takes \"name=value\""
"argument", __func__);
retval = 1;
goto bailout;
}
value = strsep(&tmpstr, "=");
if (value == 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;
}
free(tmpstr);
nvlist_add_string(option_list, name, value);
break;
}
case 'p': case 'p':
targ_port = strtol(optarg, NULL, 0); targ_port = strtol(optarg, NULL, 0);
break; break;
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…
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.
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: {
bzero(&req, sizeof(req));
strlcpy(req.driver, "ioctl", sizeof(req.driver));
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…
if (port_mode == CCTL_PORT_MODE_REMOVE) {
req.reqtype = CTL_REQ_REMOVE;
nvlist_add_stringf(option_list, "port_id", "%d",
targ_port);
} else
req.reqtype = CTL_REQ_CREATE;
req.args = nvlist_pack(option_list, &req.args_len);
if (req.args == NULL) {
warn("%s: error packing nvlist", __func__);
retval = 1;
goto bailout;
}
retval = ioctl(fd, CTL_PORT_REQ, &req);
nvlist_destroy(req.args_nvl);
if (retval == -1) {
warn("%s: CTL_PORT_REQ ioctl failed", __func__);
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…
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:
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…
warnx("warning: %s", req.error_str);
break;
mavUnsubmitted
Done
Inline Actions

Why?

mav: Why?
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.
char *device_id = NULL; char *device_id = NULL;
int lun_size_set = 0, blocksize_set = 0, lun_id_set = 0; int lun_size_set = 0, blocksize_set = 0, lun_id_set = 0;
char *backend_name = NULL; char *backend_name = NULL;
STAILQ_HEAD(, cctl_req_option) option_list; nvlist_t *option_list;
int num_options = 0;
int retval = 0, c; int retval = 0, c;
STAILQ_INIT(&option_list);
option_list = nvlist_create(0);
if (option_list == NULL)
err(1, "%s: unable to allocate nvlist", __func__);
while ((c = getopt(argc, argv, combinedopt)) != -1) { while ((c = getopt(argc, argv, combinedopt)) != -1) {
switch (c) { switch (c) {
Context not available.
lun_id_set = 1; lun_id_set = 1;
break; break;
case 'o': { case 'o': {
struct cctl_req_option *option;
char *tmpstr; char *tmpstr;
char *name, *value; char *name, *value;
Context not available.
retval = 1; retval = 1;
goto bailout; goto bailout;
} }
option = malloc(sizeof(*option));
if (option == NULL) {
warn("%s: error allocating %zd bytes",
__func__, sizeof(*option));
retval = 1;
goto bailout;
}
option->name = strdup(name);
option->namelen = strlen(name) + 1;
option->value = strdup(value);
option->vallen = strlen(value) + 1;
free(tmpstr); free(tmpstr);
nvlist_add_string(option_list, name, value);
STAILQ_INSERT_TAIL(&option_list, option, links);
num_options++;
break; break;
} }
case 's': case 's':
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; req.args = nvlist_pack(option_list, &req.args_len);
if (num_options > 0) { if (req.args == NULL) {
struct cctl_req_option *option, *next_option; warn("%s: error packing nvlist", __func__);
int i; retval = 1;
goto bailout;
req.be_args = malloc(num_options * sizeof(*req.be_args));
if (req.be_args == NULL) {
warn("%s: error allocating %zd bytes", __func__,
num_options * sizeof(*req.be_args));
retval = 1;
goto bailout;
}
for (i = 0, option = STAILQ_FIRST(&option_list);
i < num_options; i++, option = next_option) {
next_option = STAILQ_NEXT(option, links);
req.be_args[i].namelen = option->namelen;
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,
links);
free(option->name);
free(option->value);
free(option);
}
} }
if (ioctl(fd, CTL_LUN_REQ, &req) == -1) { retval = ioctl(fd, CTL_LUN_REQ, &req);
nvlist_destroy(option_list);
if (retval == -1) {
warn("%s: error issuing CTL_LUN_REQ ioctl", __func__); warn("%s: error issuing CTL_LUN_REQ ioctl", __func__);
retval = 1; retval = 1;
goto bailout; goto bailout;
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);
Context not available.
uint32_t lun_id = 0; uint32_t lun_id = 0;
int lun_id_set = 0; int lun_id_set = 0;
char *backend_name = NULL; char *backend_name = NULL;
STAILQ_HEAD(, cctl_req_option) option_list; nvlist_t *option_list;
int num_options = 0;
int retval = 0, c; int retval = 0, c;
STAILQ_INIT(&option_list); option_list = nvlist_create(0);
if (option_list == NULL)
err(1, "%s: unable to allocate nvlist", __func__);
while ((c = getopt(argc, argv, combinedopt)) != -1) { while ((c = getopt(argc, argv, combinedopt)) != -1) {
switch (c) { switch (c) {
Context not available.
lun_id_set = 1; lun_id_set = 1;
break; break;
case 'o': { case 'o': {
struct cctl_req_option *option;
char *tmpstr; char *tmpstr;
char *name, *value; char *name, *value;
Context not available.
retval = 1; retval = 1;
goto bailout; goto bailout;
} }
option = malloc(sizeof(*option));
if (option == NULL) {
warn("%s: error allocating %zd bytes",
__func__, sizeof(*option));
retval = 1;
goto bailout;
}
option->name = strdup(name);
option->namelen = strlen(name) + 1;
option->value = strdup(value);
option->vallen = strlen(value) + 1;
free(tmpstr); free(tmpstr);
nvlist_add_string(option_list, name, value);
STAILQ_INSERT_TAIL(&option_list, option, links);
num_options++;
break; break;
} }
default: default:
Context not available.
req.reqtype = CTL_LUNREQ_RM; req.reqtype = CTL_LUNREQ_RM;
req.reqdata.rm.lun_id = lun_id; req.reqdata.rm.lun_id = lun_id;
req.num_be_args = num_options; req.args = nvlist_pack(option_list, &req.args_len);
if (num_options > 0) { if (req.args == NULL) {
struct cctl_req_option *option, *next_option; warn("%s: error packing nvlist", __func__);
int i; retval = 1;
goto bailout;
req.be_args = malloc(num_options * sizeof(*req.be_args));
if (req.be_args == NULL) {
warn("%s: error allocating %zd bytes", __func__,
num_options * sizeof(*req.be_args));
retval = 1;
goto bailout;
}
for (i = 0, option = STAILQ_FIRST(&option_list);
i < num_options; i++, option = next_option) {
next_option = STAILQ_NEXT(option, links);
req.be_args[i].namelen = option->namelen;
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,
links);
free(option->name);
free(option->value);
free(option);
}
} }
if (ioctl(fd, CTL_LUN_REQ, &req) == -1) { retval = ioctl(fd, CTL_LUN_REQ, &req);
nvlist_destroy(option_list);
if (retval == -1) {
warn("%s: error issuing CTL_LUN_REQ ioctl", __func__); warn("%s: error issuing CTL_LUN_REQ ioctl", __func__);
retval = 1; retval = 1;
goto bailout; goto bailout;
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.
uint32_t lun_id = 0; uint32_t lun_id = 0;
int lun_id_set = 0, lun_size_set = 0; int lun_id_set = 0, lun_size_set = 0;
char *backend_name = NULL; char *backend_name = NULL;
STAILQ_HEAD(, cctl_req_option) option_list; nvlist_t *option_list;
int num_options = 0;
int retval = 0, c; int retval = 0, c;
STAILQ_INIT(&option_list); option_list = nvlist_create(0);
if (option_list == NULL)
err(1, "%s: unable to allocate nvlist", __func__);
while ((c = getopt(argc, argv, combinedopt)) != -1) { while ((c = getopt(argc, argv, combinedopt)) != -1) {
switch (c) { switch (c) {
case 'b': case 'b':
Context not available.
lun_id_set = 1; lun_id_set = 1;
break; break;
case 'o': { case 'o': {
struct cctl_req_option *option;
char *tmpstr; char *tmpstr;
char *name, *value; char *name, *value;
Context not available.
retval = 1; retval = 1;
goto bailout; goto bailout;
} }
option = malloc(sizeof(*option));
if (option == NULL) {
warn("%s: error allocating %zd bytes",
__func__, sizeof(*option));
retval = 1;
goto bailout;
}
option->name = strdup(name);
option->namelen = strlen(name) + 1;
option->value = strdup(value);
option->vallen = strlen(value) + 1;
free(tmpstr); free(tmpstr);
nvlist_add_string(option_list, name, value);
STAILQ_INSERT_TAIL(&option_list, option, links);
num_options++;
break; break;
} }
case 's': case 's':
Context not available.
if (lun_id_set == 0) if (lun_id_set == 0)
errx(1, "%s: LUN id (-l) must be specified", __func__); errx(1, "%s: LUN id (-l) must be specified", __func__);
if (lun_size_set == 0 && num_options == 0) if (lun_size_set == 0 && nvlist_empty(option_list))
errx(1, "%s: size (-s) or options (-o) must be specified", errx(1, "%s: size (-s) or options (-o) must be specified",
__func__); __func__);
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; req.args = nvlist_pack(option_list, &req.args_len);
if (num_options > 0) { if (req.args == NULL) {
struct cctl_req_option *option, *next_option; warn("%s: error packing nvlist", __func__);
int i; retval = 1;
goto bailout;
req.be_args = malloc(num_options * sizeof(*req.be_args));
if (req.be_args == NULL) {
warn("%s: error allocating %zd bytes", __func__,
num_options * sizeof(*req.be_args));
retval = 1;
goto bailout;
}
for (i = 0, option = STAILQ_FIRST(&option_list);
i < num_options; i++, option = next_option) {
next_option = STAILQ_NEXT(option, links);
req.be_args[i].namelen = option->namelen;
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,
links);
free(option->name);
free(option->value);
free(option);
}
} }
if (ioctl(fd, CTL_LUN_REQ, &req) == -1) { retval = ioctl(fd, CTL_LUN_REQ, &req);
nvlist_destroy(option_list);
if (retval == -1) {
warn("%s: error issuing CTL_LUN_REQ ioctl", __func__); warn("%s: error issuing CTL_LUN_REQ ioctl", __func__);
retval = 1; retval = 1;
goto bailout; goto bailout;
Context not available.
mavUnsubmitted
Done
Inline Actions

Same as above.

mav: Same as above.
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.