svn commit: r256349 - projects/zfsd/head/cddl/sbin/zfsd
Alan Somers
asomers at FreeBSD.org
Fri Oct 11 21:24:34 UTC 2013
Author: asomers
Date: Fri Oct 11 21:24:33 2013
New Revision: 256349
URL: http://svnweb.freebsd.org/changeset/base/256349
Log:
Miscellaneous bug fixes in zfsd
* Created a new abstract Reader class that provides a common
interface to read from both file descriptors and std:istreams,
implemented as separate derived classes FDReader and
IstreamReader. Changed EventBuffer to get its input from a
Reader. Changed CaseFile::DeSerializeFile to open case files as
ifstreams instead of as file descriptors.
* Eliminated ZFSDaemon::ConnectToDevd's call to set the socket to
nonblocking mode. Instead, use an API that never attempts to read
more data than is available. EventBuffer::Fill() no longer ignore
EAGAIN, which shouldn't happen now that the socket blocks.
* Fix two file leaks
* Fix file leak in CaseFile::Serialize(): open() without close()
* Fix potential file leak in DeSerializeFile(): The exception
handler for ZfsdException did not close the open case file.
This code path was probably not exercised.
* Fix four memory leaks
* CaseFile::DeSerialize() was freeing its dirent structure when
it had entries, but not when the directory was empty
* zfsd did not purge case files on exit. This isn't really a
problem because the memory won't leak until the program quits,
but it clutters valgrind's output.
* ZfsDaemon::RescanSystem() wasn't deleting newly created events
after processing them.
* DeSerializeFile(): an ifstream was closed() instead of deleted.
* Miscellaneous style changes.
Submitted by: asomers
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/zfsd.cc
projects/zfsd/head/cddl/sbin/zfsd/zfsd.h
Modified: projects/zfsd/head/cddl/sbin/zfsd/case_file.cc
==============================================================================
--- projects/zfsd/head/cddl/sbin/zfsd/case_file.cc Fri Oct 11 21:23:44 2013 (r256348)
+++ projects/zfsd/head/cddl/sbin/zfsd/case_file.cc Fri Oct 11 21:24:33 2013 (r256349)
@@ -40,6 +40,7 @@
*/
#include <dirent.h>
#include <iomanip>
+#include <fstream>
#include <sstream>
#include <syslog.h>
#include <unistd.h>
@@ -53,6 +54,7 @@
/*============================ Namespace Control =============================*/
using std::auto_ptr;
using std::hex;
+using std::ifstream;
using std::stringstream;
using std::setfill;
using std::setw;
@@ -116,8 +118,12 @@ CaseFile::DeSerialize()
int numCaseFiles(scandir(s_caseFilePath.c_str(), &caseFiles,
DeSerializeSelector, /*compar*/NULL));
- if (numCaseFiles == 0 || numCaseFiles == -1)
+ if (numCaseFiles == -1)
return;
+ if (numCaseFiles == 0) {
+ free(caseFiles);
+ return;
+ }
for (int i = 0; i < numCaseFiles; i++) {
@@ -472,7 +478,7 @@ CaseFile::DeSerializeFile(const char *fi
string evString;
CaseFile *existingCaseFile(NULL);
CaseFile *caseFile(NULL);
- int fd(-1);
+ ifstream *caseStream(NULL);
try {
uintmax_t poolGUID;
@@ -519,28 +525,30 @@ CaseFile::DeSerializeFile(const char *fi
*/
caseFile = new CaseFile(Vdev(zpl.front(), vdevConf));
}
-
- fd = open(fullName.c_str(), O_RDONLY);
- if (fd == -1) {
+
+ caseStream = new ifstream(fullName.c_str());
+ if ( (caseStream == NULL) || (! *caseStream) ) {
throw ZfsdException("CaseFile::DeSerialize: Unable to "
"read %s.\n", fileName);
return;
}
+ IstreamReader caseReader(caseStream);
/* Re-load EventData */
- EventBuffer eventBuffer(fd);
+ EventBuffer eventBuffer(caseReader);
while (eventBuffer.ExtractEvent(evString)) {
DevCtlEvent *event(DevCtlEvent::CreateEvent(evString));
if (event != NULL)
caseFile->m_events.push_back(event);
}
- close(fd);
+ delete caseStream;
} catch (const ParseException &exp) {
exp.Log(evString);
if (caseFile != existingCaseFile)
delete caseFile;
- close(fd);
+ if (caseStream)
+ delete caseStream;
/*
* Since we can't parse the file, unlink it so we don't
@@ -552,6 +560,8 @@ CaseFile::DeSerializeFile(const char *fi
zfsException.Log();
if (caseFile != existingCaseFile)
delete caseFile;
+ if (caseStream)
+ delete caseStream;
}
}
@@ -632,6 +642,7 @@ CaseFile::Serialize()
write(fd, eventString.c_str(), eventString.length());
}
+ close(fd);
}
void
Modified: projects/zfsd/head/cddl/sbin/zfsd/zfsd.cc
==============================================================================
--- projects/zfsd/head/cddl/sbin/zfsd/zfsd.cc Fri Oct 11 21:23:44 2013 (r256348)
+++ projects/zfsd/head/cddl/sbin/zfsd/zfsd.cc Fri Oct 11 21:24:33 2013 (r256349)
@@ -43,6 +43,7 @@
#include <sys/cdefs.h>
#include <sys/disk.h>
+#include <sys/filio.h>
#include <sys/param.h>
#include <sys/poll.h>
#include <sys/socket.h>
@@ -86,6 +87,32 @@ const char g_devdSock[] = "/var/ru
int g_debug = 0;
libzfs_handle_t *g_zfsHandle;
+/*-------------------------------- FDReader -------------------------------*/
+//- FDReader Public Methods ----------------------------------------------------
+size_t
+FDReader::in_avail() const
+{
+ int bytes;
+ if (ioctl(m_fd, FIONREAD, &bytes)) {
+ syslog(LOG_ERR, "ioctl FIONREAD: %s", strerror(errno));
+ return (0);
+ }
+ return (bytes);
+}
+
+
+/*-------------------------------- IstreamReader ---------------------------*/
+//- IstreamReader Public Methods ----------------------------------------------
+ssize_t
+IstreamReader::read(char* buf, size_t count)
+{
+ m_stream->read(buf, count);
+ if (m_stream->fail())
+ return (-1);
+ return (m_stream->gcount());
+}
+
+
/*-------------------------------- EventBuffer -------------------------------*/
//- EventBuffer Static Data ----------------------------------------------------
/**
@@ -104,8 +131,8 @@ const char EventBuffer::s_eventEndTokens
const char EventBuffer::s_keyPairSepTokens[] = " \t\n";
//- EventBuffer Public Methods -------------------------------------------------
-EventBuffer::EventBuffer(int fd)
- : m_fd(fd),
+EventBuffer::EventBuffer(Reader& reader)
+ : m_reader(reader),
m_validLen(0),
m_parsedLen(0),
m_nextEventOffset(0),
@@ -187,7 +214,8 @@ EventBuffer::ExtractEvent(string &eventS
bool
EventBuffer::Fill()
{
- ssize_t result;
+ size_t avail;
+ ssize_t consumed(0);
/* Compact the buffer. */
if (m_nextEventOffset != 0) {
@@ -199,19 +227,26 @@ EventBuffer::Fill()
}
/* Fill any empty space. */
- result = read(m_fd, m_buf + m_validLen, MAX_READ_SIZE - m_validLen);
- if (result == -1) {
- if (errno == EINTR || errno == EAGAIN) {
- return (false);
- } else {
- err(1, "Read from devd socket failed");
+ avail = m_reader.in_avail();
+ if (avail) {
+ size_t want;
+
+ want = std::min(avail, MAX_READ_SIZE - m_validLen);
+ consumed = m_reader.read(m_buf + m_validLen, want);
+ if (consumed == -1) {
+ if (errno == EINTR) {
+ return (false);
+ } else {
+ err(1, "EventBuffer::Fill(): Read failed");
+ }
}
}
- m_validLen += result;
+
+ m_validLen += consumed;
/* Guarantee our buffer is always NUL terminated. */
m_buf[m_validLen] = '\0';
- return (result > 0);
+ return (consumed > 0);
}
/*--------------------------------- ZfsDaemon --------------------------------*/
@@ -220,6 +255,7 @@ bool ZfsDaemon::s_logCaseFil
bool ZfsDaemon::s_terminateEventLoop;
char ZfsDaemon::s_pidFilePath[] = "/var/run/zfsd.pid";
pidfh *ZfsDaemon::s_pidFH;
+FDReader* ZfsDaemon::s_reader;
int ZfsDaemon::s_devdSockFD = -1;
int ZfsDaemon::s_signalPipeFD[2];
bool ZfsDaemon::s_systemRescanRequested(false);
@@ -306,6 +342,7 @@ ZfsDaemon::Init()
void
ZfsDaemon::Fini()
{
+ PurgeCaseFiles();
ClosePIDFile();
}
@@ -387,10 +424,9 @@ ZfsDaemon::ConnectToDevd()
return (false);
}
- /* Don't block on reads. */
- if (fcntl(s_devdSockFD, F_SETFL, O_NONBLOCK) == -1)
- err(1, "Unable to enable nonblocking behavior on devd socket");
+ /* Connect the stream to the file descriptor */
+ s_reader = new FDReader(s_devdSockFD);
syslog(LOG_INFO, "Connection to devd successful");
return (true);
}
@@ -398,7 +434,10 @@ ZfsDaemon::ConnectToDevd()
void
ZfsDaemon::DisconnectFromDevd()
{
+ delete s_reader;
+ s_reader = NULL;
close(s_devdSockFD);
+ s_devdSockFD = -1;
}
void
@@ -448,8 +487,8 @@ ZfsDaemon::FlushEvents()
{
char discardBuf[256];
- while (read(s_devdSockFD, discardBuf, sizeof(discardBuf)) > 0)
- ;
+ while (s_reader->in_avail())
+ s_reader->read(discardBuf, sizeof(discardBuf));
}
bool
@@ -528,6 +567,7 @@ ZfsDaemon::RescanSystem()
event = DevCtlEvent::CreateEvent(evString);
if (event != NULL)
event->Process();
+ delete event;
}
}
}
@@ -561,7 +601,7 @@ ZfsDaemon::DetectMissedEvents()
void
ZfsDaemon::EventLoop()
{
- EventBuffer eventBuffer(s_devdSockFD);
+ EventBuffer eventBuffer(*s_reader);
while (s_terminateEventLoop == false) {
struct pollfd fds[2];
Modified: projects/zfsd/head/cddl/sbin/zfsd/zfsd.h
==============================================================================
--- projects/zfsd/head/cddl/sbin/zfsd/zfsd.h Fri Oct 11 21:23:44 2013 (r256348)
+++ projects/zfsd/head/cddl/sbin/zfsd/zfsd.h Fri Oct 11 21:24:33 2013 (r256349)
@@ -42,6 +42,7 @@
#define _ZFSD_H_
#include <cstdarg>
+#include <iostream>
#include <list>
#include <map>
#include <utility>
@@ -57,6 +58,7 @@
using std::auto_ptr;
using std::map;
using std::pair;
+using std::istream;
using std::string;
/*================================ Global Data ===============================*/
@@ -74,21 +76,131 @@ typedef int LeafIterFunc(zpool_handle_t
#define NUM_ELEMENTS(x) (sizeof(x) / sizeof(*x))
/*============================= Class Definitions ============================*/
+
+/*-------------------------------- Reader -------------------------------*/
+/**
+ * \brief A class that presents a common interface to both file descriptors and
+ * istreams .
+ *
+ * Standard C++ provides no way to create an iostream from a file descriptor or
+ * a FILE. The GNU, Apache, HPUX, and Solaris C++ libraries all provide
+ * non-standard ways to construct such a stream using similar semantics, but
+ * LLVM does not. Therefore this class is needed to ensure that zfsd can
+ * compile under LLVM. This class supports only the functionality needed by
+ * ZFSD; it does not implement the iostream API.
+ */
+class Reader
+{
+public:
+ /**
+ * \brief Return the number of bytes immediately available for reading
+ */
+ virtual size_t in_avail() const = 0;
+
+ /**
+ * \brief Reads up to count bytes
+ *
+ * Whether this call blocks depends on the underlying input source.
+ * On error, -1 is returned, and errno will be set by the underlying
+ * source.
+ *
+ * \param buf Destination for the data
+ * \param count Maximum amount of data to read
+ * \returns Amount of data that was actually read
+ */
+ virtual ssize_t read(char* buf, size_t count) = 0;
+};
+
+
+/*-------------------------------- FDReader -------------------------------*/
+/**
+ * \brief Specialization of Reader that uses a file descriptor
+ */
+class FDReader : public Reader
+{
+public:
+ /**
+ * \brief Constructor
+ *
+ * \param fd An open file descriptor. It will not be garbage
+ * collected by the destructor.
+ */
+ FDReader(int fd);
+
+ virtual size_t in_avail() const;
+
+ virtual ssize_t read(char* buf, size_t count);
+
+protected:
+ /** Copy of the underlying file descriptor */
+ int m_fd;
+};
+
+//- FDReader Inline Public Methods -----------------------------------------
+inline FDReader::FDReader(int fd)
+ : m_fd(fd)
+{
+}
+
+inline ssize_t
+FDReader::read(char* buf, size_t count)
+{
+ return (::read(m_fd, buf, count));
+}
+
+
+/*-------------------------------- IstreamReader------------------------------*/
+/**
+ * \brief Specialization of Reader that uses a std::istream
+ */
+class IstreamReader : public Reader
+{
+public:
+ /**
+ * Constructor
+ *
+ * \param stream Pointer to an open istream. It will not be
+ * garbage collected by the destructor.
+ */
+ IstreamReader(istream* stream);
+
+ virtual size_t in_avail() const;
+
+ virtual ssize_t read(char* buf, size_t count);
+
+protected:
+ /** Copy of the underlying stream */
+ istream* m_stream;
+};
+
+//- IstreamReader Inline Public Methods ----------------------------------------
+inline IstreamReader::IstreamReader(istream* stream)
+ : m_stream(stream)
+{
+}
+
+inline size_t
+IstreamReader::in_avail() const
+{
+ return (m_stream->rdbuf()->in_avail());
+}
+
+
/*-------------------------------- EventBuffer -------------------------------*/
/**
- * \brief Class buffering event data from Devd and splitting it
- * into individual event strings.
+ * \brief Class buffering event data from Devd or a similar source and
+ * splitting it into individual event strings.
*
- * Users of this class initialize it with the file descriptor associated
- * with the unix domain socket connection with devd. The lifetime of
- * an EventBuffer instance should match that of the file descriptor passed
- * to it. This is required as data from partially received events is
- * retained in the EventBuffer in order to allow reconstruction of these
- * events across multiple reads of the Devd file descriptor.
+ * Users of this class initialize it with a Reader associated with the unix
+ * domain socket connection with devd or a compatible source. The lifetime of
+ * an EventBuffer instance should match that of the Reader passed to it. This
+ * is required as data from partially received events is retained in the
+ * EventBuffer in order to allow reconstruction of these events across multiple
+ * reads of the stream.
*
- * Once the program determines that the Devd file descriptor is ready
- * for reading, the EventBuffer::ExtractEvent() should be called in a
- * loop until the method returns false.
+ * Once the program determines that the Reader is ready for reading, the
+ * EventBuffer::ExtractEvent() should be called in a loop until the method
+ * returns false.
*/
class EventBuffer
{
@@ -96,9 +208,9 @@ public:
/**
* Constructor
*
- * \param fd The file descriptor on which to buffer/parse event data.
+ * \param reader The data source on which to buffer/parse event data.
*/
- EventBuffer(int fd);
+ EventBuffer(Reader& reader);
/**
* Pull a single event string out of the event buffer.
@@ -163,8 +275,8 @@ private:
/** Temporary space for event data during our parsing. */
char m_buf[EVENT_BUFSIZE];
- /** Copy of the file descriptor linked to devd's domain socket. */
- int m_fd;
+ /** Reference to the reader linked to devd's domain socket. */
+ Reader& m_reader;
/** Valid bytes in m_buf. */
size_t m_validLen;
@@ -362,6 +474,11 @@ private:
static int s_devdSockFD;
/**
+ * Reader object used by the EventBuffer
+ */
+ static FDReader* s_reader;
+
+ /**
* Pipe file descriptors used to close races with our
* signal handlers.
*/
More information about the svn-src-projects
mailing list