git: edbd489d09ba - main - ctladm: don't require the use of "-p" with "port -r"

From: Alan Somers <asomers_at_FreeBSD.org>
Date: Mon, 10 Jun 2024 16:01:45 UTC
The branch main has been updated by asomers:

URL: https://cgit.FreeBSD.org/src/commit/?id=edbd489d09babebdc6c03924a912013be584c409

commit edbd489d09babebdc6c03924a912013be584c409
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2024-06-06 19:14:43 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2024-06-10 16:01:25 +0000

    ctladm: don't require the use of "-p" with "port -r"
    
    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:   https://github.com/freebsd/freebsd-src/pull/1279
---
 sys/cam/ctl/ctl_frontend_ioctl.c |  2 +-
 usr.sbin/ctladm/ctladm.8         |  3 +--
 usr.sbin/ctladm/ctladm.c         | 10 +++-------
 usr.sbin/ctladm/tests/port.sh    | 28 +++++++++++++++++++++++-----
 4 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/sys/cam/ctl/ctl_frontend_ioctl.c b/sys/cam/ctl/ctl_frontend_ioctl.c
index 7fc8deac82b3..3449154afb38 100644
--- a/sys/cam/ctl/ctl_frontend_ioctl.c
+++ b/sys/cam/ctl/ctl_frontend_ioctl.c
@@ -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;
 	}
 
diff --git a/usr.sbin/ctladm/ctladm.8 b/usr.sbin/ctladm/ctladm.8
index 5c6e4aa04c77..72f0162eed54 100644
--- a/usr.sbin/ctladm/ctladm.8
+++ b/usr.sbin/ctladm/ctladm.8
@@ -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 ,
diff --git a/usr.sbin/ctladm/ctladm.c b/usr.sbin/ctladm/ctladm.c
index 14951797ddf1..46b7b88547dd 100644
--- a/usr.sbin/ctladm/ctladm.c
+++ b/usr.sbin/ctladm/ctladm.c
@@ -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));
@@ -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;
 
diff --git a/usr.sbin/ctladm/tests/port.sh b/usr.sbin/ctladm/tests/port.sh
index 1f2c9aaed5c1..139e1a7d29a0 100644
--- a/usr.sbin/ctladm/tests/port.sh
+++ b/usr.sbin/ctladm/tests/port.sh
@@ -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
@@ -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()
 {
@@ -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
@@ -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()
 {
@@ -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
 }