svn commit: r256479 - projects/zfsd/head/cddl/sbin/zfsd

[Alan Somers]

Author: asomers
Date: Mon Oct 14 23:41:55 2013
New Revision: 256479
URL: http://svnweb.freebsd.org/changeset/base/256479

Log:
  Fix a bug in zfsd: zfsd will fail to autoreplace by physical path a drive if
  it had previously replaced that same drive when it belonged to a different
  pool.  The solution is to delete case files when their pools are destroyed.
  A better fix would be to refactor zfsd to eliminate casefiles altogether,
  and use a more stateless approach.
  
  	cddl/sbin/zfsd/case_file.cc
  	cddl/sbin/zfsd/zfsd_event.cc
  		Close a case file when its pool is destroyed.  Log a warning
  		when two open case files correspond to the same physical
  		path.
  
  Submitted by:	alans
  Approved by:	ken (mentor)
  Sponsored by:	Spectra Logic Corporation

Modified:
  projects/zfsd/head/cddl/sbin/zfsd/case_file.cc
  projects/zfsd/head/cddl/sbin/zfsd/case_file.h
  projects/zfsd/head/cddl/sbin/zfsd/zfsd_event.cc

Modified: projects/zfsd/head/cddl/sbin/zfsd/case_file.cc
==============================================================================
--- projects/zfsd/head/cddl/sbin/zfsd/case_file.cc	Mon Oct 14 23:36:10 2013	(r256478)
+++ projects/zfsd/head/cddl/sbin/zfsd/case_file.cc	Mon Oct 14 23:41:55 2013	(r256479)
@@ -150,15 +150,23 @@ CaseFile::Find(Guid poolGUID, Guid vdevG
 CaseFile *
 CaseFile::Find(const string &physPath)
 {
+	CaseFile *result = NULL;
+
 	for (CaseFileList::iterator curCase = s_activeCases.begin();
 	     curCase != s_activeCases.end(); curCase++) {
 
 		if ((*curCase)->PhysicalPath() != physPath)
 			continue;
 
-		return (*curCase);
+		if (result != NULL) {
+			syslog(LOG_WARNING, "Multiple casefiles found for "
+			    "physical path %s.  "
+			    "This is most likely a bug in zfsd",
+			    physPath.c_str());
+		}
+		result = *curCase;
 	}
-	return (NULL);
+	return (result);
 }
 
 
@@ -370,8 +378,12 @@ CaseFile::ReEvaluate(const ZfsEvent &eve
 		Close();
 
 		return (/*consumed*/true);
-	}
-	else if (event.Value("type") == "misc.fs.zfs.config_sync") {
+	} else if (event.Value("type") == "misc.fs.zfs.pool_destroy") {
+		/* This Pool has been destroyed.  Discard the case */
+		Close();
+
+		return (/*consumed*/true);
+	} else if (event.Value("type") == "misc.fs.zfs.config_sync") {
 		RefreshVdevState();
 		if (VdevState() < VDEV_STATE_HEALTHY)
 			consumed = ActivateSpare();

Modified: projects/zfsd/head/cddl/sbin/zfsd/case_file.h
==============================================================================
--- projects/zfsd/head/cddl/sbin/zfsd/case_file.h	Mon Oct 14 23:36:10 2013	(r256478)
+++ projects/zfsd/head/cddl/sbin/zfsd/case_file.h	Mon Oct 14 23:41:55 2013	(r256479)
@@ -296,7 +296,7 @@ protected:
 	/**
 	 * \brief Unconditionally close a CaseFile.
 	 */
-	void Close();
+	virtual void Close();
 
 	/**
 	 * \brief Callout callback invoked when the remove timer grace

Modified: projects/zfsd/head/cddl/sbin/zfsd/zfsd_event.cc
==============================================================================
--- projects/zfsd/head/cddl/sbin/zfsd/zfsd_event.cc	Mon Oct 14 23:36:10 2013	(r256478)
+++ projects/zfsd/head/cddl/sbin/zfsd/zfsd_event.cc	Mon Oct 14 23:41:55 2013	(r256479)
@@ -195,6 +195,10 @@ DevfsEvent::Process() const
 		       "as a replace by physical path candidate.\n",
 		       devName.c_str());
 	} else if (havePhysPath && IsWholeDev()) {
+		/* 
+		 * TODO: attempt to resolve events using every casefile
+		 * that matches this physpath
+		 */
 		CaseFile *caseFile(CaseFile::Find(physPath));
 		if (caseFile != NULL) {
 			syslog(LOG_INFO,
@@ -371,6 +375,12 @@ ZfsEvent::ProcessPoolEvent() const
 {
 	bool degradedDevice(false);
 
+	/* The pool is destroyed.  Discard any open cases */
+	if (Value("type") == "misc.fs.zfs.pool_destroy") {
+		CaseFile::ReEvaluateByGuid(PoolGUID(), *this);
+		return;
+	}
+
 	CaseFile *caseFile(CaseFile::Find(PoolGUID(), VdevGUID()));
 	if (caseFile != NULL) {
 		if (caseFile->VdevState() != VDEV_STATE_UNKNOWN