12721 would like svcadm disable -c

Reviewed by: Jason King <jason.brian.king@gmail.com>
Heckling from the gallery by: Joshua M. Clulow <josh@sysmgr.org>
Approved by: Dan McDonald <danmcd@joyent.com>

14 files changed, 454 insertions, 217 deletions

diff --git a/usr/src/cmd/svc/startd/libscf.c b/usr/src/cmd/svc/startd/libscf.c
index c4a3b20117..fe54b500b6 100644
--- a/usr/src/cmd/svc/startd/libscf.c
+++ b/usr/src/cmd/svc/startd/libscf.c
@@ -22,6 +22,7 @@
 /*
  * Copyright (c) 2004, 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright 2012 Milan Jurik. All rights reserved.
+ * Copyright 2020 Joyent, Inc.
  */
 
 
@@ -1681,13 +1682,20 @@ libscf_set_deathrow(scf_instance_t *inst, int deathrow)
 }
 
 /*
+ * Since we're clearing the over-ridden enabled state for the service, we'll
+ * also take the opportunity to remove any comment.
+ *
  * Returns 0, ECONNABORTED, ECANCELED, or EPERM.
  */
 int
 libscf_delete_enable_ovr(scf_instance_t *inst)
 {
+	int r = scf_instance_delete_prop(inst, SCF_PG_GENERAL_OVR,
+	    SCF_PROPERTY_ENABLED);
+	if (r != 0)
+		return (r);
 	return (scf_instance_delete_prop(inst, SCF_PG_GENERAL_OVR,
-	    SCF_PROPERTY_ENABLED));
+	    SCF_PROPERTY_COMMENT));
 }
 
 /*
diff --git a/usr/src/cmd/svc/svcadm/svcadm.c b/usr/src/cmd/svc/svcadm/svcadm.c
index b51d218f45..c5a046565f 100644
--- a/usr/src/cmd/svc/svcadm/svcadm.c
+++ b/usr/src/cmd/svc/svcadm/svcadm.c
@@ -24,7 +24,7 @@
  */
 
 /*
- * Copyright 2015, Joyent, Inc. All rights reserved.
+ * Copyright 2020, Joyent, Inc. All rights reserved.
  */
 
 /*
@@ -82,6 +82,19 @@ struct ht_elt {
 };
 
 
+/*
+ * Callback data for enable/disable.
+ */
+#define	SET_ENABLED	0x1
+#define	SET_TEMPORARY	0x2
+#define	SET_RECURSIVE	0x4
+
+typedef struct {
+	char ed_comment[SCF_COMMENT_MAX_LENGTH];
+	int ed_flags;
+} enable_data_t;
+
+
 scf_handle_t *h;
 ssize_t max_scf_fmri_sz;
 static const char *emsg_permission_denied;
