Skip to content

Commit

Permalink
ctladm: don't require the use of "-p" with "port -r"
Browse files Browse the repository at this point in the history
When removing a port, the ioctl frontend requires the "-p" argument.
But other frontends, like cfiscsi, do not.  So don't require that
argument in the ctladm command.  The frontend driver will report an
error if any required argument is missing.

MFC after:	2 weeks
Sponsored by:	Axcient
Reviewed by:    mav
Pull Request:   freebsd#1279
  • Loading branch information
asomers committed Jun 10, 2024
1 parent afecc74 commit edbd489
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 15 deletions.
2 changes: 1 addition & 1 deletion sys/cam/ctl/ctl_frontend_ioctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ cfi_ioctl_port_remove(struct ctl_req *req)
if (port_id == -1) {
req->status = CTL_LUN_ERROR;
snprintf(req->error_str, sizeof(req->error_str),
"port_id not provided");
"Missing required argument: port_id");
return;
}

Expand Down
3 changes: 1 addition & 2 deletions usr.sbin/ctladm/ctladm.8
Original file line number Diff line number Diff line change
Expand Up @@ -673,8 +673,7 @@ and
Specify the frontend port number.
The port numbers can be found in the frontend port list.
.It Fl r
Remove port specified with
.Pq Fl p Ar targ_port .
Remove a port.
.It Fl t Ar fe_type
Specify the frontend type used by the
.Pq Fl o ,
Expand Down
10 changes: 3 additions & 7 deletions usr.sbin/ctladm/ctladm.c
Original file line number Diff line number Diff line change
Expand Up @@ -580,11 +580,6 @@ cctl_port(int fd, int argc, char **argv, char *combinedopt)
break;
}
case CCTL_PORT_MODE_REMOVE:
if (targ_port == -1) {
warnx("%s: -r requires -p", __func__);
retval = 1;
goto bailout;
}
/* FALLTHROUGH */
case CCTL_PORT_MODE_CREATE: {
bzero(&req, sizeof(req));
Expand All @@ -594,8 +589,9 @@ cctl_port(int fd, int argc, char **argv, char *combinedopt)

if (port_mode == CCTL_PORT_MODE_REMOVE) {
req.reqtype = CTL_REQ_REMOVE;
nvlist_add_stringf(option_list, "port_id", "%d",
targ_port);
if (targ_port != -1)
nvlist_add_stringf(option_list, "port_id", "%d",
targ_port);
} else
req.reqtype = CTL_REQ_CREATE;

Expand Down
28 changes: 23 additions & 5 deletions usr.sbin/ctladm/tests/port.sh
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ cleanup() {
;;
"iscsi")
TARGET=`awk '/target:/ {print $2}' port-create.txt`
# PORTNUM is ignored, but must be set
PORTNUM=9999
ctladm port -r -d $driver -p "$PORTNUM" -O cfiscsi_portal_group_tag=$PGTAG -O cfiscsi_target=$TARGET
;;
esac
Expand Down Expand Up @@ -68,6 +66,25 @@ create_ioctl_cleanup()
cleanup ioctl
}

atf_test_case remove_ioctl_without_required_args cleanup
remove_ioctl_without_required_args_head()
{
atf_set "descr" "ctladm will gracefully fail to remove an ioctl target if required arguments are missing"
atf_set "require.user" "root"
}
remove_ioctl_without_required_args_body()
{
skip_if_ctld

atf_check -o save:port-create.txt ctladm port -c -d "ioctl"
atf_check egrep -q "Port created successfully" port-create.txt
atf_check -s exit:1 -e match:"Missing required argument: port_id" ctladm port -r -d "ioctl"
}
remove_ioctl_without_required_args_cleanup()
{
cleanup ioctl
}

atf_test_case create_iscsi cleanup
create_iscsi_head()
{
Expand Down Expand Up @@ -246,7 +263,7 @@ remove_iscsi_body()
atf_check -o save:port-create.txt ctladm port -c -d "iscsi" -O cfiscsi_portal_group_tag=$PGTAG -O cfiscsi_target="$TARGET"
portnum=`awk '/port:/ {print $2}' port-create.txt`
atf_check -o save:portlist.txt ctladm portlist -qf iscsi
atf_check -o inline:"Port destroyed successfully\n" ctladm port -r -d iscsi -p 9999 -O cfiscsi_portal_group_tag=$PGTAG -O cfiscsi_target="$TARGET"
atf_check -o inline:"Port destroyed successfully\n" ctladm port -r -d iscsi -O cfiscsi_portal_group_tag=$PGTAG -O cfiscsi_target="$TARGET"
# Check that the port was removed. A new port may have been added with
# the same ID, so match against the target and tag, too.
PGTAGHEX=0x7631 # PGTAG in hex
Expand All @@ -270,8 +287,8 @@ remove_iscsi_without_required_args_body()
TARGET=iqn.2018-10.myhost.remove_iscsi_without_required_args
atf_check -o save:port-create.txt ctladm port -c -d "iscsi" -O cfiscsi_portal_group_tag=$PGTAG -O cfiscsi_target="$TARGET"
echo "target: $TARGET" >> port-create.txt
atf_check -s exit:1 -e match:"Missing required argument: cfiscsi_portal_group_tag" ctladm port -r -d iscsi -p 9999 -O cfiscsi_target="$TARGET"
atf_check -s exit:1 -e match:"Missing required argument: cfiscsi_target" ctladm port -r -d iscsi -p 9999 -O cfiscsi_portal_group_tag=$PGTAG
atf_check -s exit:1 -e match:"Missing required argument: cfiscsi_portal_group_tag" ctladm port -r -d iscsi -O cfiscsi_target="$TARGET"
atf_check -s exit:1 -e match:"Missing required argument: cfiscsi_target" ctladm port -r -d iscsi -O cfiscsi_portal_group_tag=$PGTAG
}
remove_iscsi_without_required_args_cleanup()
{
Expand All @@ -288,6 +305,7 @@ atf_init_test_cases()
atf_add_test_case disable_ioctl
atf_add_test_case enable_ioctl
atf_add_test_case remove_ioctl
atf_add_test_case remove_ioctl_without_required_args
atf_add_test_case remove_iscsi
atf_add_test_case remove_iscsi_without_required_args
}

0 comments on commit edbd489

Please sign in to comment.