Skip site navigation (1)Skip section navigation (2) 
Date:      Mon, 14 Oct 2013 21:34:49 +0000 (UTC)
From:      Alan Somers <asomers@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-projects@freebsd.org
Subject:   svn commit: r256461 - projects/zfsd/head/cddl/sbin/zfsd
Message-ID:  <201310142134.r9ELYnSd073042@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: asomers
Date: Mon Oct 14 21:34:49 2013
New Revision: 256461
URL: http://svnweb.freebsd.org/changeset/base/256461

Log:
  When a vdev generates IO errors, mark it as FAULTED instead of DEGRADED.
  Checksum errors will still be marked as DEGRADED.  This matches the behavior
  of Illumos.  I had to make a number of methods virtual to facilitate unit
  testing.
  
  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/vdev.h

Modified: projects/zfsd/head/cddl/sbin/zfsd/case_file.cc
==============================================================================
--- projects/zfsd/head/cddl/sbin/zfsd/case_file.cc	Mon Oct 14 21:30:00 2013	(r256460)
+++ projects/zfsd/head/cddl/sbin/zfsd/case_file.cc	Mon Oct 14 21:34:49 2013	(r256461)
@@ -806,11 +806,13 @@ CaseFile::Close()
 void
 CaseFile::OnGracePeriodEnded()
 {
+	bool should_fault, should_degrade;
+	ZpoolList zpl(ZpoolList::ZpoolByGUID, &m_poolGUID);
 	m_events.splice(m_events.begin(), m_tentativeEvents);
+	should_fault = ShouldFault();
+	should_degrade = ShouldDegrade();
 
-	if (m_events.size() > ZFS_DEGRADE_IO_COUNT) {
-
-		ZpoolList zpl(ZpoolList::ZpoolByGUID, &m_poolGUID);
+	if (should_fault || should_degrade) {
 		if (zpl.empty()
 		 || (VdevIterator(zpl.front()).Find(m_vdevGUID)) == NULL) {
 			/*
@@ -822,6 +824,28 @@ CaseFile::OnGracePeriodEnded()
 			return;
 		}
 
+	}
+
+	/* A fault condition has priority over a degrade condition */
+	if (ShouldFault()) {
+		/* Fault the vdev and close the case. */
+		if (zpool_vdev_fault(zpl.front(), (uint64_t)m_vdevGUID,
+				       VDEV_AUX_ERR_EXCEEDED) == 0) {
+			syslog(LOG_INFO, "Faulting vdev(%s/%s)",
+			       PoolGUIDString().c_str(),
+			       VdevGUIDString().c_str());
+			Close();
+			return;
+		}
+		else {
+			syslog(LOG_ERR, "Fault vdev(%s/%s): %s: %s\n",
+			       PoolGUIDString().c_str(),
+			       VdevGUIDString().c_str(),
+			       libzfs_error_action(g_zfsHandle),
+			       libzfs_error_description(g_zfsHandle));
+		}
+	}
+	else if (ShouldDegrade()) {
 		/* Degrade the vdev and close the case. */
 		if (zpool_vdev_degrade(zpl.front(), (uint64_t)m_vdevGUID,
 				       VDEV_AUX_ERR_EXCEEDED) == 0) {
@@ -907,3 +931,25 @@ CaseFile::Replace(const char* vdev_type,
 
 	return (true);
 }
+
+/* Does the argument event refer to a checksum error? */
+static bool IsChecksumEvent(const DevCtlEvent* const event){
+	return ("ereport.fs.zfs.checksum" == event->Value("type"));
+}
+
+/* Does the argument event refer to an IO error? */
+static bool IsIOEvent(const DevCtlEvent* const event){
+	return ("ereport.fs.zfs.io" == event->Value("type"));
+}
+
+bool
+CaseFile::ShouldDegrade() const {
+	return (std::count_if(m_events.begin(), m_events.end(),
+	    		      IsChecksumEvent) > ZFS_DEGRADE_IO_COUNT);
+}
+
+bool
+CaseFile::ShouldFault() const {
+	return (std::count_if(m_events.begin(), m_events.end(),
+	    		      IsIOEvent) > ZFS_DEGRADE_IO_COUNT);
+}

Modified: projects/zfsd/head/cddl/sbin/zfsd/case_file.h
==============================================================================
--- projects/zfsd/head/cddl/sbin/zfsd/case_file.h	Mon Oct 14 21:30:00 2013	(r256460)
+++ projects/zfsd/head/cddl/sbin/zfsd/case_file.h	Mon Oct 14 21:34:49 2013	(r256461)
@@ -114,7 +114,7 @@ public:
 	 *
 	 * \param vdev  The vdev object for which to find/create a CaseFile.
 	 *
-	 * \return  A referenc eto a valid CaseFile object.
+	 * \return  A reference to a valid CaseFile object.
 	 */
 	static CaseFile &Create(Vdev &vdev);
 
@@ -173,7 +173,7 @@ public:
 	/**
 	 * \brief Register an itimer callout for the given event, if necessary
 	 */
-	void RegisterCallout(const DevCtlEvent &event);
+	virtual void RegisterCallout(const DevCtlEvent &event);
 
 	/**
 	 * \brief Close a case if it is no longer relevant.
@@ -192,6 +192,16 @@ public:
 	 */
 	void Log();
 
+	/**
+	 * \brief Whether we should degrade this vdev
+	 */
+	bool ShouldDegrade() const;
+
+	/**
+	 * \brief Whether we should fault this vdev
+	 */
+	bool ShouldFault() const;
+
 protected:
 	enum {
 		/**
@@ -228,8 +238,11 @@ protected:
 	/** Constructor. */
 	CaseFile(const Vdev &vdev);
 
-	/** Destructor. */
-	~CaseFile();
+	/**
+	 * Destructor.
+	 * Must be virtual so it can be subclassed in the unit tests
+	 */
+	virtual ~CaseFile();
 
 	/**
 	 * \brief Reload state for the vdev associated with this CaseFile.
@@ -237,7 +250,7 @@ protected:
 	 * \return  True if the refresh was successful.  False if the system
 	 *          has no record of the pool or vdev for this CaseFile.
 	 */
-	bool RefreshVdevState();
+	virtual bool RefreshVdevState();
 
 	/**
 	 * \brief Free all events in the m_events list.

Modified: projects/zfsd/head/cddl/sbin/zfsd/vdev.h
==============================================================================
--- projects/zfsd/head/cddl/sbin/zfsd/vdev.h	Mon Oct 14 21:30:00 2013	(r256460)
+++ projects/zfsd/head/cddl/sbin/zfsd/vdev.h	Mon Oct 14 21:34:49 2013	(r256461)
@@ -96,11 +96,11 @@ public:
 	 */
 	Vdev(nvlist_t *vdevConfig);
 
-	Guid		 GUID()		const;
-	Guid		 PoolGUID()	const;
-	vdev_state	 State()	const;
-	std::string	 Path()		const;
-	std::string	 PhysicalPath()	const;
+	virtual Guid	 	GUID()		const;
+	virtual Guid	 	PoolGUID()	const;
+	virtual vdev_state	State()		const;
+	std::string	 	Path()		const;
+	virtual std::string	PhysicalPath()	const;
 	std::string	 GUIDString()	const;
 	nvlist_t	*PoolConfig()	const;
 	nvlist_t	*Config()	const;

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201310142134.r9ELYnSd073042>