@@ -146,7 +159,7 @@ usage()
 	(void) fprintf(stderr, gettext(
 	"Usage: %1$s [-S <state>] [-v] [-Z | -z zone] [cmd [args ... ]]\n\n"
 	"\t%1$s enable [-rst] [<service> ...]\t- enable and online service(s)\n"
-	"\t%1$s disable [-st] [<service> ...]\t- disable and offline "
+	"\t%1$s disable [-c comment] [-st] [<service> ...] - disable "
 	"service(s)\n"
 	"\t%1$s restart [-d] [<service> ...]\t- restart specified service(s)\n"
 	"\t%1$s refresh [<service> ...]\t\t- re-read service configuration\n"
@@ -567,20 +580,150 @@ out:
 	return (ret);
 }
 
+static int
+delete_prop(const char *fmri, scf_instance_t *inst, const char *pgname,
+    const char *propname)
+{
+	int r = scf_instance_delete_prop(inst, pgname, propname);
+
+	switch (r) {
+	case 0:
+		break;
+
+	case ECANCELED:
+		uu_warn(emsg_no_service, fmri);
+		break;
+
+	case EACCES:
+		uu_warn(gettext("Could not delete %s/%s "
+		    "property of %s: backend access denied.\n"),
+		    pgname, propname, fmri);
+		break;
+
+	case EROFS:
+		uu_warn(gettext("Could not delete %s/%s "
+		    "property of %s: backend is read-only.\n"),
+		    pgname, propname, fmri);
+		break;
+
+	default:
+		bad_error("scf_instance_delete_prop", r);
+	}
+
+	return (r);
+}
+
 /*
- * Enable or disable inst, per enable.  If temp is true, set
- * general_ovr/enabled.  Otherwise set general/enabled and delete
- * general_ovr/enabled if it exists (order is important here: we don't want the
- * enabled status to glitch).
+ * Returns 0, EPERM, or EROFS.
+ */
+static int
+set_enabled_props(scf_propertygroup_t *pg, enable_data_t *ed)
+{
+	scf_transaction_entry_t *ent1;
+	scf_transaction_entry_t *ent2;
+	scf_transaction_t *tx;
+	scf_value_t *v2;
+	scf_value_t *v1;
+	int ret = 0, r;
+
+	if ((tx = scf_transaction_create(h)) == NULL ||
+	    (ent1 = scf_entry_create(h)) == NULL ||
+	    (ent2 = scf_entry_create(h)) == NULL ||
+	    (v1 = scf_value_create(h)) == NULL ||
+	    (v2 = scf_value_create(h)) == NULL)
+		scfdie();
+
+	for (;;) {
+		if (scf_transaction_start(tx, pg) == -1) {
+			switch (scf_error()) {
+			case SCF_ERROR_PERMISSION_DENIED:
+				ret = EPERM;
+				goto out;
+
+			case SCF_ERROR_BACKEND_READONLY:
+				ret = EROFS;
+				goto out;
+
+			default:
+				scfdie();
+			}
+		}
+
+		if (scf_transaction_property_change_type(tx, ent1,
+		    SCF_PROPERTY_ENABLED, SCF_TYPE_BOOLEAN) != 0) {
+			if (scf_error() != SCF_ERROR_NOT_FOUND)
+				scfdie();
+
+			if (scf_transaction_property_new(tx, ent1,
+			    SCF_PROPERTY_ENABLED, SCF_TYPE_BOOLEAN) != 0)
+				scfdie();
+		}
+
+		scf_value_set_boolean(v1, !!(ed->ed_flags & SET_ENABLED));
+
+		r = scf_entry_add_value(ent1, v1);
+		assert(r == 0);
+
+		if (scf_transaction_property_change_type(tx, ent2,
+		    SCF_PROPERTY_COMMENT, SCF_TYPE_ASTRING) != 0) {
+			if (scf_error() != SCF_ERROR_NOT_FOUND)
+				scfdie();
+
+			if (scf_transaction_property_new(tx, ent2,
+			    SCF_PROPERTY_COMMENT, SCF_TYPE_ASTRING) != 0)
+				scfdie();
+		}
+
+		if (scf_value_set_astring(v2, ed->ed_comment) != SCF_SUCCESS)
+			scfdie();
+
+		if (scf_entry_add_value(ent2, v2) != SCF_SUCCESS)
+			scfdie();
+
+		r = scf_transaction_commit(tx);
+		if (r == 1)
+			break;
+
+		scf_transaction_reset(tx);
+
+		if (r != 0) {
+			switch (scf_error()) {
+			case SCF_ERROR_PERMISSION_DENIED:
+				ret = EPERM;
+				goto out;
+
+			case SCF_ERROR_BACKEND_READONLY:
+				ret = EROFS;
+				goto out;
+
+			default:
+				scfdie();
+			}
+		}
+
+		if (scf_pg_update(pg) == -1)
+			scfdie();
+	}
+
+out:
+	scf_transaction_destroy(tx);
+	scf_entry_destroy(ent1);
+	scf_entry_destroy(ent2);
+	scf_value_destroy(v1);
+	scf_value_destroy(v2);
+	return (ret);
+}
+
+/*
+ * Enable or disable an instance.  SET_TEMPORARY modifications apply to
+ * general_ovr/ property group.
  */
 static void
-set_inst_enabled(const char *fmri, scf_instance_t *inst, boolean_t temp,
-    boolean_t enable)
+set_inst_enabled(const char *fmri, scf_instance_t *inst, enable_data_t *ed)
 {
 	scf_propertygroup_t *pg;
 	uint8_t b;
 	const char *pgname = NULL;	/* For emsg_pg_perm_denied */
-	int r;
 
 	pg = scf_pg_create(h);
 	if (pg == NULL)
@@ -625,14 +768,13 @@ set_inst_enabled(const char *fmri, scf_instance_t *inst, boolean_t temp,
 		}
 	}
 
-	if (temp) {
-		/* Set general_ovr/enabled */
+	if (ed->ed_flags & SET_TEMPORARY) {
 		pgname = SCF_PG_GENERAL_OVR;
 		if (pg_get_or_add(inst, pgname, SCF_PG_GENERAL_OVR_TYPE,
 		    SCF_PG_GENERAL_OVR_FLAGS, pg) != 0)
 			goto eperm;
 
-		switch (set_bool_prop(pg, SCF_PROPERTY_ENABLED, enable)) {
+		switch (set_enabled_props(pg, ed)) {
 		case 0:
 			break;
 
@@ -656,7 +798,7 @@ set_inst_enabled(const char *fmri, scf_instance_t *inst, boolean_t temp,
 		}
 
 		if (verbose)
-			(void) printf(enable ?
+			(void) printf((ed->ed_flags & SET_ENABLED) ?
 			    gettext("%s temporarily enabled.\n") :
 			    gettext("%s temporarily disabled.\n"), fmri);
 	} else {
@@ -671,7 +813,7 @@ again:
 		    SCF_PG_GENERAL_FLAGS, pg) != 0)
 			goto eperm;
 
-		switch (set_bool_prop(pg, SCF_PROPERTY_ENABLED, enable)) {
+		switch (set_enabled_props(pg, ed)) {
 		case 0:
 			break;
 
@@ -685,7 +827,7 @@ again:
 			 */
 			switch (get_bool_prop(pg, SCF_PROPERTY_ENABLED, &b)) {
 			case 0:
-				if ((b != 0) == (enable != B_FALSE))
+				if (!(b) == !(ed->ed_flags & SET_ENABLED))
 					break;
 				/* FALLTHROUGH */
 
@@ -716,39 +858,35 @@ again:
 			abort();
 		}
 
-		pgname = SCF_PG_GENERAL_OVR;
-		r = scf_instance_delete_prop(inst, pgname,
-		    SCF_PROPERTY_ENABLED);
-		switch (r) {
+		switch (delete_prop(fmri, inst, SCF_PG_GENERAL_OVR,
+		    SCF_PROPERTY_ENABLED)) {
 		case 0:
 			break;
 
-		case ECANCELED:
-			uu_warn(emsg_no_service, fmri);
-			goto out;
-
 		case EPERM:
 			goto eperm;
 
-		case EACCES:
-			uu_warn(gettext("Could not delete %s/%s "
-			    "property of %s: backend access denied.\n"),
-			    pgname, SCF_PROPERTY_ENABLED, fmri);
+		default:
 			goto out;
+		}
 
-		case EROFS:
-			uu_warn(gettext("Could not delete %s/%s "
-			    "property of %s: backend is read-only.\n"),
-			    pgname, SCF_PROPERTY_ENABLED, fmri);
-			goto out;
+		switch (delete_prop(fmri, inst, SCF_PG_GENERAL_OVR,
+		    SCF_PROPERTY_COMMENT)) {
+		case 0:
+			break;
+
+		case EPERM:
+			goto eperm;
 
 		default:
-			bad_error("scf_instance_delete_prop", r);
+			goto out;
 		}
 
-		if (verbose)
-			(void) printf(enable ?  gettext("%s enabled.\n") :
+		if (verbose) {
+			(void) printf((ed->ed_flags & SET_ENABLED) ?
+			    gettext("%s enabled.\n") :
 			    gettext("%s disabled.\n"), fmri);
+		}
 	}
 
 	scf_pg_destroy(pg);
@@ -1014,7 +1152,7 @@ out:
  * on cycle detection, or 0 on success.
  */
 static int
-enable_fmri_rec(char *fmri, boolean_t temp)
+enable_fmri_rec(char *fmri, enable_data_t *ed)
 {
 	scf_instance_t *inst;
 	scf_snapshot_t *snap;
@@ -1068,7 +1206,7 @@ enable_fmri_rec(char *fmri, boolean_t temp)
 		return (0);
 	}
 
-	set_inst_enabled(fmri, inst, temp, B_TRUE);
+	set_inst_enabled(fmri, inst, ed);
 
 	if ((snap = scf_snapshot_create(h)) == NULL ||
 	    (pg = scf_pg_create(h)) == NULL ||
@@ -1118,7 +1256,7 @@ enable_fmri_rec(char *fmri, boolean_t temp)
 		ret = 0;
 		goto out;
 	} else if (sz >= 0) {
-		switch (enable_fmri_rec(buf, temp)) {
+		switch (enable_fmri_rec(buf, ed)) {
 		case 0:
 			break;
 
@@ -1269,7 +1407,7 @@ enable_fmri_rec(char *fmri, boolean_t temp)
 			    -1)
 				scfdie();
 
-			switch (enable_fmri_rec(buf, temp)) {
+			switch (enable_fmri_rec(buf, ed)) {
 			case 0:
 				break;
 
@@ -1668,17 +1806,10 @@ out:
 }
 
 
-/*
- * Flags to control enable and disable actions.
- */
-#define	SET_ENABLED	0x1
-#define	SET_TEMPORARY	0x2
-#define	SET_RECURSIVE	0x4
-
 static int
 set_fmri_enabled(void *data, scf_walkinfo_t *wip)
 {
-	int flags = (int)data;
+	enable_data_t *ed = data;
 
 	assert(wip->inst != NULL);
 	assert(wip->pg == NULL);
@@ -1692,7 +1823,7 @@ set_fmri_enabled(void *data, scf_walkinfo_t *wip)
 			return (0);
 	}
 
-	if (flags & SET_RECURSIVE) {
+	if (ed->ed_flags & SET_RECURSIVE) {
 		char *fmri_buf = malloc(max_scf_fmri_sz);
 		if (fmri_buf == NULL)
 			uu_die(emsg_nomem);
@@ -1705,7 +1836,7 @@ set_fmri_enabled(void *data, scf_walkinfo_t *wip)
 		assert(strlen(wip->fmri) <= max_scf_fmri_sz);
 		(void) strlcpy(fmri_buf, wip->fmri, max_scf_fmri_sz);
 
-		switch (enable_fmri_rec(fmri_buf, (flags & SET_TEMPORARY))) {
+		switch (enable_fmri_rec(fmri_buf, ed)) {
 		case E2BIG:
 			uu_warn(gettext("operation on service %s is ambiguous; "
 			    "instance specification needed.\n"), fmri_buf);
@@ -1720,8 +1851,7 @@ set_fmri_enabled(void *data, scf_walkinfo_t *wip)
 		free(fmri_buf);
 
 	} else {
-		set_inst_enabled(wip->fmri, wip->inst,
-		    (flags & SET_TEMPORARY) != 0, (flags & SET_ENABLED) != 0);
+		set_inst_enabled(wip->fmri, wip->inst, ed);
 	}
 
 	return (0);
@@ -1972,7 +2102,6 @@ set_milestone(const char *fmri, boolean_t temporary)
 {
 	scf_instance_t *inst;
 	scf_propertygroup_t *pg;
-	int r;
 
 	if (temporary) {
 		set_astring_prop(SCF_SERVICE_STARTD, SCF_PG_OPTIONS_OVR,
@@ -1998,43 +2127,10 @@ set_milestone(const char *fmri, boolean_t temporary)
 	    SCF_PG_OPTIONS_TYPE, SCF_PG_OPTIONS_FLAGS, SCF_PROPERTY_MILESTONE,
 	    fmri);
 
-	r = scf_instance_delete_prop(inst, SCF_PG_OPTIONS_OVR,
-	    SCF_PROPERTY_MILESTONE);
-	switch (r) {
-	case 0:
-		break;
-
-	case ECANCELED:
-		uu_warn(emsg_no_service, fmri);
+	if (delete_prop(SCF_SERVICE_STARTD, inst, SCF_PG_OPTIONS_OVR,
+	    SCF_PROPERTY_MILESTONE) != 0)
 		exit_status = 1;
-		goto out;
 
-	case EPERM:
-		uu_warn(gettext("Could not delete %s/%s property of "
-		    "%s: permission denied.\n"), SCF_PG_OPTIONS_OVR,
-		    SCF_PROPERTY_MILESTONE, SCF_SERVICE_STARTD);
-		exit_status = 1;
-		goto out;
-
-	case EACCES:
-		uu_warn(gettext("Could not delete %s/%s property of "
-		    "%s: access denied.\n"), SCF_PG_OPTIONS_OVR,
-		    SCF_PROPERTY_MILESTONE, SCF_SERVICE_STARTD);
-		exit_status = 1;
-		goto out;
-
-	case EROFS:
-		uu_warn(gettext("Could not delete %s/%s property of "
-		    "%s: backend read-only.\n"), SCF_PG_OPTIONS_OVR,
-		    SCF_PROPERTY_MILESTONE, SCF_SERVICE_STARTD);
-		exit_status = 1;
-		goto out;
-
-	default:
-		bad_error("scf_instance_delete_prop", r);
-	}
-
-out:
 	scf_pg_destroy(pg);
 	scf_instance_destroy(inst);
 }
@@ -2315,7 +2411,10 @@ again:
 		usage();
 
 	if (strcmp(argv[optind], "enable") == 0) {
-		int flags = SET_ENABLED;
+		enable_data_t ed = {
+			.ed_flags = SET_ENABLED,
+			.ed_comment = ""
+		};
 		int wait = 0;
 		int error = 0;
 
@@ -2323,9 +2422,9 @@ again:
 
 		while ((o = getopt(argc, argv, "rst")) != -1) {
 			if (o == 'r')
-				flags |= SET_RECURSIVE;
+				ed.ed_flags |= SET_RECURSIVE;
 			else if (o == 't')
-				flags |= SET_TEMPORARY;
+				ed.ed_flags |= SET_TEMPORARY;
 			else if (o == 's')
 				wait = 1;
 			else if (o == '?')
@@ -2351,7 +2450,7 @@ again:
 		 * the errors the first time.
 		 */
 		if ((err = scf_walk_fmri(h, argc, argv, WALK_FLAGS,
-		    set_fmri_enabled, (void *)flags, &error, pr_warn)) != 0) {
+		    set_fmri_enabled, &ed, &error, pr_warn)) != 0) {
 
 			pr_warn(gettext("failed to iterate over "
 			    "instances: %s\n"), scf_strerror(err));
@@ -2359,7 +2458,7 @@ again:
 
 		} else if (wait && exit_status == 0 &&
 		    (err = scf_walk_fmri(h, argc, argv, WALK_FLAGS,
-		    wait_fmri_enabled, (void *)flags, &error, quiet)) != 0) {
+		    wait_fmri_enabled, NULL, &error, quiet)) != 0) {
 
 			pr_warn(gettext("failed to iterate over "
 			    "instances: %s\n"), scf_strerror(err));
@@ -2370,15 +2469,25 @@ again:
 			exit_status = error;
 
 	} else if (strcmp(argv[optind], "disable") == 0) {
-		int flags = 0;
+		enable_data_t ed = {
+			.ed_flags = 0,
+			.ed_comment = ""
+		};
 		int wait = 0;
 		int error = 0;
 
 		++optind;
 
-		while ((o = getopt(argc, argv, "st")) != -1) {
-			if (o == 't')
-				flags |= SET_TEMPORARY;
+		while ((o = getopt(argc, argv, "c:st")) != -1) {
+			if (o == 'c') {
+				if (strlcpy(ed.ed_comment, optarg,
+				    sizeof (ed.ed_comment)) >=
+				    sizeof (ed.ed_comment)) {
+					uu_die(gettext("disable -c comment "
+					    "too long.\n"));
+				}
+			} else if (o == 't')
+				ed.ed_flags |= SET_TEMPORARY;
 			else if (o == 's')
 				wait = 1;
 			else if (o == '?')
@@ -2404,7 +2513,7 @@ again:
 		 * the errors the first time.
 		 */
 		if ((err = scf_walk_fmri(h, argc, argv, WALK_FLAGS,
-		    set_fmri_enabled, (void *)flags, &exit_status,
+		    set_fmri_enabled, &ed, &exit_status,
 		    pr_warn)) != 0) {
 
 			pr_warn(gettext("failed to iterate over "
@@ -2413,7 +2522,7 @@ again:
 
 		} else if (wait && exit_status == 0 &&
 		    (err = scf_walk_fmri(h, argc, argv, WALK_FLAGS,
-		    wait_fmri_disabled, (void *)flags, &error, quiet)) != 0) {
+		    wait_fmri_disabled, NULL, &error, quiet)) != 0) {
 
 			pr_warn(gettext("failed to iterate over "
 			    "instances: %s\n"), scf_strerror(err));
diff --git a/usr/src/cmd/svc/svccfg/svccfg_libscf.c b/usr/src/cmd/svc/svccfg/svccfg_libscf.c
index 9fec777bd3..38dd45c75c 100644
--- a/usr/src/cmd/svc/svccfg/svccfg_libscf.c
+++ b/usr/src/cmd/svc/svccfg/svccfg_libscf.c
@@ -21,7 +21,7 @@
 
 /*
  * Copyright (c) 2004, 2010, Oracle and/or its affiliates. All rights reserved.
- * Copyright 2019 Joyent, Inc.
+ * Copyright 2020 Joyent, Inc.
  * Copyright 2012 Milan Jurik. All rights reserved.
  * Copyright 2017 RackTop Systems.
  * Copyright 2018 OmniOS Community Edition (OmniOSce) Association.
@@ -10522,6 +10522,8 @@ export_inst_general(scf_propertygroup_t *pg, xmlNodePtr inode,
 
 		if (strcmp(exp_str, scf_property_enabled) == 0) {
 			continue;
+		} else if (strcmp(exp_str, SCF_PROPERTY_COMMENT) == 0) {
+			continue;
 		} else if (strcmp(exp_str, SCF_PROPERTY_RESTARTER) == 0) {
 			xmlNodePtr rnode, sfnode;
 
diff --git a/usr/src/cmd/svc/svcs/explain.c b/usr/src/cmd/svc/svcs/explain.c
index 533437cbee..331a5375fd 100644
--- a/usr/src/cmd/svc/svcs/explain.c
+++ b/usr/src/cmd/svc/svcs/explain.c
@@ -22,7 +22,7 @@
 /*
  * Copyright 2009 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
- * Copyright (c) 2015, Joyent, Inc. All rights reserved.
+ * Copyright 2020 Joyent, Inc.
  */
 
 /*
@@ -133,6 +133,8 @@ typedef struct {
 	const char *restarter;
 	uu_list_t *dependencies;	/* list of dependency_group's */
 
+	char comment[SCF_COMMENT_MAX_LENGTH];
+
 	int active;			/* In use?  (cycle detection) */
 	int restarter_bad;
 	const char *summary;
@@ -403,7 +405,7 @@ add_instance(const char *svcname, const char *instname, scf_instance_t *inst)
 {
 	inst_t *instp;
 	svc_t *svcp;
-	int have_enabled = 0;
+	int ovr_set = 0;
 	uint8_t i;
 	uint32_t h;
 	int r;
@@ -479,7 +481,10 @@ add_instance(const char *svcname, const char *instname, scf_instance_t *inst)
 	if (scf_instance_get_pg(inst, SCF_PG_GENERAL_OVR, g_pg) == 0) {
 		if (pg_get_single_val(g_pg, SCF_PROPERTY_ENABLED,
 		    SCF_TYPE_BOOLEAN, &instp->enabled, 0, 0) == 0)
-			have_enabled = 1;
+			ovr_set = 1;
+		(void) pg_get_single_val(g_pg, SCF_PROPERTY_COMMENT,
+		    SCF_TYPE_ASTRING, instp->comment,
+		    sizeof (instp->comment), EMPTY_OK);
 	} else {
 		switch (scf_error()) {
 		case SCF_ERROR_NOT_FOUND:
@@ -508,11 +513,18 @@ add_instance(const char *svcname, const char *instname, scf_instance_t *inst)
 	if (pg_get_single_val(g_pg, SCF_PROPERTY_ENABLED, SCF_TYPE_BOOLEAN,
 	    &i, 0, 0) != 0)
 		return;
-	if (!have_enabled) {
+
+	if (ovr_set) {
+		instp->temporary = (instp->enabled != i);
+	} else {
 		instp->enabled = i;
 		instp->temporary = 0;
-	} else {
-		instp->temporary = (instp->enabled != i);
+	}
+
+	if (!instp->temporary) {
+		(void) pg_get_single_val(g_pg, SCF_PROPERTY_COMMENT,
+		    SCF_TYPE_ASTRING, instp->comment,
+		    sizeof (instp->comment), EMPTY_OK);
 	}
 
 	if (pg_get_single_val(g_pg, SCF_PROPERTY_RESTARTER, SCF_TYPE_ASTRING,
@@ -1736,12 +1748,23 @@ print_reasons(const inst_t *svcp, int verbose)
 
 	} else if (strcmp(svcp->state, SCF_STATE_STRING_DISABLED) == 0) {
 		if (!svcp->temporary) {
-			(void) puts(gettext(
-			    "Reason: Disabled by an administrator."));
+			if (svcp->comment[0] != '\0') {
+				(void) printf(gettext("Reason: Disabled by "
+				    "an administrator: %s\n"), svcp->comment);
+			} else {
+				(void) printf(gettext("Reason: Disabled by "
+				    "an administrator.\n"));
+			}
 			dc = DC_DISABLED;
 		} else {
-			(void) puts(gettext("Reason: "
-			    "Temporarily disabled by an administrator."));
+			if (svcp->comment[0] != '\0') {
+				(void) printf(gettext("Reason: Temporarily "
+				    "disabled by an administrator: %s\n"),
+				    svcp->comment);
+			} else {
+				(void) printf(gettext("Reason: Temporarily "
+				    "disabled by an administrator.\n"));
+			}
 			dc = DC_TEMPDISABLED;
 		}
 
diff --git a/usr/src/cmd/svc/svcs/svcs.c b/usr/src/cmd/svc/svcs/svcs.c
index ec3dba52d1..b6cf4f7fd1 100644
--- a/usr/src/cmd/svc/svcs/svcs.c
+++ b/usr/src/cmd/svc/svcs/svcs.c
@@ -21,7 +21,7 @@
 
 /*
  * Copyright (c) 2004, 2010, Oracle and/or its affiliates. All rights reserved.
- * Copyright 2019 Joyent, Inc.
+ * Copyright 2020 Joyent, Inc.
  * Copyright (c) 2015, 2016 by Delphix. All rights reserved.
  */
 
@@ -87,10 +87,6 @@
 
 #define	LEGACY_UNKNOWN	"unknown"
 
-/* Flags for pg_get_single_val() */
-#define	EMPTY_OK	0x01
-#define	MULTI_OK	0x02
-
 /*
  * Per proc(4) when pr_nlwp, pr_nzomb, and pr_lwp.pr_lwpid are all 0,
  * the process is a zombie.
@@ -409,7 +405,13 @@ pg_get_single_val(scf_propertygroup_t *pg, const char *propname, scf_type_t ty,
 
 	switch (ty) {
 	case SCF_TYPE_ASTRING:
-		r = scf_value_get_astring(g_val, vp, sz) > 0 ? SCF_SUCCESS : -1;
+		r = scf_value_get_astring(g_val, vp, sz);
+		if (r == 0 && !(flags & EMPTY_OK)) {
+			uu_die(gettext("Unexpected empty string for property "
+			    "%s.  Exiting.\n"), propname);
+		}
+		if (r >= 0)
+			r = SCF_SUCCESS;
 		break;
 
 	case SCF_TYPE_BOOLEAN:
@@ -2477,6 +2479,21 @@ print_detailed(void *unused, scf_walkinfo_t *wip)
 		    perm ? gettext("true") : gettext("false"));
 	}
 
+	if (temp == 0 || (temp == -1 && perm == 0)) {
+		char comment[SCF_COMMENT_MAX_LENGTH] = "";
+		const char *pg = (temp != -1 && temp != perm) ?
+		    SCF_PG_GENERAL_OVR : SCF_PG_GENERAL;
+
+		(void) inst_get_single_val(wip->inst, pg, SCF_PROPERTY_COMMENT,
+		    SCF_TYPE_ASTRING, &comment, sizeof (comment),
+		    EMPTY_OK, 0, 0);
+
+		if (comment[0] != '\0') {
+			printf(fmt, DETAILED_WIDTH, gettext("comment"),
+			    comment);
+		}
+	}
+
 	/*
 	 * Property values may be longer than max_scf_fmri_length, but these
 	 * shouldn't be, so we'll just reuse buf.  The user can use svcprop if
diff --git a/usr/src/cmd/svc/svcs/svcs.h b/usr/src/cmd/svc/svcs/svcs.h
index 5f0cb83360..78b3c26dd3 100644
--- a/usr/src/cmd/svc/svcs/svcs.h
+++ b/usr/src/cmd/svc/svcs/svcs.h
@@ -22,13 +22,13 @@
 /*
  * Copyright 2004 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
+ *
+ * Copyright 2020 Joyent, Inc.
  */
 
 #ifndef	_SVCS_H
 #define	_SVCS_H
 
-#pragma ident	"%Z%%M%	%I%	%E% SMI"
-
 #include <libscf.h>
 
 #ifdef	__cplusplus
@@ -52,6 +52,10 @@ void scfdie(void);
 void *safe_malloc(size_t);
 char *safe_strdup(const char *);
 
+/* Flags for pg_get_single_val() */
+#define	EMPTY_OK	0x01
+#define	MULTI_OK	0x02
+
 int pg_get_single_val(scf_propertygroup_t *, const char *, scf_type_t, void *,
     size_t, uint_t);
 int inst_get_single_val(scf_instance_t *, const char *, const char *,
diff --git a/usr/src/lib/libscf/common/mapfile-vers b/usr/src/lib/libscf/common/mapfile-vers
index 049912185c..c9616640de 100644
--- a/usr/src/lib/libscf/common/mapfile-vers
+++ b/usr/src/lib/libscf/common/mapfile-vers
@@ -20,6 +20,7 @@
 #
 #
 # Copyright (c) 2006, 2010, Oracle and/or its affiliates. All rights reserved.
+# Copyright 2020 Joyent, Inc.
 #
 
 #
@@ -38,6 +39,11 @@
 
 $mapfile_version 2
 
+SYMBOL_VERSION ILLUMOS_0.1 {
+    global:
+	smf_disable_instance_with_comment;
+} SUNW_1.2;
+
 SYMBOL_VERSION SUNW_1.2 {
     global:
         scf_count_ranges_destroy;
diff --git a/usr/src/lib/libscf/common/midlevel.c b/usr/src/lib/libscf/common/midlevel.c
index 3c75364548..03eda8a4e8 100644
--- a/usr/src/lib/libscf/common/midlevel.c
+++ b/usr/src/lib/libscf/common/midlevel.c
@@ -23,6 +23,7 @@
  * Copyright (c) 2004, 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright 2013 Nexenta Systems, Inc.  All rights reserved.
  * Copyright 2018 RackTop Systems.
+ * Copyright 2020 Joyent, Inc.
  */
 
 #include "libscf_impl.h"
@@ -512,16 +513,21 @@ out:
  * instance and the desired state for the enabled bit in the instance's
  * named property group.  If the group doesn't exist, it's created with the
  * given flags.  Called by smf_{dis,en}able_instance().
+ *
+ * Note that if we're enabling, comment will be "", and we use that to clear out
+ * any old disabled comment.
  */
 static int
 set_inst_enabled(const scf_instance_t *inst, uint8_t desired,
-    const char *pgname, uint32_t pgflags)
+    const char *pgname, uint32_t pgflags, const char *comment)
 {
 	scf_transaction_t	*tx = NULL;
-	scf_transaction_entry_t *ent = NULL;
+	scf_transaction_entry_t *ent1 = NULL;
+	scf_transaction_entry_t *ent2 = NULL;
 	scf_propertygroup_t	*gpg = NULL;
 	scf_property_t		*eprop = NULL;
-	scf_value_t		*v = NULL;
+	scf_value_t		*v1 = NULL;
+	scf_value_t		*v2 = NULL;
 	scf_handle_t		*h = NULL;
 	int			ret = -1;
 	int			committed;
@@ -532,9 +538,11 @@ set_inst_enabled(const scf_instance_t *inst, uint8_t desired,
 
 	if ((gpg = scf_pg_create(h)) == NULL ||
 	    (eprop = scf_property_create(h)) == NULL ||
-	    (v = scf_value_create(h)) == NULL ||
+	    (v1 = scf_value_create(h)) == NULL ||
+	    (v2 = scf_value_create(h)) == NULL ||
 	    (tx = scf_transaction_create(h)) == NULL ||
-	    (ent = scf_entry_create(h)) == NULL)
+	    (ent1 = scf_entry_create(h)) == NULL ||
+	    (ent2 = scf_entry_create(h)) == NULL)
 		goto out;
 
 general_pg_get:
@@ -575,7 +583,7 @@ get:
 	/*
 	 * If it's already set the way we want, forgo the transaction.
 	 */
-	if (scf_property_get_value(eprop, v) == -1) {
+	if (scf_property_get_value(eprop, v1) == -1) {
 		switch (scf_error()) {
 		case SCF_ERROR_CONSTRAINT_VIOLATED:
 		case SCF_ERROR_NOT_FOUND:
@@ -586,7 +594,7 @@ get:
 			goto out;
 		}
 	}
-	if (scf_value_get_boolean(v, &b) == -1) {
+	if (scf_value_get_boolean(v1, &b) == -1) {
 		if (scf_error() != SCF_ERROR_TYPE_MISMATCH)
 			goto out;
 		goto set;
@@ -601,7 +609,7 @@ set:
 		if (scf_transaction_start(tx, gpg) == -1)
 			goto out;
 
-		if (transaction_property_set(tx, ent, SCF_PROPERTY_ENABLED,
+		if (transaction_property_set(tx, ent1, SCF_PROPERTY_ENABLED,
 		    SCF_TYPE_BOOLEAN) != 0) {
 			switch (scf_error()) {
 			case SCF_ERROR_CONNECTION_BROKEN:
@@ -618,8 +626,31 @@ set:
 			}
 		}
 
-		scf_value_set_boolean(v, desired);
-		if (scf_entry_add_value(ent, v) == -1)
+		scf_value_set_boolean(v1, desired);
+		if (scf_entry_add_value(ent1, v1) == -1)
+			goto out;
+
+		if (transaction_property_set(tx, ent2,
+		    SCF_PROPERTY_COMMENT, SCF_TYPE_ASTRING) != 0) {
+			switch (scf_error()) {
+			case SCF_ERROR_CONNECTION_BROKEN:
+			case SCF_ERROR_DELETED:
+			default:
+				goto out;
+
+			case SCF_ERROR_HANDLE_MISMATCH:
+			case SCF_ERROR_INVALID_ARGUMENT:
+			case SCF_ERROR_NOT_BOUND:
+			case SCF_ERROR_NOT_SET:
+				bad_error("transaction_property_set",
+				    scf_error());
+			}
+		}
+
+		if (scf_value_set_astring(v2, comment) == -1)
+			goto out;
+
+		if (scf_entry_add_value(ent2, v2) == -1)
 			goto out;
 
 		committed = scf_transaction_commit(tx);
@@ -637,8 +668,10 @@ set:
 	ret = 0;
 
 out:
-	scf_value_destroy(v);
-	scf_entry_destroy(ent);
+	scf_value_destroy(v1);
+	scf_value_destroy(v2);
+	scf_entry_destroy(ent1);
+	scf_entry_destroy(ent2);
 	scf_transaction_destroy(tx);
 	scf_property_destroy(eprop);
 	scf_pg_destroy(gpg);
@@ -650,7 +683,8 @@ static int
 delete_inst_enabled(const scf_instance_t *inst, const char *pgname)
 {
 	scf_transaction_t	*tx = NULL;
-	scf_transaction_entry_t *ent = NULL;
+	scf_transaction_entry_t *ent1 = NULL;
+	scf_transaction_entry_t *ent2 = NULL;
 	scf_propertygroup_t	*gpg = NULL;
 	scf_handle_t		*h = NULL;
 	int			ret = -1;
@@ -661,16 +695,31 @@ delete_inst_enabled(const scf_instance_t *inst, const char *pgname)
 
 	if ((gpg = scf_pg_create(h)) == NULL ||
 	    (tx = scf_transaction_create(h)) == NULL ||
-	    (ent = scf_entry_create(h)) == NULL)
+	    (ent1 = scf_entry_create(h)) == NULL ||
+	    (ent2 = scf_entry_create(h)) == NULL)
 		goto out;
 
 	if (scf_instance_get_pg(inst, pgname, gpg) != 0)
 		goto error;
 	do {
-		if (scf_transaction_start(tx, gpg) == -1 ||
-		    scf_transaction_property_delete(tx, ent,
-		    SCF_PROPERTY_ENABLED) == -1 ||
-		    (committed = scf_transaction_commit(tx)) == -1)
+		if (scf_transaction_start(tx, gpg) == -1)
+			goto error;
+
+		ret = scf_transaction_property_delete(tx, ent1,
+		    SCF_PROPERTY_ENABLED);
+
+		if (ret == -1 && scf_error() != SCF_ERROR_DELETED &&
+		    scf_error() != SCF_ERROR_NOT_FOUND)
+			goto error;
+
+		ret = scf_transaction_property_delete(tx, ent2,
+		    SCF_PROPERTY_COMMENT);
+
+		if (ret == -1 && scf_error() != SCF_ERROR_DELETED &&
+		    scf_error() != SCF_ERROR_NOT_FOUND)
+			goto error;
+
+		if ((committed = scf_transaction_commit(tx)) == -1)
 			goto error;
 
 		scf_transaction_reset(tx);
@@ -691,7 +740,8 @@ error:
 	}
 
 out:
-	scf_entry_destroy(ent);
+	scf_entry_destroy(ent1);
+	scf_entry_destroy(ent2);
 	scf_transaction_destroy(tx);
 	scf_pg_destroy(gpg);
 
@@ -1016,7 +1066,8 @@ out:
  * present state, if it is different.
  */
 static int
-set_inst_enabled_atboot(scf_instance_t *inst, uint8_t desired)
+set_inst_enabled_atboot(scf_instance_t *inst, uint8_t desired,
+    const char *comment)
 {
 	int enabled;
 	int persistent;
@@ -1034,13 +1085,14 @@ set_inst_enabled_atboot(scf_instance_t *inst, uint8_t desired)
 			 * Temporarily store the present enabled state.
 			 */
 			if (set_inst_enabled(inst, persistent,
-			    SCF_PG_GENERAL_OVR, SCF_PG_GENERAL_OVR_FLAGS))
+			    SCF_PG_GENERAL_OVR, SCF_PG_GENERAL_OVR_FLAGS,
+			    comment))
 				goto out;
 		}
 	}
 	if (persistent != desired)
 		if (set_inst_enabled(inst, desired, SCF_PG_GENERAL,
-		    SCF_PG_GENERAL_FLAGS))
+		    SCF_PG_GENERAL_FLAGS, comment))
 			goto out;
 	if (enabled == desired)
 		ret = delete_inst_enabled(inst, SCF_PG_GENERAL_OVR);
@@ -1052,7 +1104,8 @@ out:
 }
 
 static int
-set_inst_enabled_flags(const char *fmri, int flags, uint8_t desired)
+set_inst_enabled_flags(const char *fmri, int flags, uint8_t desired,
+    const char *comment)
 {
 	int ret = -1;
 	scf_handle_t *h;
@@ -1080,11 +1133,11 @@ set_inst_enabled_flags(const char *fmri, int flags, uint8_t desired)
 	}
 
 	if (flags & SMF_AT_NEXT_BOOT) {
-		ret = set_inst_enabled_atboot(inst, desired);
+		ret = set_inst_enabled_atboot(inst, desired, comment);
 	} else {
 		if (set_inst_enabled(inst, desired, flags & SMF_TEMPORARY ?
 		    SCF_PG_GENERAL_OVR : SCF_PG_GENERAL, flags & SMF_TEMPORARY ?
-		    SCF_PG_GENERAL_OVR_FLAGS : SCF_PG_GENERAL_FLAGS))
+		    SCF_PG_GENERAL_OVR_FLAGS : SCF_PG_GENERAL_FLAGS, comment))
 			goto out;
 
 		/*
@@ -1150,16 +1203,22 @@ get_instance_pg(scf_simple_handle_t *simple_h)
 int
 smf_enable_instance(const char *fmri, int flags)
 {
-	return (set_inst_enabled_flags(fmri, flags, B_TRUE));
+	return (set_inst_enabled_flags(fmri, flags, B_TRUE, ""));
 }
 
 int
-smf_disable_instance(const char *fmri, int flags)
+smf_disable_instance_with_comment(const char *fmri, int flags,
+    const char *comment)
 {
-	return (set_inst_enabled_flags(fmri, flags, B_FALSE));
+	return (set_inst_enabled_flags(fmri, flags, B_FALSE, comment));
 }
 
 int
+smf_disable_instance(const char *fmri, int flags)
+{
+	return (set_inst_enabled_flags(fmri, flags, B_FALSE, ""));
+}
+int
 _smf_refresh_instance_i(scf_instance_t *inst)
 {
 	return (set_inst_action_inst(inst, SCF_PROPERTY_REFRESH));
diff --git a/usr/src/lib/libscf/inc/libscf.h b/usr/src/lib/libscf/inc/libscf.h
index 773c0ca1d6..b2730eb810 100644
--- a/usr/src/lib/libscf/inc/libscf.h
+++ b/usr/src/lib/libscf/inc/libscf.h
@@ -22,7 +22,7 @@
 /*
  * Copyright (c) 2004, 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright 2016 RackTop Systems.
- * Copyright (c) 2012, Joyent, Inc. All rights reserved.
+ * Copyright 2020 Joyent, Inc.
  */
 
 #ifndef	_LIBSCF_H
@@ -326,6 +326,7 @@ typedef struct {
 #define	SCF_PROPERTY_AUX_STATE			"auxiliary_state"
 #define	SCF_PROPERTY_AUX_FMRI			"auxiliary_fmri"
 #define	SCF_PROPERTY_AUX_TTY			"auxiliary_tty"
+#define	SCF_PROPERTY_COMMENT			"comment"
 #define	SCF_PROPERTY_CONTRACT			"contract"
 #define	SCF_PROPERTY_COREFILE_PATTERN		"corefile_pattern"
 #define	SCF_PROPERTY_DEGRADED			"degraded"
@@ -485,6 +486,9 @@ ssize_t scf_limit(uint32_t code);
 #define	SCF_LIMIT_MAX_PG_TYPE_LENGTH	-2002U
 #define	SCF_LIMIT_MAX_FMRI_LENGTH	-2003U
 
+
+#define	SCF_COMMENT_MAX_LENGTH		(1024)
+
 scf_handle_t *scf_handle_create(scf_version_t);
 
 int scf_handle_decorate(scf_handle_t *, const char *, scf_value_t *);
@@ -807,6 +811,7 @@ int scf_tmpl_error_value(const scf_tmpl_error_t *, char **);
  * Simplified calls
  */
 int smf_enable_instance(const char *, int);
+int smf_disable_instance_with_comment(const char *, int, const char *);
 int smf_disable_instance(const char *, int);
 int smf_refresh_instance(const char *);
 int smf_restart_instance(const char *);
diff --git a/usr/src/man/man1/svcs.1 b/usr/src/man/man1/svcs.1
index a7b9d1b5be..cb8b3cd120 100644
--- a/usr/src/man/man1/svcs.1
+++ b/usr/src/man/man1/svcs.1
@@ -1,13 +1,13 @@
 '\" te
 .\" Copyright (c) 2008, Sun Microsystems, Inc. All Rights Reserved
+.\" Copyright 2020 Joyent, Inc.
 .\" The contents of this file are subject to the terms of the Common Development and Distribution License (the "License").  You may not use this file except in compliance with the License.
 .\" You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE or http://www.opensolaris.org/os/licensing.  See the License for the specific language governing permissions and limitations under the License.
 .\" When distributing Covered Code, include this CDDL HEADER in each file and include the License file at usr/src/OPENSOLARIS.LICENSE.  If applicable, add the following below this CDDL HEADER, with the fields enclosed by brackets "[]" replaced with your own identifying information: Portions Copyright [yyyy] [name of copyright owner]
-.TH SVCS 1 "Apr 17, 2011"
+.TH SVCS 1 "May 11, 2020"
 .SH NAME
 svcs \- report service status
 .SH SYNOPSIS
-.LP
 .nf
 \fBsvcs\fR [\fB-aHpv?\fR] [\fB-Z\fR | \fB-z\fR \fIzone\fR] [\fB-o\fR \fIcol\fR[,\fIcol\fR]]... [\fB-R\fR \fIFMRI-instance\fR]...
      [\fB-sS\fR \fIcol\fR]... [\fIFMRI\fR | \fIpattern\fR]...
@@ -30,8 +30,6 @@ svcs \- report service status
 .fi
 
 .SH DESCRIPTION
-.sp
-.LP
 The \fBsvcs\fR command displays information about service instances as recorded
 in the service configuration repository.
 .sp
@@ -62,8 +60,6 @@ Error messages are printed to the standard error stream.
 The output of this command can be used appropriately as input to the
 \fBsvcadm\fR(1M) command.
 .SH OPTIONS
-.sp
-.LP
 The following options are supported:
 .sp
 .ne 2
@@ -212,6 +208,10 @@ temporarily (until the next system reboot). The former is specified as either
 A service might be temporarily disabled because an administrator has run
 \fBsvcadm disable -t\fR, used \fBsvcadm milestone\fR, or booted the system to a
 specific milestone. See \fBsvcadm\fR(1M) for details.
+.sp
+If a service instance was disabled via
+\fBsvcadm disable -c\fR
+, then the provided comment will also be displayed.
 .RE
 
 .RE
@@ -337,8 +337,6 @@ zone, see \fBzones\fR(5).
 .RE
 
 .SH OPERANDS
-.sp
-.LP
 The following operands are supported:
 .sp
 .ne 2
@@ -421,8 +419,6 @@ An FMRI that specifies an instance.
 .RE
 
 .SH COLUMNS
-.sp
-.LP
 Column names are case insensitive. The default output format is equivalent to
 "\fB-o\fR \fBstate,stime,fmri\fR". The default sorting columns are \fBSTATE\fR,
 \fBSTIME\fR, \fBFMRI\fR.
@@ -607,7 +603,6 @@ blanks.
 .RE
 
 .SH EXAMPLES
-.LP
 \fBExample 1 \fRDisplaying the Default Output
 .sp
 .LP
@@ -767,8 +762,6 @@ Impact: This service is not running.
 .sp
 
 .SH EXIT STATUS
-.sp
-.LP
 The following exit values are returned:
 .sp
 .ne 2
@@ -798,8 +791,6 @@ Invalid command line options were specified.
 .RE
 
 .SH ATTRIBUTES
-.sp
-.LP
 See \fBattributes\fR(5) for descriptions of the following attributes:
 .sp
 
@@ -817,8 +808,6 @@ Interface Stability	See below.
 .LP
 Screen output is Uncommitted. The invocation is Committed.
 .SH SEE ALSO
-.sp
-.LP
 \fBps\fR(1), \fBsvcprop\fR(1), \fBsvcadm\fR(1M), \fBsvccfg\fR(1M),
 \fBsvc.startd\fR(1M), \fBstat\fR(2), \fBlibscf\fR(3LIB), \fBattributes\fR(5),
 \fBfnmatch\fR(5), \fBsmf\fR(5), \fBzones\fR(5)
diff --git a/usr/src/man/man1m/svcadm.1m b/usr/src/man/man1m/svcadm.1m
index 4bfefe5eb3..550e093239 100644
--- a/usr/src/man/man1m/svcadm.1m
+++ b/usr/src/man/man1m/svcadm.1m
@@ -1,10 +1,10 @@
 '\" te
 .\" Copyright (c) 2008, Sun Microsystems, Inc. All Rights Reserved.
-.\" Copyright 2019 Joyent, Inc.
+.\" Copyright 2020 Joyent, Inc.
 .\" The contents of this file are subject to the terms of the Common Development and Distribution License (the "License").  You may not use this file except in compliance with the License.
 .\" You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE or http://www.opensolaris.org/os/licensing.  See the License for the specific language governing permissions and limitations under the License.
 .\" When distributing Covered Code, include this CDDL HEADER in each file and include the License file at usr/src/OPENSOLARIS.LICENSE.  If applicable, add the following below this CDDL HEADER, with the fields enclosed by brackets "[]" replaced with your own identifying information: Portions Copyright [yyyy] [name of copyright owner]
-.TH SVCADM 1M "Oct 24, 2019"
+.TH SVCADM 1M "May 11, 2020"
 .SH NAME
 svcadm \- manipulate service instances
 .SH SYNOPSIS
@@ -15,8 +15,8 @@ svcadm \- manipulate service instances
 
 .LP
 .nf
-\fB/usr/sbin/svcadm\fR [\fB-S state\fR] [\fB-v\fR] [\fB-Z\fR | \fB-z\fR \fIzone\fR] disable [\fB-st\fR]
-     [{\fIFMRI\fR | \fIpattern\fR}...]
+\fB/usr/sbin/svcadm\fR [\fB-S state\fR] [\fB-v\fR] [\fB-Z\fR | \fB-z\fR \fIzone\fR] disable
+     [\fB-c\fR \fIcomment\fR] [\fB-st\fR] [{\fIFMRI\fR | \fIpattern\fR}...]
 .fi
 
 .LP
@@ -142,7 +142,7 @@ persistent across reboot.
 .sp
 .ne 2
 .na
-\fB\fBdisable\fR [\fB-st\fR] {\fIFMRI\fR | \fIpattern\fR}...\fR
+\fB\fBdisable\fR [\fB-c\fR \fIcomment\fR] [\fB-st\fR] {\fIFMRI\fR | \fIpattern\fR}...\fR
 .ad
 .sp .6
 .RS 4n
@@ -161,6 +161,11 @@ service instance. Temporary disable only lasts until reboot. This action
 requires permission to modify the "restarter_actions" property group of the
 service instance (see \fBsmf_security\fR(5)). By default, \fBdisable\fR is
 persistent across reboot.
+.sp
+If the \fB-c\fR option is specified, the given free-form comment is recorded
+in the (temporary) service configuration under the \fBgeneral/comment\fR
+property.  This can serve as an administrator reason for disabling the service,
+and is reported by \fBsvcs\fR(1).
 .RE
 
 .sp
@@ -379,16 +384,16 @@ However, you can abbreviate the full \fIFMRI\fR as follows:
 .sp
 
 .LP
-\fBExample 2 \fRDisabling the Standard HTTP Server
+\fBExample 2 \fRDisabling a service
 .sp
 .LP
-The following command disables the standard HTTP server, using an abbreviated
-\fIFMRI\fR:
+The following command synchronously disables a service, using an abbreviated
+\fIFMRI\fR, and recording a ticket ID as the administrative reason:
 
 .sp
 .in +2
 .nf
-$ svcadm disable http
+$ svcadm disable -c "OPS-1000" -s http
 .fi
 .in -2
 .sp
diff --git a/usr/src/man/man3scf/Makefile b/usr/src/man/man3scf/Makefile
index 088037ad15..cbebef7221 100644
--- a/usr/src/man/man3scf/Makefile
+++ b/usr/src/man/man3scf/Makefile
@@ -12,35 +12,36 @@
 #
 # Copyright 2011, Richard Lowe
 # Copyright 2013 Nexenta Systems, Inc.  All rights reserved.
+# Copyright 2020 Joyent, Inc.
 #
 
 include		$(SRC)/Makefile.master
 
-MANSECT= 	3scf
+MANSECT=	3scf
 
-MANFILES= 	scf_entry_create.3scf		\
-	 	scf_error.3scf			\
-	 	scf_handle_create.3scf		\
-	 	scf_handle_decode_fmri.3scf	\
-	 	scf_instance_create.3scf	\
-	 	scf_iter_create.3scf		\
-	 	scf_limit.3scf			\
-	 	scf_pg_create.3scf		\
-	 	scf_property_create.3scf	\
-	 	scf_scope_create.3scf		\
-	 	scf_service_create.3scf		\
-	 	scf_simple_prop_get.3scf	\
-	 	scf_simple_walk_instances.3scf	\
-	 	scf_snaplevel_create.3scf	\
-	 	scf_snapshot_create.3scf	\
-	 	scf_tmpl_pg_create.3scf		\
-	 	scf_tmpl_pg_name.3scf		\
-	 	scf_tmpl_prop_create.3scf	\
-	 	scf_tmpl_prop_name.3scf		\
-	 	scf_tmpl_validate_fmri.3scf	\
-	 	scf_transaction_create.3scf	\
-	 	scf_value_create.3scf		\
-	 	smf_enable_instance.3scf
+MANFILES=	scf_entry_create.3scf		\
+		scf_error.3scf			\
+		scf_handle_create.3scf		\
+		scf_handle_decode_fmri.3scf	\
+		scf_instance_create.3scf	\
+		scf_iter_create.3scf		\
+		scf_limit.3scf			\
+		scf_pg_create.3scf		\
+		scf_property_create.3scf	\
+		scf_scope_create.3scf		\
+		scf_service_create.3scf		\
+		scf_simple_prop_get.3scf	\
+		scf_simple_walk_instances.3scf	\
+		scf_snaplevel_create.3scf	\
+		scf_snapshot_create.3scf	\
+		scf_tmpl_pg_create.3scf		\
+		scf_tmpl_pg_name.3scf		\
+		scf_tmpl_prop_create.3scf	\
+		scf_tmpl_prop_name.3scf		\
+		scf_tmpl_validate_fmri.3scf	\
+		scf_transaction_create.3scf	\
+		scf_value_create.3scf		\
+		smf_enable_instance.3scf
 
 
 MANLINKS=	scf_count_ranges_destroy.3scf			\
@@ -241,6 +242,7 @@ MANLINKS=	scf_count_ranges_destroy.3scf			\
 		scf_values_destroy.3scf				\
 		smf_degrade_instance.3scf			\
 		smf_disable_instance.3scf			\
+		smf_disable_instance_with_comment.3scf		\
 		smf_get_state.3scf				\
 		smf_maintain_instance.3scf			\
 		smf_refresh_instance.3scf			\
@@ -465,6 +467,7 @@ scf_value_type.3scf				:= LINKSRC = scf_value_create.3scf
 
 smf_degrade_instance.3scf			:= LINKSRC = smf_enable_instance.3scf
 smf_disable_instance.3scf			:= LINKSRC = smf_enable_instance.3scf
+smf_disable_instance_with_comment.3scf		:= LINKSRC = smf_enable_instance.3scf
 smf_get_state.3scf				:= LINKSRC = smf_enable_instance.3scf
 smf_maintain_instance.3scf			:= LINKSRC = smf_enable_instance.3scf
 smf_refresh_instance.3scf			:= LINKSRC = smf_enable_instance.3scf
diff --git a/usr/src/man/man3scf/smf_enable_instance.3scf b/usr/src/man/man3scf/smf_enable_instance.3scf
index 6e8663f77f..9333b16265 100644
--- a/usr/src/man/man3scf/smf_enable_instance.3scf
+++ b/usr/src/man/man3scf/smf_enable_instance.3scf
@@ -1,16 +1,16 @@
 '\" te
 .\" Copyright (c) 2007, Sun Microsystems, Inc. All Rights Reserved.
+.\" Copyright 2020 Joyent, Inc.
 .\" The contents of this file are subject to the terms of the Common Development and Distribution License (the "License").  You may not use this file except in compliance with the License.
 .\" You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE or http://www.opensolaris.org/os/licensing.  See the License for the specific language governing permissions and limitations under the License.
 .\" When distributing Covered Code, include this CDDL HEADER in each file and include the License file at usr/src/OPENSOLARIS.LICENSE.  If applicable, add the following below this CDDL HEADER, with the fields enclosed by brackets "[]" replaced with your own identifying information: Portions Copyright [yyyy] [name of copyright owner]
-.TH SMF_ENABLE_INSTANCE 3SCF "Nov 5, 2007"
+.TH SMF_ENABLE_INSTANCE 3SCF "May 15, 2020"
 .SH NAME
-smf_enable_instance, smf_disable_instance, smf_refresh_instance,
-smf_restart_instance, smf_maintain_instance, smf_degrade_instance,
-smf_restore_instance, smf_get_state \- administrative interface to the Service
-Configuration Facility
+smf_enable_instance, smf_disable_instance, smf_disable_instance_with_comment,
+smf_refresh_instance, smf_restart_instance, smf_maintain_instance,
+smf_degrade_instance, smf_restore_instance, smf_get_state \-
+administrative interface to the Service Configuration Facility
 .SH SYNOPSIS
-.LP
 .nf
 cc [ \fIflag\fR\&.\|.\|. ] \fIfile\fR\&.\|.\|. \fB-lscf\fR [ \fIlibrary\fR\&.\|.\|. ]
 #include <libscf.h>
@@ -25,6 +25,12 @@ cc [ \fIflag\fR\&.\|.\|. ] \fIfile\fR\&.\|.\|. \fB-lscf\fR [ \fIlibrary\fR\&.\|.
 
 .LP
 .nf
+\fBint\fR \fBsmf_disable_instance_with_comment\fR(\fBconst char *\fR\fIinstance\fR,
+    \fBint\fR \fIflags\fR, \fBconst char *\fR\fIcomment\fR);
+.fi
+
+.LP
+.nf
 \fBint\fR \fBsmf_refresh_instance\fR(\fBconst char *\fR\fIinstance\fR);
 .fi
 
@@ -54,8 +60,6 @@ cc [ \fIflag\fR\&.\|.\|. ] \fIfile\fR\&.\|.\|. \fB-lscf\fR [ \fIlibrary\fR\&.\|.
 .fi
 
 .SH DESCRIPTION
-.sp
-.LP
 These functions provide administrative control over service instances. Using
 these functions, an administrative tool can make a request to enable, disable,
 refresh, or restart an instance. All calls are asynchronous. They request an
@@ -77,6 +81,13 @@ the current system instance. The \fIflags\fR argument is set to \fB0\fR if no
 flags are to be use.
 .sp
 .LP
+The \fBsmf_disable_instance_with_comment()\fR function behaves the same as
+\fBsmf_disable_instance()\fR, except the given free-form comment is recorded
+under the \fIcomment\fR property, as reported by \fBsvcs\fR(1).
+The comment may be up to \fBSCF_COMMENT_MAX_LENGTH\fR characters including
+the NUL terminator.
+.sp
+.LP
 The \fBsmf_refresh_instance()\fR function causes the service instance specified
 by \fIinstance\fR FMRI to re-read its configuration information.
 .sp
@@ -123,20 +134,17 @@ string. Possible state strings are defined as the following:
 .in -2
 
 .SH RETURN VALUES
-.sp
-.LP
 Upon successful completion, \fBsmf_enable_instance()\fR,
-\fBsmf_disable_instance()\fR, \fBsmf_refresh_instance()\fR,
-\fBsmf_restart_instance()\fR, \fBsmf_maintain_instance()\fR,
-\fBsmf_degrade_instance()\fR, and \fBsmf_restore_instance()\fR return \fB0\fR.
+\fBsmf_disable_instance()\fR, \fBsmf_disable_instance_with_comment()\fR,
+\fBsmf_refresh_instance()\fR, \fBsmf_restart_instance()\fR,
+\fBsmf_maintain_instance()\fR, \fBsmf_degrade_instance()\fR,
+and \fBsmf_restore_instance()\fR return \fB0\fR.
 Otherwise, they return \fB-1\fR\&.
 .sp
 .LP
 Upon successful completion, smf_get_state returns an allocated string.
 Otherwise, it returns \fINULL\fR.
 .SH ERRORS
-.sp
-.LP
 These functions will fail if:
 .sp
 .ne 2
@@ -241,8 +249,6 @@ The function is called on an instance in an inappropriate state.
 .LP
 The \fBscf_error\fR(3SCF) function can be used to retrieve the error value.
 .SH ATTRIBUTES
-.sp
-.LP
 See \fBattributes\fR(5) for descriptions of the following attributes:
 .sp
 
@@ -259,7 +265,5 @@ MT-Level	Safe
 .TE
 
 .SH SEE ALSO
-.sp
-.LP
 \fBsvc.startd\fR(1M), \fBlibscf\fR(3LIB), \fBscf_error\fR(3SCF),
-\fBattributes\fR(5), \fBsmf_security\fR(5)
+\fBattributes\fR(5), \fBsmf_security\fR(5), \fBsvcs\fR(1)
diff --git a/usr/src/pkg/manifests/system-library.man3scf.inc b/usr/src/pkg/manifests/system-library.man3scf.inc
index d26af9e51b..7d8c7380cd 100644
--- a/usr/src/pkg/manifests/system-library.man3scf.inc
+++ b/usr/src/pkg/manifests/system-library.man3scf.inc
@@ -12,6 +12,7 @@
 #
 # Copyright 2011, Richard Lowe
 # Copyright 2012 Nexenta Systems, Inc. All rights reserved.
+# Copyright 2020 Joyent, Inc.
 #
 
 file path=usr/share/man/man3scf/scf_entry_create.3scf
@@ -425,6 +426,8 @@ link path=usr/share/man/man3scf/smf_degrade_instance.3scf \
     target=smf_enable_instance.3scf
 link path=usr/share/man/man3scf/smf_disable_instance.3scf \
     target=smf_enable_instance.3scf
+link path=usr/share/man/man3scf/smf_disable_instance_with_comment.3scf \
+    target=smf_enable_instance.3scf
 link path=usr/share/man/man3scf/smf_get_state.3scf \
     target=smf_enable_instance.3scf
 link path=usr/share/man/man3scf/smf_maintain_instance.3scf \