Date: Fri, 11 Oct 2013 23:11:33 +0000 (UTC) From: Alan Somers <asomers@FreeBSD.org> To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r256360 - projects/zfsd/head/cddl/sbin/zfsd Message-ID: <201310112311.r9BNBX6Y080090@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: asomers Date: Fri Oct 11 23:11:33 2013 New Revision: 256360 URL: http://svnweb.freebsd.org/changeset/base/256360 Log: Style, organization, and syntax improvements to zfsd. cddl/sbin/zfsd/guid.cc: cddl/sbin/zfsd/guid.h: cddl/sbin/zfsd/vdev.h: cddl/sbin/zfsd/vdev.cc: cddl/sbin/zfsd/Makefile: o Since GUIDs pertain to more than just vdevs, move its implementation out into its own file. No functional changes. cddl/sbin/zfsd/case_file.cc: cddl/sbin/zfsd/zfsd.cc: cddl/sbin/zfsd/dev_ctl_event.cc: o Local varables declared in the same block are aligned by their first character. cddl/sbin/zfsd/case_file.cc: cddl/sbin/zfsd/zfsd.cc: cddl/sbin/zfsd/dev_ctl_event.cc: o Function arguments wrap to the first column after the function's opening '('. o syslog strings referencing a method use the fully qualified method name. o There are no spaces after '(' or before ')' in conditionals. o Boolean operators in conditionals are aligned on the right side and indented based on the level of their operation. o Cache return values in local variable to avoid the need to compress whitespace to make lines fit in 80 cols. o There is one newline between function declarations. cddl/sbin/zfsd/vdev.h: cddl/sbin/zfsd/vdev.cc: o Wrap comments at 76 cols (i.e. what fmt does by default). o Method names start with a capital letter. o Implementations are never inlined within a class definition. This just encourages consumers of an interface to depend unnecessarily upon the implementation. cddl/sbin/zfsd/zfsd.cc: o Simple checks in conditionals (like '==' and '!=') do not require extra parenthesis. cddl/sbin/zfsd/dev_ctl_event.cc: o Use a constant format string so that syslog() cannot be confused by random '%' characters in the string we are logging. Submitted by: gibbs Approved by: ken (mentor) Sponsored by: Spectra Logic Corporation Added: projects/zfsd/head/cddl/sbin/zfsd/guid.cc projects/zfsd/head/cddl/sbin/zfsd/guid.h Modified: projects/zfsd/head/cddl/sbin/zfsd/Makefile projects/zfsd/head/cddl/sbin/zfsd/case_file.cc projects/zfsd/head/cddl/sbin/zfsd/dev_ctl_event.cc projects/zfsd/head/cddl/sbin/zfsd/vdev.cc projects/zfsd/head/cddl/sbin/zfsd/vdev.h projects/zfsd/head/cddl/sbin/zfsd/zfsd.cc Modified: projects/zfsd/head/cddl/sbin/zfsd/Makefile ============================================================================== --- projects/zfsd/head/cddl/sbin/zfsd/Makefile Fri Oct 11 23:02:46 2013 (r256359) +++ projects/zfsd/head/cddl/sbin/zfsd/Makefile Fri Oct 11 23:11:33 2013 (r256360) @@ -4,11 +4,12 @@ PROG_CXX= zfsd SRCS= callout.cc \ case_file.cc \ dev_ctl_event.cc \ + guid.cc \ vdev.cc \ vdev_iterator.cc \ zfsd.cc \ zfsd_exception.cc \ - zpool_list.cc \ + zpool_list.cc \ zfsd_main.cc NO_MAN= YES Modified: projects/zfsd/head/cddl/sbin/zfsd/case_file.cc ============================================================================== --- projects/zfsd/head/cddl/sbin/zfsd/case_file.cc Fri Oct 11 23:02:46 2013 (r256359) +++ projects/zfsd/head/cddl/sbin/zfsd/case_file.cc Fri Oct 11 23:11:33 2013 (r256360) @@ -372,37 +372,36 @@ CaseFile::ReEvaluate(const ZfsEvent &eve bool CaseFile::ActivateSpare() { - nvlist_t *config, *nvroot; - nvlist_t **spares; - zpool_handle_t *zhp; - char *devPath, *vdev_type; - const char* poolname; - unsigned nspares, i; + nvlist_t *config, *nvroot; + nvlist_t **spares; + zpool_handle_t *zhp; + char *devPath, *vdev_type; + const char *poolname; + u_int nspares, i; + int error; ZpoolList zpl(ZpoolList::ZpoolByGUID, &m_poolGUID); if (zpl.empty()) { - syslog(LOG_ERR, "CaseFile::Replace: could not find pool for " - "pool_guid %ju", (uint64_t)m_poolGUID); + syslog(LOG_ERR, "CaseFile::ActivateSpare: Could not find pool " + "for pool_guid %ju.", (uint64_t)m_poolGUID); return (false); } zhp = zpl.front(); poolname = zpool_get_name(zhp); config = zpool_get_config(zhp, NULL); if (config == NULL) { - syslog(LOG_ERR, - "ActivateSpare: Could not find pool config for pool %s", - poolname); + syslog(LOG_ERR, "CaseFile::ActivateSpare: Could not find pool " + "config for pool %s", poolname); return (false); } if (nvlist_lookup_nvlist(config, ZPOOL_CONFIG_VDEV_TREE, &nvroot) != 0){ - syslog(LOG_ERR, - "ActivateSpare: Could not find vdev tree for pool %s", - poolname); + syslog(LOG_ERR, "CaseFile::ActivateSpare: Could not find vdev " + "tree for pool %s", poolname); return (false); } nspares = 0; nvlist_lookup_nvlist_array(nvroot, ZPOOL_CONFIG_SPARES, &spares, - &nspares); + &nspares); if (nspares == 0) { /* The pool has no spares configured */ return (false); @@ -413,14 +412,14 @@ CaseFile::ActivateSpare() { if (nvlist_lookup_uint64_array(spares[i], ZPOOL_CONFIG_VDEV_STATS, (uint64_t**)&vs, &nstats) != 0) { - syslog(LOG_ERR, "ActivateSpare: Could not find vdev " - "stats for pool %s, spare %d", - poolname, i); + syslog(LOG_ERR, "CaseFile::ActivateSpare: Could not " + "find vdev stats for pool %s, spare %d", + poolname, i); return (false); } - if ( (vs->vs_aux != VDEV_AUX_SPARED) - && (vs->vs_state == VDEV_STATE_HEALTHY)) { + if ((vs->vs_aux != VDEV_AUX_SPARED) + && (vs->vs_state == VDEV_STATE_HEALTHY)) { /* We found a usable spare */ break; } @@ -431,15 +430,19 @@ CaseFile::ActivateSpare() { return (false); } - if (nvlist_lookup_string(spares[i], ZPOOL_CONFIG_PATH, &devPath) != 0){ - syslog(LOG_ERR, "ActivateSpare: Cannot determine the path of " - "pool %s, spare %d", poolname, i); + error = nvlist_lookup_string(spares[i], ZPOOL_CONFIG_PATH, &devPath); + if (error != 0) { + syslog(LOG_ERR, "CaseFile::ActivateSpare: Cannot determine " + "the path of pool %s, spare %d. Error %d", + poolname, i, error); return (false); } - if (nvlist_lookup_string(spares[i], ZPOOL_CONFIG_TYPE, &vdev_type)!= 0){ - syslog(LOG_ERR, "ActivateSpare: Cannot determine the vdev type " - "of pool %s, spare %d", poolname, i); + error = nvlist_lookup_string(spares[i], ZPOOL_CONFIG_TYPE, &vdev_type); + if (error != 0) { + syslog(LOG_ERR, "CaseFile::ActivateSpare: Cannot determine " + "the vdev type of pool %s, spare %d. Error %d", + poolname, i, error); return (false); } @@ -728,7 +731,6 @@ CaseFile::SerializeEvList(const DevCtlEv } } - void CaseFile::Serialize() { @@ -800,15 +802,17 @@ CaseFile::OnGracePeriodEnded() if (zpool_vdev_degrade(zpl.front(), (uint64_t)m_vdevGUID, VDEV_AUX_ERR_EXCEEDED) == 0) { syslog(LOG_INFO, "Degrading vdev(%s/%s)", - PoolGUIDString().c_str(), VdevGUIDString().c_str()); + PoolGUIDString().c_str(), + VdevGUIDString().c_str()); Close(); return; } else { syslog(LOG_ERR, "Degrade vdev(%s/%s): %s: %s\n", - PoolGUIDString().c_str(), VdevGUIDString().c_str(), - libzfs_error_action(g_zfsHandle), - libzfs_error_description(g_zfsHandle)); + PoolGUIDString().c_str(), + VdevGUIDString().c_str(), + libzfs_error_action(g_zfsHandle), + libzfs_error_description(g_zfsHandle)); } } Serialize(); @@ -824,7 +828,7 @@ CaseFile::Replace(const char* vdev_type, ZpoolList zpl(ZpoolList::ZpoolByGUID, &m_poolGUID); if (zpl.empty()) { syslog(LOG_ERR, "CaseFile::Replace: could not find pool for " - "pool_guid %ju", (uint64_t)m_poolGUID); + "pool_guid %ju.", (uint64_t)m_poolGUID); return (false); } zhp = zpl.front(); Modified: projects/zfsd/head/cddl/sbin/zfsd/dev_ctl_event.cc ============================================================================== --- projects/zfsd/head/cddl/sbin/zfsd/dev_ctl_event.cc Fri Oct 11 23:02:46 2013 (r256359) +++ projects/zfsd/head/cddl/sbin/zfsd/dev_ctl_event.cc Fri Oct 11 23:11:33 2013 (r256360) @@ -165,7 +165,7 @@ DevCtlEvent::CreateEvent(const string &e EventFactoryRegistry::iterator foundMethod(s_factoryRegistry.find(key)); if (foundMethod == s_factoryRegistry.end()) { syslog(LOG_INFO, "DevCtlEvent::CreateEvent: unhandled event %s", - eventString.c_str()); + eventString.c_str()); return (NULL); } return ((foundMethod->second)(type, nvpairs, eventString)); @@ -684,7 +684,7 @@ ZfsEvent::Process() const msg << zpool_get_name(zpl.front()) << ","; msg << vdev.GUID() << ") "; msg << (queued ? "queued" : "dropped"); - syslog(priority, msg.str().c_str()); + syslog(priority, "%s", msg.str().c_str()); return; } } Added: projects/zfsd/head/cddl/sbin/zfsd/guid.cc ============================================================================== --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ projects/zfsd/head/cddl/sbin/zfsd/guid.cc Fri Oct 11 23:11:33 2013 (r256360) @@ -0,0 +1,62 @@ +/*- + * Copyright (c) 2012 Spectra Logic Corporation + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions, and the following disclaimer, + * without modification. + * 2. Redistributions in binary form must reproduce at minimum a disclaimer + * substantially similar to the "NO WARRANTY" disclaimer below + * ("Disclaimer") and any redistribution must be conditioned upon + * including a substantially similar Disclaimer requirement for further + * binary redistribution. + * + * NO WARRANTY + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * HOLDERS OR CONTRIBUTORS BE LIABLE FOR SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING + * IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGES. + * + * Authors: Alan Somers (Spectra Logic Corporation) + * + * $FreeBSD$ + */ + +/** + * \file guid.cc + * + * Implementation of the Guid class. + */ +#include <sys/cdefs.h> + +#include <stdlib.h> +#include <limits.h> +#include <inttypes.h> + +#include <sstream> +#include <string> + +#include "guid.h" + +__FBSDID("$FreeBSD$"); +/*=========================== Class Implementations ==========================*/ +/*----------------------------------- Guid -----------------------------------*/ +std::ostream& +operator<< (std::ostream& out, Guid g) +{ + if (g.IsValid()) + out << (uint64_t)g; + else + out << "None"; + return (out); +} Added: projects/zfsd/head/cddl/sbin/zfsd/guid.h ============================================================================== --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ projects/zfsd/head/cddl/sbin/zfsd/guid.h Fri Oct 11 23:11:33 2013 (r256360) @@ -0,0 +1,136 @@ +/*- + * Copyright (c) 2012 Spectra Logic Corporation + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions, and the following disclaimer, + * without modification. + * 2. Redistributions in binary form must reproduce at minimum a disclaimer + * substantially similar to the "NO WARRANTY" disclaimer below + * ("Disclaimer") and any redistribution must be conditioned upon + * including a substantially similar Disclaimer requirement for further + * binary redistribution. + * + * NO WARRANTY + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * HOLDERS OR CONTRIBUTORS BE LIABLE FOR SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING + * IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGES. + * + * Authors: Alan Somers (Spectra Logic Corporation) + * + * $FreeBSD$ + */ + +/** + * \file guid.h + * + * Definition of the Guid class. + */ +#ifndef _GUID_H_ +#define _GUID_H_ + +#include <string> + +/** + * \brief Object that represents guids. + * + * It can generally be manipulated as a uint64_t, but with a special + * value "None" that does not equal any valid guid. + * + * As of this writing, spa_generate_guid() in spa_misc.c explicitly + * refuses to return a guid of 0. So this class uses 0 as a flag + * value for "None". In the future, if 0 is allowed to be a valid + * guid, the implementation of this class must change. + */ +class Guid +{ +public: + /* Constructors */ + Guid(); + Guid(uint64_t guid); + + /* Assignment */ + Guid& operator=(const Guid& rhs); + + /* Test the validity of this guid. */ + bool IsValid() const; + + /* Comparison to other Guid operators */ + bool operator==(const Guid& rhs) const; + bool operator!=(const Guid& rhs) const; + + /* Integer conversion operators */ + operator uint64_t() const; + operator bool() const; + + static const uint64_t NONE_FLAG = 0; +protected: + /* The stored value. 0 is a flag for "None" */ + uint64_t m_GUID; +}; + +//- Guid Inline Public Methods ------------------------------------------------ +inline +Guid::Guid() + : m_GUID(NONE_FLAG) +{ +} + +inline +Guid::Guid(uint64_t guid) + : m_GUID(guid) +{ +} + +inline Guid& +Guid::operator=(const Guid &rhs) +{ + m_GUID = rhs.m_GUID; + return (*this); +} + +inline bool +Guid::IsValid() const +{ + return (m_GUID != NONE_FLAG); +} + +inline bool +Guid::operator==(const Guid& rhs) const +{ + return (m_GUID == rhs.m_GUID); +} + +inline bool +Guid::operator!=(const Guid& rhs) const +{ + return (m_GUID != rhs.m_GUID); +} + +inline +Guid::operator uint64_t() const +{ + return (m_GUID); +} + +inline +Guid::operator bool() const +{ + return (m_GUID != NONE_FLAG); +} + +/** Convert the GUID into its string representation */ +std::ostream& operator<< (std::ostream& out, Guid g); + +#endif /* _GUID_H_ */ Modified: projects/zfsd/head/cddl/sbin/zfsd/vdev.cc ============================================================================== --- projects/zfsd/head/cddl/sbin/zfsd/vdev.cc Fri Oct 11 23:02:46 2013 (r256359) +++ projects/zfsd/head/cddl/sbin/zfsd/vdev.cc Fri Oct 11 23:11:33 2013 (r256360) @@ -50,16 +50,6 @@ __FBSDID("$FreeBSD$"); using std::stringstream; /*=========================== Class Implementations ==========================*/ -/*----------------------------------- Guid -----------------------------------*/ -std::ostream& operator<< (std::ostream& out, Guid g){ - if (g.isValid()) - out << (uint64_t) g; - else - out << "None"; - return (out); -} - - /*----------------------------------- Vdev -----------------------------------*/ Vdev::Vdev(zpool_handle_t *pool, nvlist_t *config) : m_poolConfig(zpool_get_config(pool, NULL)), Modified: projects/zfsd/head/cddl/sbin/zfsd/vdev.h ============================================================================== --- projects/zfsd/head/cddl/sbin/zfsd/vdev.h Fri Oct 11 23:02:46 2013 (r256359) +++ projects/zfsd/head/cddl/sbin/zfsd/vdev.h Fri Oct 11 23:11:33 2013 (r256360) @@ -46,56 +46,7 @@ #include <sys/fs/zfs.h> #include <libzfs.h> - -/** - * \brief Object that represents guids. - * - * It can generally be manipulated as a uint64_t, but with a special value - * "None" that does not equal any valid guid. - * - * As of this writing, spa_generate_guid() in spa_misc.c explicitly refuses to - * return a guid of 0. So this class uses 0 as a flag value for "None". In the - * future, if 0 is allowed to be a valid guid, the implementation of this class - * must change. - */ -class Guid -{ -public: - /* Constructors */ - Guid(uint64_t guid) : m_GUID(guid) {}; - Guid() { m_GUID = NONE_FLAG; }; - - /* Assignment */ - Guid& operator=(const uint64_t& other) { - m_GUID = other; - return (*this); - }; - - /* Test the validity of this guid. */ - bool isValid() const { return ((bool)m_GUID); }; - - /* Comparison to other Guid operators */ - bool operator==(const Guid& other) const { - return (m_GUID == other.m_GUID); - }; - bool operator!=(const Guid& other) const { - return (m_GUID != other.m_GUID); - }; - - /* Integer conversion operators */ - operator uint64_t() const { return (m_GUID); }; - operator bool() const { return (m_GUID != NONE_FLAG); }; - -protected: - const static uint64_t NONE_FLAG = 0; - /* The stored value. 0 is a flag for "None" */ - uint64_t m_GUID; -}; - - -/** Convert the GUID into its string representation */ -std::ostream& operator<< (std::ostream& out, Guid g); - +#include "guid.h" /** * \brief Wrapper class for a vdev's name/value configuration list @@ -161,6 +112,7 @@ private: nvlist_t *m_config; }; +//- Vdev Inline Public Methods ------------------------------------------------ inline Guid Vdev::PoolGUID() const { Modified: projects/zfsd/head/cddl/sbin/zfsd/zfsd.cc ============================================================================== --- projects/zfsd/head/cddl/sbin/zfsd/zfsd.cc Fri Oct 11 23:02:46 2013 (r256359) +++ projects/zfsd/head/cddl/sbin/zfsd/zfsd.cc Fri Oct 11 23:11:33 2013 (r256360) @@ -518,16 +518,17 @@ ZfsDaemon::EventsPending() fds->events = POLLIN; fds->revents = 0; result = poll(fds, NUM_ELEMENTS(fds), /*timeout*/0); - } while ( (result == -1) && (errno == EINTR) ) ; + } while (result == -1 && errno == EINTR); if (result == -1) { /* Unexpected error; try reconnecting the socket */ - throw ZfsdException( - "ZfsdDaemon::EventsPending(): Unexpected error from poll()"); + throw ZfsdException("ZfsdDaemon::EventsPending(): " + "Unexpected error from poll()"); } if ((fds->revents & POLLHUP) != 0) { - /* The other end hung up the socket. Throw an exception + /* + * The other end hung up the socket. Throw an exception * so ZfsDaemon will try to reconnect */ throw ZfsdException("ZfsDaemon::EventsPending(): Got POLLHUP"); @@ -535,8 +536,8 @@ ZfsDaemon::EventsPending() if ((fds->revents & POLLERR) != 0) { /* Try reconnecting. */ - throw ZfsdException( - "ZfsdDaemon:EventsPending(): Got POLLERR. Reconnecting."); + throw ZfsdException("ZfsdDaemon:EventsPending(): Got POLLERR. " + " Reconnecting."); } return ((fds->revents & POLLIN) != 0); @@ -703,5 +704,3 @@ ZfsDaemon::EventLoop() } } } - -
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201310112311.r9BNBX6Y080090>