6215238 svc.startd could provide better log messages for faulty method_context contents

6469250 Service state is reset on first boot after a new netservices change
6816965 svccfg core dumps when a service, not an instance, is used for a restarter
6816948 svccfg does not see an alternate door setup on a refresh
6825048 svccfg validation is not consistent between manifest files and services when dealing with subtypes

3 files changed, 381 insertions, 178 deletions

diff --git a/usr/src/lib/librestart/common/librestart.c b/usr/src/lib/librestart/common/librestart.c
index ab05166758..a455519fe7 100644
--- a/usr/src/lib/librestart/common/librestart.c
+++ b/usr/src/lib/librestart/common/librestart.c
@@ -107,6 +107,16 @@ struct restarter_event {
 	restarter_instance_state_t	re_next_state;
 };
 
+/*
+ * A static no memory error message mc_error_t structure
+ * to be used in cases when memory errors are to be returned
+ * This avoids the need to attempt to allocate memory for the
+ * message, therefore getting into a cycle of no memory failures.
+ */
+mc_error_t mc_nomem_err = {
+	0, ENOMEM, sizeof ("Out of memory") - 1, "Out of memory"
+};
+
 static const char * const allocfail = "Allocation failure.\n";
 static const char * const rcbroken = "Repository connection broken.\n";
 
@@ -114,6 +124,61 @@ static int method_context_safety = 0;	/* Can safely call pools/projects. */
 
 int ndebug = 1;
 
+/* PRINTFLIKE3 */
+static mc_error_t *
+mc_error_create(mc_error_t *e, int type, const char *format, ...)
+{
+	mc_error_t	*le;
+	va_list		args;
+	int		size;
+
+	/*
+	 * If the type is ENOMEM and format is NULL, then
+	 * go ahead and return the default nomem error.
+	 * Otherwise, attempt to allocate the memory and if
+	 * that fails then there is no reason to continue.
+	 */
+	if (type == ENOMEM && format == NULL)
+		return (&mc_nomem_err);
+
+	if (e == NULL && (le = malloc(sizeof (mc_error_t))) == NULL)
+		return (&mc_nomem_err);
+	else
+		le = e;
+
+	le->type = type;
+	le->destroy = 1;
+	va_start(args, format);
+	size = vsnprintf(NULL, 0, format, args) + 1;
+	if (size >= RESTARTER_ERRMSGSZ) {
+		if ((le = realloc(e, sizeof (mc_error_t) +
+		    (size - RESTARTER_ERRMSGSZ))) == NULL) {
+			size = RESTARTER_ERRMSGSZ - 1;
+			le = e;
+		}
+	}
+
+	le->size = size;
+	(void) vsnprintf(le->msg, le->size, format, args);
+	va_end(args);
+
+	return (le);
+}
+
+void
+restarter_mc_error_destroy(mc_error_t *mc_err)
+{
+	if (mc_err == NULL)
+		return;
+
+	/*
+	 * If the error messages was allocated then free.
+	 */
+	if (mc_err->destroy) {
+		free(mc_err);
+	}
+}
+
 static void
 free_restarter_event_handle(struct restarter_event_handle *h)
 {
@@ -1975,16 +2040,20 @@ get_groups(char *str, struct method_context *ci)
 	return (0);
 }
 
+
 /*
- * Eventually, we will return a structured error in the case of
- * retryable or abortable failures such as memory allocation errors and
- * repository connection failures.  For now, these failures are just
- * encoded in the failure string.
+ * Return an error message structure containing the error message
+ * with context, and the error so the caller can make a decision
+ * on what to do next.
+ *
+ * Because get_ids uses the mc_error_create() function which can
+ * reallocate the merr, this function must return the merr pointer
+ * in case it was reallocated.
  */
