Changeset View
Standalone View
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. | |||||
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; | ||||
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 '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 " | ||||
mav: I am not sure what to do, but I am not exactly happy about changing option name here. Why -i… | |||||
"\"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) { | |||||
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__); | |||||
Not Done Inline ActionsWhy 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. | |||||
retval = 1; | |||||
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->value = strdup(value); | |||||
free(tmpstr); | |||||
Done Inline ActionsHere 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. | |||||
STAILQ_INSERT_TAIL(&option_list, option, links); | |||||
break; | |||||
} | |||||
case 'p': | case 'p': | ||||
targ_port = strtol(optarg, NULL, 0); | targ_port = strtol(optarg, NULL, 0); | ||||
break; | break; | ||||
Not Done Inline ActionsI 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… | |||||
Not Done Inline ActionsThis 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… | |||||
Not Done Inline ActionsI'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… | |||||
Done Inline ActionsYou'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… | |||||
Done Inline ActionsWhy? mav: Why? | |||||
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); | ||||
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) { | ||||
Done Inline ActionsI 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… | |||||
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) { | ||||
Done Inline ActionsSame 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. | |||||
Done Inline ActionsWhy was that new-line removed? mav: Why was that new-line removed? | |||||
Not Done Inline ActionsBetter, but still seems like a pointless whitespace change. mav: Better, but still seems like a pointless whitespace change. | |||||
Done Inline ActionsWhy not port_str? mav: Why not port_str? | |||||
Not Done Inline ActionsWhy -d is twice here, while -r is missing? mav: Why -d is twice here, while -r is missing? | |||||
Not Done Inline ActionsHere missing -c -d and -r options. mav: Here missing -c -d and -r options. |
I am not sure what to do, but I am not exactly happy about changing option name here. Why -i actually?