-static const char *
+static mc_error_t *
 get_profile(scf_propertygroup_t *methpg, scf_propertygroup_t *instpg,
     scf_property_t *prop, scf_value_t *val, const char *cmdline,
-    struct method_context *ci)
+    struct method_context *ci, mc_error_t *merr)
 {
 	char *buf = ci->vbuf;
 	ssize_t buf_sz = ci->vbuf_sz;
@@ -1992,12 +2061,16 @@ get_profile(scf_propertygroup_t *methpg, scf_propertygroup_t *instpg,
 	char *cp, *value;
 	const char *cmdp;
 	execattr_t *eap;
-	const char *errstr = NULL;
+	mc_error_t *err = merr;
+	int r;
 
 	if (!(get_astring_val(methpg, SCF_PROPERTY_PROFILE, buf, buf_sz, prop,
 	    val) == 0 || get_astring_val(instpg, SCF_PROPERTY_PROFILE, buf,
 	    buf_sz, prop, val) == 0))
-		return ("Could not get profile property.");
+		return (mc_error_create(merr, scf_error(),
+		    "Method context requires a profile, but the  \"%s\" "
+		    "property could not be read. scf_error is %s",
+		    SCF_PROPERTY_PROFILE, scf_strerror(scf_error())));
 
 	/* Extract the command from the command line. */
 	cp = strpbrk(cmdline, " \t");
@@ -2014,23 +2087,31 @@ get_profile(scf_propertygroup_t *methpg, scf_propertygroup_t *instpg,
 
 	eap = getexecprof(buf, KV_COMMAND, cmdp, GET_ONE);
 	if (eap == NULL)
-		return ("Could not find profile.");
+		return (mc_error_create(merr, ENOENT,
+		    "Could not find the execution profile \"%s\", "
+		    "command %s.", buf, cmdp));
 
 	/* Based on pfexec.c */
 
 	/* Get the euid first so we don't override ci->pwd for the uid. */
 	if ((value = kva_match(eap->attr, EXECATTR_EUID_KW)) != NULL) {
-		if (get_uid(value, ci, &ci->euid) != 0) {
+		if ((r = get_uid(value, ci, &ci->euid)) != 0) {
 			ci->euid = (uid_t)-1;
-			errstr = "Could not interpret profile euid.";
+			err = mc_error_create(merr, r,
+			    "Could not interpret profile euid value \"%s\", "
+			    "from the execution profile \"%s\", error %d.",
+			    value, buf, r);
 			goto out;
 		}
 	}
 
 	if ((value = kva_match(eap->attr, EXECATTR_UID_KW)) != NULL) {
-		if (get_uid(value, ci, &ci->uid) != 0) {
+		if ((r = get_uid(value, ci, &ci->uid)) != 0) {
 			ci->euid = ci->uid = (uid_t)-1;
-			errstr = "Could not interpret profile uid.";
+			err = mc_error_create(merr, r,
+			    "Could not interpret profile uid value \"%s\", "
+			    "from the execution profile \"%s\", error %d.",
+			    value, buf, r);
 			goto out;
 		}
 		ci->euid = ci->uid;
@@ -2039,7 +2120,9 @@ get_profile(scf_propertygroup_t *methpg, scf_propertygroup_t *instpg,
 	if ((value = kva_match(eap->attr, EXECATTR_GID_KW)) != NULL) {
 		ci->egid = ci->gid = get_gid(value);
 		if (ci->gid == (gid_t)-1) {
-			errstr = "Could not interpret profile gid.";
+			err = mc_error_create(merr, EINVAL,
+			    "Could not interpret profile gid value \"%s\", "
+			    "from the execution profile \"%s\".", value, buf);
 			goto out;
 		}
 	}
@@ -2047,7 +2130,9 @@ get_profile(scf_propertygroup_t *methpg, scf_propertygroup_t *instpg,
 	if ((value = kva_match(eap->attr, EXECATTR_EGID_KW)) != NULL) {
 		ci->egid = get_gid(value);
 		if (ci->egid == (gid_t)-1) {
-			errstr = "Could not interpret profile egid.";
+			err = mc_error_create(merr, EINVAL,
+			    "Could not interpret profile egid value \"%s\", "
+			    "from the execution profile \"%s\".", value, buf);
 			goto out;
 		}
 	}
@@ -2056,10 +2141,14 @@ get_profile(scf_propertygroup_t *methpg, scf_propertygroup_t *instpg,
 		ci->lpriv_set = priv_str_to_set(value, ",", NULL);
 		if (ci->lpriv_set == NULL) {
 			if (errno != EINVAL)
-				errstr = ALLOCFAIL;
+				err = mc_error_create(merr, ENOMEM,
+				    ALLOCFAIL);
 			else
-				errstr = "Could not interpret profile "
-				    "limitprivs.";
+				err = mc_error_create(merr, EINVAL,
+				    "Could not interpret profile "
+				    "limitprivs value \"%s\", from "
+				    "the execution profile \"%s\".",
+				    value, buf);
 			goto out;
 		}
 	}
@@ -2068,9 +2157,13 @@ get_profile(scf_propertygroup_t *methpg, scf_propertygroup_t *instpg,
 		ci->priv_set = priv_str_to_set(value, ",", NULL);
 		if (ci->priv_set == NULL) {
 			if (errno != EINVAL)
-				errstr = ALLOCFAIL;
+				err = mc_error_create(merr, ENOMEM,
+				    ALLOCFAIL);
 			else
-				errstr = "Could not interpret profile privs.";
+				err = mc_error_create(merr, EINVAL,
+				    "Could not interpret profile privs value "
+				    "\"%s\", from the execution profile "
+				    "\"%s\".", value, buf);
 			goto out;
 		}
 	}
@@ -2078,20 +2171,23 @@ get_profile(scf_propertygroup_t *methpg, scf_propertygroup_t *instpg,
 out:
 	free_execattr(eap);
 
-	return (errstr);
+	return (err);
 }
 
 /*
- * Eventually, we will return a structured error in the case of
- * retryable or abortable failures such as memory allocation errors and
- * repository connection failures.  For now, these failures are just
- * encoded in the failure string.
+ * Return an error message structure containing the error message
+ * with context, and the error so the caller can make a decision
+ * on what to do next.
+ *
+ * Because get_ids uses the mc_error_create() function which can
+ * reallocate the merr, this function must return the merr pointer
+ * in case it was reallocated.
  */
-static const char *
+static mc_error_t *
 get_ids(scf_propertygroup_t *methpg, scf_propertygroup_t *instpg,
-    scf_property_t *prop, scf_value_t *val, struct method_context *ci)
+    scf_property_t *prop, scf_value_t *val, struct method_context *ci,
+    mc_error_t *merr)
 {
-	char *errstr = NULL;
 	char *vbuf = ci->vbuf;
 	ssize_t vbuf_sz = ci->vbuf_sz;
 	int r;
@@ -2101,23 +2197,22 @@ get_ids(scf_propertygroup_t *methpg, scf_propertygroup_t *instpg,
 	 * another path of just setting the ids to defaults, instead of
 	 * attempting to get the ids here.
 	 */
-	if (methpg == NULL && instpg == NULL) {
-		errstr = "No property groups to get ids from.";
-		goto out;
-	}
+	if (methpg == NULL && instpg == NULL)
+		return (mc_error_create(merr, ENOENT,
+		    "No property groups to get ids from."));
 
 	if (!(get_astring_val(methpg, SCF_PROPERTY_USER,
 	    vbuf, vbuf_sz, prop, val) == 0 || get_astring_val(instpg,
 	    SCF_PROPERTY_USER, vbuf, vbuf_sz, prop,
-	    val) == 0)) {
-		errstr = "Could not get user property.";
-		goto out;
-	}
+	    val) == 0))
+		return (mc_error_create(merr, ENOENT,
+		    "Could not get \"%s\" property.", SCF_PROPERTY_USER));
 
-	if (get_uid(vbuf, ci, &ci->uid) != 0) {
+	if ((r = get_uid(vbuf, ci, &ci->uid)) != 0) {
 		ci->uid = (uid_t)-1;
-		errstr = "Could not interpret user property.";
-		goto out;
+		return (mc_error_create(merr, r,
+		    "Could not interpret \"%s\" property value \"%s\", "
+		    "error %d.", SCF_PROPERTY_USER, vbuf, r));
 	}
 
 	if (!(get_astring_val(methpg, SCF_PROPERTY_GROUP, vbuf, vbuf_sz, prop,
@@ -2126,16 +2221,18 @@ get_ids(scf_propertygroup_t *methpg, scf_propertygroup_t *instpg,
 		if (scf_error() == SCF_ERROR_NOT_FOUND) {
 			(void) strcpy(vbuf, ":default");
 		} else {
-			errstr = "Could not get group property.";
-			goto out;
+			return (mc_error_create(merr, ENOENT,
+			    "Could not get \"%s\" property.",
+			    SCF_PROPERTY_GROUP));
 		}
 	}
 
 	if (strcmp(vbuf, ":default") != 0) {
 		ci->gid = get_gid(vbuf);
 		if (ci->gid == (gid_t)-1) {
-			errstr = "Could not interpret group property.";
-			goto out;
+			return (mc_error_create(merr, ENOENT,
+			    "Could not interpret \"%s\" property value \"%s\".",
+			    SCF_PROPERTY_GROUP, vbuf));
 		}
 	} else {
 		switch (r = lookup_pwd(ci)) {
@@ -2145,18 +2242,18 @@ get_ids(scf_propertygroup_t *methpg, scf_propertygroup_t *instpg,
 
 		case ENOENT:
 			ci->gid = (gid_t)-1;
-			errstr = "No passwd entry.";
-			goto out;
+			return (mc_error_create(merr, ENOENT,
+			    "No passwd entry for uid \"%d\".", ci->uid));
 
 		case ENOMEM:
-			errstr = "Out of memory.";
-			goto out;
+			return (mc_error_create(merr, ENOMEM,
+			    "Out of memory."));
 
 		case EIO:
 		case EMFILE:
 		case ENFILE:
-			errstr = "getpwuid_r() failed.";
-			goto out;
+			return (mc_error_create(merr, ENFILE,
+			    "getpwuid_r() failed, error %d.", r));
 
 		default:
 			bad_fail("lookup_pwd", r);
@@ -2169,8 +2266,9 @@ get_ids(scf_propertygroup_t *methpg, scf_propertygroup_t *instpg,
 		if (scf_error() == SCF_ERROR_NOT_FOUND) {
 			(void) strcpy(vbuf, ":default");
 		} else {
-			errstr = "Could not get supplemental groups property.";
-			goto out;
+			return (mc_error_create(merr, ENOENT,
+			    "Could not get supplemental groups (\"%s\") "
+			    "property.", SCF_PROPERTY_SUPP_GROUPS));
 		}
 	}
 
@@ -2180,13 +2278,15 @@ get_ids(scf_propertygroup_t *methpg, scf_propertygroup_t *instpg,
 			break;
 
 		case EINVAL:
-			errstr =
-			    "Could not interpret supplemental groups property.";
-			goto out;
+			return (mc_error_create(merr, EINVAL,
+			    "Could not interpret supplemental groups (\"%s\") "
+			    "property value \"%s\".", SCF_PROPERTY_SUPP_GROUPS,
+			    vbuf));
 
 		case E2BIG:
-			errstr = "Too many supplemental groups.";
-			goto out;
+			return (mc_error_create(merr, E2BIG,
+			    "Too many supplemental groups values in \"%s\".",
+			    vbuf));
 
 		default:
 			bad_fail("get_groups", r);
@@ -2201,8 +2301,9 @@ get_ids(scf_propertygroup_t *methpg, scf_propertygroup_t *instpg,
 		if (scf_error() == SCF_ERROR_NOT_FOUND) {
 			(void) strcpy(vbuf, ":default");
 		} else {
-			errstr = "Could not get privileges property.";
-			goto out;
+			return (mc_error_create(merr, ENOENT,
+			    "Could not get \"%s\" property.",
+			    SCF_PROPERTY_PRIVILEGES));
 		}
 	}
 
@@ -2214,12 +2315,14 @@ get_ids(scf_propertygroup_t *methpg, scf_propertygroup_t *instpg,
 		ci->priv_set = priv_str_to_set(vbuf, ",", NULL);
 		if (ci->priv_set == NULL) {
 			if (errno != EINVAL) {
-				errstr = ALLOCFAIL;
+				return (mc_error_create(merr, ENOMEM,
+				    ALLOCFAIL));
 			} else {
-				errstr =
-				    "Could not interpret privileges property.";
+				return (mc_error_create(merr, EINVAL,
+				    "Could not interpret \"%s\" "
+				    "property value \"%s\".",
+				    SCF_PROPERTY_PRIVILEGES, vbuf));
 			}
-			goto out;
 		}
 	}
 
@@ -2229,8 +2332,9 @@ get_ids(scf_propertygroup_t *methpg, scf_propertygroup_t *instpg,
 		if (scf_error() == SCF_ERROR_NOT_FOUND) {
 			(void) strcpy(vbuf, ":default");
 		} else {
-			errstr = "Could not get limit_privileges property.";
-			goto out;
+			return (mc_error_create(merr, ENOENT,
+			    "Could not get \"%s\" property.",
+			    SCF_PROPERTY_LIMIT_PRIVILEGES));
 		}
 	}
 
@@ -2244,17 +2348,16 @@ get_ids(scf_propertygroup_t *methpg, scf_propertygroup_t *instpg,
 
 	ci->lpriv_set = priv_str_to_set(vbuf, ",", NULL);
 	if (ci->lpriv_set == NULL) {
-		if (errno != EINVAL)
-			errstr = ALLOCFAIL;
-		else {
-			errstr =
-			    "Could not interpret limit_privileges property.";
+		if (errno != EINVAL) {
+			return (mc_error_create(merr, ENOMEM, ALLOCFAIL));
+		} else {
+			return (mc_error_create(merr, EINVAL,
+			    "Could not interpret \"%s\" property value \"%s\".",
+			    SCF_PROPERTY_LIMIT_PRIVILEGES, vbuf));
 		}
-		goto out;
 	}
 
-out:
-	return (errstr);
+	return (merr);
 }
 
 static int
@@ -2328,19 +2431,42 @@ out:
 
 /*
  * Fetch method context information from the repository, allocate and fill
- * a method_context structure, return it in *mcpp, and return NULL.  On error,
- * return a human-readable string which indicates the error.
+ * a method_context structure, return it in *mcpp, and return NULL.
  *
  * If no method_context is defined, original init context is provided, where
  * the working directory is '/', and uid/gid are 0/0.  But if a method_context
  * is defined at any level the smf_method(5) method_context defaults are used.
  *
- * Eventually, we will return a structured error in the case of
- * retryable or abortable failures such as memory allocation errors and
- * repository connection failures.  For now, these failures are just
- * encoded in the failure string.
+ * Return an error message structure containing the error message
+ * with context, and the error so the caller can make a decision
+ * on what to do next.
+ *
+ * Error Types :
+ * 	E2BIG		Too many values or entry is too big
+ * 	EINVAL		Invalid value
+ * 	EIO		an I/O error has occured
+ * 	ENOENT		no entry for value
+ * 	ENOMEM		out of memory
+ * 	ENOTSUP		Version mismatch
+ * 	ERANGE		value is out of range
+ * 	EMFILE/ENFILE	out of file descriptors
+ *
+ * 	SCF_ERROR_BACKEND_ACCESS
+ * 	SCF_ERROR_CONNECTION_BROKEN
+ * 	SCF_ERROR_DELETED
+ * 	SCF_ERROR_CONSTRAINT_VIOLATED
+ * 	SCF_ERROR_HANDLE_DESTROYED
+ * 	SCF_ERROR_INTERNAL
+ * 	SCF_ERROR_INVALID_ARGUMENT
+ * 	SCF_ERROR_NO_MEMORY
+ * 	SCF_ERROR_NO_RESOURCES
+ * 	SCF_ERROR_NOT_BOUND
+ * 	SCF_ERROR_NOT_FOUND
+ * 	SCF_ERROR_NOT_SET
+ * 	SCF_ERROR_TYPE_MISMATCH
+ *
  */
-const char *
+mc_error_t *
 restarter_get_method_context(uint_t version, scf_instance_t *inst,
     scf_snapshot_t *snap, const char *mname, const char *cmdline,
     struct method_context **mcpp)
@@ -2353,22 +2479,31 @@ restarter_get_method_context(uint_t version, scf_instance_t *inst,
 	scf_value_t *val = NULL;
 	scf_type_t ty;
 	uint8_t use_profile;
-	int ret;
+	int ret = 0;
 	int mc_used = 0;
-	const char *errstr = NULL;
+	mc_error_t *err = NULL;
 	struct method_context *cip;
 
+	if ((err = malloc(sizeof (mc_error_t))) == NULL)
+		return (mc_error_create(NULL, ENOMEM, NULL));
+
+	/* Set the type to zero to track if an error occured. */
+	err->type = 0;
+
 	if (version != RESTARTER_METHOD_CONTEXT_VERSION)
-		return ("Unknown method_context version.");
+		return (mc_error_create(err, ENOTSUP,
+		    "Invalid client version %d. (Expected %d)",
+		    version, RESTARTER_METHOD_CONTEXT_VERSION));
 
 	/* Get the handle before we allocate anything. */
 	h = scf_instance_handle(inst);
 	if (h == NULL)
-		return (scf_strerror(scf_error()));
+		return (mc_error_create(err, scf_error(),
+		    scf_strerror(scf_error())));
 
 	cip = malloc(sizeof (*cip));
 	if (cip == NULL)
-		return (ALLOCFAIL);
+		return (mc_error_create(err, ENOMEM, ALLOCFAIL));
 
 	(void) memset(cip, 0, sizeof (*cip));
 	cip->uid = (uid_t)-1;
@@ -2381,14 +2516,16 @@ restarter_get_method_context(uint_t version, scf_instance_t *inst,
 	cip->vbuf = malloc(cip->vbuf_sz);
 	if (cip->vbuf == NULL) {
 		free(cip);
-		return (ALLOCFAIL);
+		return (mc_error_create(err, ENOMEM, ALLOCFAIL));
 	}
 
 	if ((instpg = scf_pg_create(h)) == NULL ||
 	    (methpg = scf_pg_create(h)) == NULL ||
 	    (prop = scf_property_create(h)) == NULL ||
 	    (val = scf_value_create(h)) == NULL) {
-		errstr = ALLOCFAIL;
+		err = mc_error_create(err, scf_error(),
+		    "Failed to create repository object: %s\n",
+		    scf_strerror(scf_error()));
 		goto out;
 	}
 
@@ -2401,14 +2538,17 @@ restarter_get_method_context(uint_t version, scf_instance_t *inst,
 
 	if (scf_instance_get_pg_composed(inst, snap, mname, methpg) !=
 	    SCF_SUCCESS) {
-		errstr = scf_strerror(scf_error());
+		err = mc_error_create(err, scf_error(), "Unable to get the "
+		    "\"%s\" method, %s", mname, scf_strerror(scf_error()));
 		goto out;
 	}
 
 	if (scf_instance_get_pg_composed(inst, snap, SCF_PG_METHOD_CONTEXT,
 	    instpg) != SCF_SUCCESS) {
 		if (scf_error() != SCF_ERROR_NOT_FOUND) {
-			errstr = scf_strerror(scf_error());
+			err = mc_error_create(err, scf_error(),
+			    "Unable to retrieve the \"%s\" property group, %s",
+			    SCF_PG_METHOD_CONTEXT, scf_strerror(scf_error()));
 			goto out;
 		}
 		scf_pg_destroy(instpg);
@@ -2429,13 +2569,14 @@ restarter_get_method_context(uint_t version, scf_instance_t *inst,
 	case ENOENT:
 		break;
 	case ENOMEM:
-		errstr = "Out of memory.";
+		err = mc_error_create(err, ret, "Out of memory.");
 		goto out;
 	case EINVAL:
-		errstr = "Invalid method environment.";
+		err = mc_error_create(err, ret, "Invalid method environment.");
 		goto out;
 	default:
-		errstr = scf_strerror(ret);
+		err = mc_error_create(err, ret,
+		    "Get method environment failed : %s\n", scf_strerror(ret));
 		goto out;
 	}
 
@@ -2457,11 +2598,14 @@ restarter_get_method_context(uint_t version, scf_instance_t *inst,
 			break;
 
 		case SCF_ERROR_CONNECTION_BROKEN:
-			errstr = RCBROKEN;
+			err = mc_error_create(err, SCF_ERROR_CONNECTION_BROKEN,
+			    RCBROKEN);
 			goto out;
 
 		case SCF_ERROR_DELETED:
-			errstr = "\"use_profile\" property deleted.";
+			err = mc_error_create(err, SCF_ERROR_NOT_FOUND,
+			    "Could not find property group \"%s\"",
+			    pg == NULL ? SCF_PG_METHOD_CONTEXT : mname);
 			goto out;
 
 		case SCF_ERROR_HANDLE_MISMATCH:
@@ -2472,47 +2616,60 @@ restarter_get_method_context(uint_t version, scf_instance_t *inst,
 		}
 	} else {
 		if (scf_property_type(prop, &ty) != SCF_SUCCESS) {
-			switch (scf_error()) {
+			ret = scf_error();
+			switch (ret) {
 			case SCF_ERROR_CONNECTION_BROKEN:
-				errstr = RCBROKEN;
+				err = mc_error_create(err,
+				    SCF_ERROR_CONNECTION_BROKEN, RCBROKEN);
 				break;
 
 			case SCF_ERROR_DELETED:
-				errstr = "\"use profile\" property deleted.";
+				err = mc_error_create(err,
+				    SCF_ERROR_NOT_FOUND,
+				    "Could not find property group \"%s\"",
+				    pg == NULL ? SCF_PG_METHOD_CONTEXT : mname);
 				break;
 
 			case SCF_ERROR_NOT_SET:
 			default:
-				bad_fail("scf_property_type", scf_error());
+				bad_fail("scf_property_type", ret);
 			}
 
 			goto out;
 		}
 
 		if (ty != SCF_TYPE_BOOLEAN) {
-			errstr = "\"use profile\" property is not boolean.";
+			err = mc_error_create(err,
+			    SCF_ERROR_TYPE_MISMATCH,
+			    "\"%s\" property is not boolean in property group "
+			    "\"%s\".", SCF_PROPERTY_USE_PROFILE,
+			    pg == NULL ? SCF_PG_METHOD_CONTEXT : mname);
 			goto out;
 		}
 
 		if (scf_property_get_value(prop, val) != SCF_SUCCESS) {
-			switch (scf_error()) {
+			ret = scf_error();
+			switch (ret) {
 			case SCF_ERROR_CONNECTION_BROKEN:
-				errstr = RCBROKEN;
+				err = mc_error_create(err,
+				    SCF_ERROR_CONNECTION_BROKEN, RCBROKEN);
 				break;
 
 			case SCF_ERROR_CONSTRAINT_VIOLATED:
-				errstr =
-				    "\"use profile\" property has multiple "
-				    "values.";
+				err = mc_error_create(err,
+				    SCF_ERROR_CONSTRAINT_VIOLATED,
+				    "\"%s\" property has multiple values.",
+				    SCF_PROPERTY_USE_PROFILE);
 				break;
 
 			case SCF_ERROR_NOT_FOUND:
-				errstr = "\"use profile\" property has no "
-				    "values.";
+				err = mc_error_create(err,
+				    SCF_ERROR_NOT_FOUND,
+				    "\"%s\" property has no values.",
+				    SCF_PROPERTY_USE_PROFILE);
 				break;
-
 			default:
-				bad_fail("scf_property_get_value", scf_error());
+				bad_fail("scf_property_get_value", ret);
 			}
 
 			goto out;
@@ -2524,12 +2681,12 @@ restarter_get_method_context(uint_t version, scf_instance_t *inst,
 
 		/* get ids & privileges */
 		if (use_profile)
-			errstr = get_profile(pg, instpg, prop, val, cmdline,
-			    cip);
+			err = get_profile(pg, instpg, prop, val, cmdline,
+			    cip, err);
 		else
-			errstr = get_ids(pg, instpg, prop, val, cip);
+			err = get_ids(pg, instpg, prop, val, cip, err);
 
-		if (errstr != NULL)
+		if (err->type != 0)
 			goto out;
 	}
 
@@ -2539,24 +2696,26 @@ restarter_get_method_context(uint_t version, scf_instance_t *inst,
 	    (instpg != NULL && scf_pg_get_property(instpg,
 	    SCF_PROPERTY_WORKING_DIRECTORY, prop) == SCF_SUCCESS)) {
 		if (scf_property_get_value(prop, val) != SCF_SUCCESS) {
-			switch (scf_error()) {
+			ret = scf_error();
+			switch (ret) {
 			case SCF_ERROR_CONNECTION_BROKEN:
-				errstr = RCBROKEN;
+				err = mc_error_create(err, ret, RCBROKEN);
 				break;
 
 			case SCF_ERROR_CONSTRAINT_VIOLATED:
-				errstr =
-				    "\"working directory\" property has "
-				    "multiple values.";
+				err = mc_error_create(err, ret,
+				    "\"%s\" property has multiple values.",
+				    SCF_PROPERTY_WORKING_DIRECTORY);
 				break;
 
 			case SCF_ERROR_NOT_FOUND:
-				errstr = "\"working directory\" property has "
-				    "no values.";
+				err = mc_error_create(err, ret,
+				    "\"%s\" property has no values.",
+				    SCF_PROPERTY_WORKING_DIRECTORY);
 				break;
 
 			default:
-				bad_fail("scf_property_get_value", scf_error());
+				bad_fail("scf_property_get_value", ret);
 			}
 
 			goto out;
@@ -2566,25 +2725,27 @@ restarter_get_method_context(uint_t version, scf_instance_t *inst,
 		ret = scf_value_get_astring(val, cip->vbuf, cip->vbuf_sz);
 		assert(ret != -1);
 	} else {
-		switch (scf_error()) {
+		ret = scf_error();
+		switch (ret) {
 		case SCF_ERROR_NOT_FOUND:
 			/* okay if missing. */
 			(void) strcpy(cip->vbuf, ":default");
 			break;
 
 		case SCF_ERROR_CONNECTION_BROKEN:
-			errstr = RCBROKEN;
+			err = mc_error_create(err, ret, RCBROKEN);
 			goto out;
 
 		case SCF_ERROR_DELETED:
-			errstr = "\"working_directory\" property deleted.";
+			err = mc_error_create(err, ret,
+			    "Property group could not be found");
 			goto out;
 
 		case SCF_ERROR_HANDLE_MISMATCH:
 		case SCF_ERROR_INVALID_ARGUMENT:
 		case SCF_ERROR_NOT_SET:
 		default:
-			bad_fail("scf_pg_get_property", scf_error());
+			bad_fail("scf_pg_get_property", ret);
 		}
 	}
 
@@ -2595,14 +2756,15 @@ restarter_get_method_context(uint_t version, scf_instance_t *inst,
 			break;
 
 		case ENOMEM:
-			errstr = "Out of memory.";
+			err = mc_error_create(err, ret, "Out of memory.");
 			goto out;
 
 		case ENOENT:
 		case EIO:
 		case EMFILE:
 		case ENFILE:
-			errstr = "Could not get passwd entry.";
+			err = mc_error_create(err, ret,
+			    "Could not get passwd entry.");
 			goto out;
 
 		default:
@@ -2611,13 +2773,13 @@ restarter_get_method_context(uint_t version, scf_instance_t *inst,
 
 		cip->working_dir = strdup(cip->pwd.pw_dir);
 		if (cip->working_dir == NULL) {
-			errstr = ALLOCFAIL;
+			err = mc_error_create(err, ENOMEM, ALLOCFAIL);
 			goto out;
 		}
 	} else {
 		cip->working_dir = strdup(cip->vbuf);
 		if (cip->working_dir == NULL) {
-			errstr = ALLOCFAIL;
+			err = mc_error_create(err, ENOMEM, ALLOCFAIL);
 			goto out;
 		}
 	}
@@ -2628,24 +2790,26 @@ restarter_get_method_context(uint_t version, scf_instance_t *inst,
 	    (instpg != NULL && scf_pg_get_property(instpg,
 	    SCF_PROPERTY_COREFILE_PATTERN, prop) == SCF_SUCCESS)) {
 		if (scf_property_get_value(prop, val) != SCF_SUCCESS) {
-			switch (scf_error()) {
+			ret = scf_error();
+			switch (ret) {
 			case SCF_ERROR_CONNECTION_BROKEN:
-				errstr = RCBROKEN;
+				err = mc_error_create(err, ret, RCBROKEN);
 				break;
 
 			case SCF_ERROR_CONSTRAINT_VIOLATED:
-				errstr =
-				    "\"core_pattern\" property has multiple "
-				    "values.";
+				err = mc_error_create(err, ret,
+				    "\"%s\" property has multiple values.",
+				    SCF_PROPERTY_COREFILE_PATTERN);
 				break;
 
 			case SCF_ERROR_NOT_FOUND:
-				errstr = "\"core_pattern\" property has no "
-				    "values.";
+				err = mc_error_create(err, ret,
+				    "\"%s\" property has no values.",
+				    SCF_PROPERTY_COREFILE_PATTERN);
 				break;
 
 			default:
-				bad_fail("scf_property_get_value", scf_error());
+				bad_fail("scf_property_get_value", ret);
 			}
 
 		} else {
@@ -2656,31 +2820,33 @@ restarter_get_method_context(uint_t version, scf_instance_t *inst,
 
 			cip->corefile_pattern = strdup(cip->vbuf);
 			if (cip->corefile_pattern == NULL) {
-				errstr = ALLOCFAIL;
+				err = mc_error_create(err, ENOMEM, ALLOCFAIL);
 				goto out;
 			}
 		}
 
 		mc_used++;
 	} else {
-		switch (scf_error()) {
+		ret = scf_error();
+		switch (ret) {
 		case SCF_ERROR_NOT_FOUND:
 			/* okay if missing. */
 			break;
 
 		case SCF_ERROR_CONNECTION_BROKEN:
-			errstr = RCBROKEN;
+			err = mc_error_create(err, ret, RCBROKEN);
 			goto out;
 
 		case SCF_ERROR_DELETED:
-			errstr = "\"corefile_pattern\" property deleted.";
+			err = mc_error_create(err, ret,
+			    "Property group could not be found");
 			goto out;
 
 		case SCF_ERROR_HANDLE_MISMATCH:
 		case SCF_ERROR_INVALID_ARGUMENT:
 		case SCF_ERROR_NOT_SET:
 		default:
-			bad_fail("scf_pg_get_property", scf_error());
+			bad_fail("scf_pg_get_property", ret);
 		}
 	}
 
@@ -2691,25 +2857,27 @@ restarter_get_method_context(uint_t version, scf_instance_t *inst,
 		    (instpg != NULL && scf_pg_get_property(instpg,
 		    SCF_PROPERTY_PROJECT, prop) == SCF_SUCCESS)) {
 			if (scf_property_get_value(prop, val) != SCF_SUCCESS) {
-				switch (scf_error()) {
+				ret = scf_error();
+				switch (ret) {
 				case SCF_ERROR_CONNECTION_BROKEN:
-					errstr = RCBROKEN;
+					err = mc_error_create(err, ret,
+					    RCBROKEN);
 					break;
 
 				case SCF_ERROR_CONSTRAINT_VIOLATED:
-					errstr =
-					    "\"project\" property has "
-					    "multiple values.";
+					err = mc_error_create(err, ret,
+					    "\"%s\" property has multiple "
+					    "values.", SCF_PROPERTY_PROJECT);
 					break;
 
 				case SCF_ERROR_NOT_FOUND:
-					errstr = "\"project\" property has "
-					    "no values.";
+					err = mc_error_create(err, ret,
+					    "\"%s\" property has no values.",
+					    SCF_PROPERTY_PROJECT);
 					break;
 
 				default:
-					bad_fail("scf_property_get_value",
-					    scf_error());
+					bad_fail("scf_property_get_value", ret);
 				}
 
 				(void) strcpy(cip->vbuf, ":default");
@@ -2729,36 +2897,39 @@ restarter_get_method_context(uint_t version, scf_instance_t *inst,
 			break;
 
 		case ENOMEM:
-			errstr = "Out of memory.";
+			err = mc_error_create(err, ret, "Out of memory.");
 			goto out;
 
 		case ENOENT:
-			errstr = "Missing passwd or project entry.";
+			err = mc_error_create(err, ret,
+			    "Missing passwd or project entry for \"%s\".",
+			    cip->vbuf);
 			goto out;
 
 		case EIO:
-			errstr = "I/O error.";
+			err = mc_error_create(err, ret, "I/O error.");
 			goto out;
 
 		case EMFILE:
 		case ENFILE:
-			errstr = "Out of file descriptors.";
+			err = mc_error_create(err, ret,
+			    "Out of file descriptors.");
 			goto out;
 
 		case -1:
-			errstr = "Name service switch is misconfigured.";
+			err = mc_error_create(err, ret,
+			    "Name service switch is misconfigured.");
 			goto out;
 
 		case ERANGE:
-			errstr = "Project ID too big.";
+		case E2BIG:
+			err = mc_error_create(err, ret,
+			    "Project ID \"%s\" too big.", cip->vbuf);
 			goto out;
 
 		case EINVAL:
-			errstr = "Project ID is invalid.";
-			goto out;
-
-		case E2BIG:
-			errstr = "Project entry is too big.";
+			err = mc_error_create(err, ret,
+			    "Project ID \"%s\" is invalid.", cip->vbuf);
 			goto out;
 
 		default:
@@ -2771,25 +2942,29 @@ restarter_get_method_context(uint_t version, scf_instance_t *inst,
 		    (instpg != NULL && scf_pg_get_property(instpg,
 		    SCF_PROPERTY_RESOURCE_POOL, prop) == SCF_SUCCESS)) {
 			if (scf_property_get_value(prop, val) != SCF_SUCCESS) {
-				switch (scf_error()) {
+				ret = scf_error();
+				switch (ret) {
 				case SCF_ERROR_CONNECTION_BROKEN:
-					errstr = RCBROKEN;
+					err = mc_error_create(err, ret,
+					    RCBROKEN);
 					break;
 
 				case SCF_ERROR_CONSTRAINT_VIOLATED:
-					errstr =
-					    "\"resource_pool\" property has "
-					    "multiple values.";
+					err = mc_error_create(err, ret,
+					    "\"%s\" property has multiple "
+					    "values.",
+					    SCF_PROPERTY_RESOURCE_POOL);
 					break;
 
 				case SCF_ERROR_NOT_FOUND:
-					errstr = "\"resource_pool\" property "
-					    "has no values.";
+					err = mc_error_create(err, ret,
+					    "\"%s\" property has no "
+					    "values.",
+					    SCF_PROPERTY_RESOURCE_POOL);
 					break;
 
 				default:
-					bad_fail("scf_property_get_value",
-					    scf_error());
+					bad_fail("scf_property_get_value", ret);
 				}
 
 				(void) strcpy(cip->vbuf, ":default");
@@ -2801,32 +2976,34 @@ restarter_get_method_context(uint_t version, scf_instance_t *inst,
 
 			mc_used++;
 		} else {
-			switch (scf_error()) {
+			ret = scf_error();
+			switch (ret) {
 			case SCF_ERROR_NOT_FOUND:
 				/* okay if missing. */
 				(void) strcpy(cip->vbuf, ":default");
 				break;
 
 			case SCF_ERROR_CONNECTION_BROKEN:
-				errstr = RCBROKEN;
+				err = mc_error_create(err, ret, RCBROKEN);
 				goto out;
 
 			case SCF_ERROR_DELETED:
-				errstr = "\"resource_pool\" property deleted.";
+				err = mc_error_create(err, ret,
+				    "property group could not be found.");
 				goto out;
 
 			case SCF_ERROR_HANDLE_MISMATCH:
 			case SCF_ERROR_INVALID_ARGUMENT:
 			case SCF_ERROR_NOT_SET:
 			default:
-				bad_fail("scf_pg_get_property", scf_error());
+				bad_fail("scf_pg_get_property", ret);
 			}
 		}
 
 		if (strcmp(cip->vbuf, ":default") != 0) {
 			cip->resource_pool = strdup(cip->vbuf);
 			if (cip->resource_pool == NULL) {
-				errstr = ALLOCFAIL;
+				err = mc_error_create(err, ENOMEM, ALLOCFAIL);
 				goto out;
 			}
 		}
@@ -2857,10 +3034,14 @@ out:
 		free(cip->pwbuf);
 	free(cip->vbuf);
 
-	if (errstr != NULL)
+	if (err->type != 0) {
 		restarter_free_method_context(cip);
+	} else {
+		restarter_mc_error_destroy(err);
+		err = NULL;
+	}
 
-	return (errstr);
+	return (err);
 }
 
 /*
diff --git a/usr/src/lib/librestart/common/librestart.h b/usr/src/lib/librestart/common/librestart.h
index abf479d339..e9090aa159 100644
--- a/usr/src/lib/librestart/common/librestart.h
+++ b/usr/src/lib/librestart/common/librestart.h
@@ -78,6 +78,8 @@ typedef uint32_t restarter_event_type_t;
 
 #define	RESTARTER_FLAG_DEBUG		1
 
+#define	RESTARTER_ERRMSGSZ		1024
+
 /*
  * Event types
  *	RESTARTER_EVENT_TYPE_ADD_INSTANCE
@@ -216,7 +218,7 @@ int restarter_remove_contract(scf_instance_t *, ctid_t,
 ssize_t restarter_state_to_string(restarter_instance_state_t, char *, size_t);
 restarter_instance_state_t restarter_string_to_state(char *);
 
-#define	RESTARTER_METHOD_CONTEXT_VERSION	6
+#define	RESTARTER_METHOD_CONTEXT_VERSION	7
 
 struct method_context {
 	/* Stable */
@@ -240,10 +242,29 @@ struct method_context {
 	ssize_t		pwbufsz;
 };
 
+/*
+ * An error structure that contains a message string, and a type
+ * that can be used to determine course of action by the reciever
+ * of the error structure.
+ *
+ * type - usually will be an errno equivalent but could contain
+ * 	defined error types for exampe SCF_ERROR_XXX
+ * msg - must be at the end of the structure as if the message is
+ * 	longer than EMSGSIZE we will reallocate the structure to
+ * 	handle the overflow
+ */
+typedef struct mc_error {
+	int	destroy;	/* Flag to indicate destruction steps */
+	int	type;		/* Type of error for decision making */
+	int	size;		/* The size of the error message string */
+	char 	msg[RESTARTER_ERRMSGSZ];
+} mc_error_t;
+
 int restarter_rm_libs_loadable(void);
 /* instance, restarter name, method name, command line, structure pointer */
-const char *restarter_get_method_context(uint_t, scf_instance_t *,
+mc_error_t *restarter_get_method_context(uint_t, scf_instance_t *,
     scf_snapshot_t *, const char *, const char *, struct method_context **);
+void restarter_mc_error_destroy(mc_error_t *);
 int restarter_set_method_context(struct method_context *, const char **);
 void restarter_free_method_context(struct method_context *);
 
diff --git a/usr/src/lib/librestart/common/mapfile-vers b/usr/src/lib/librestart/common/mapfile-vers
index 143ee73685..40ac2278ae 100644
--- a/usr/src/lib/librestart/common/mapfile-vers
+++ b/usr/src/lib/librestart/common/mapfile-vers
@@ -67,6 +67,7 @@ SUNWprivate_1.1 {
 	restarter_inst_reset_ractions_aux_fmri;
 	restarter_inst_reset_aux_fmri;
 	restarter_inst_set_aux_fmri;
+	restarter_mc_error_destroy;
     local:
 	*;
 };