Adjusted StoreIOState::write() API to allow callers detect write errors.

The errors are reported without these changes, but only via callbacks, some of
which may be asynchronous. Callers such as doPages() would keep calling
write() after the failure. It is probably best to quit early instead of
ignoring "post-error" state in StoreIOState::write().

Added StoreIOState::write() API description.

diff --git a/src/StoreIOState.h b/src/StoreIOState.h
index 6f0f6d2676d28f37aff9c4361e17269656ac326a..8f1542f788f64c793c64e3c4bfd6f521422ea4b8 100644 (file)
--- a/src/StoreIOState.h
+++ b/src/StoreIOState.h
@@ -81,7 +81,11 @@ public:
     off_t offset() const;
 
     virtual void read_(char *buf, size_t size, off_t offset, STRCB * callback, void *callback_data) = 0;
-    virtual void write(char const *buf, size_t size, off_t offset, FREE * free_func) = 0;
+    /** write the given buffer and free it when it is no longer needed
+     *  \param offset zero for the very first write and -1 for all other writes
+     *  \retval false if write failed (callback has been or will be called)
+     */
+    virtual bool write(char const *buf, size_t size, off_t offset, FREE * free_func) = 0;
 
     typedef enum {
         wroteAll, ///< success: caller supplied all data it wanted to swap out

diff --git a/src/fs/coss/store_coss.h b/src/fs/coss/store_coss.h
index 3d0b1aa657d6e4c292cda562f0c0106f32a7b96f..f2afe872888a9eda1e44c6577e0dea5f558049da 100644 (file)
--- a/src/fs/coss/store_coss.h
+++ b/src/fs/coss/store_coss.h
@@ -75,8 +75,9 @@ public:
 
     CossMemBuf *locked_membuf;
     off_t st_size;
+    /* StoreIOState API */
     void read_(char *buf, size_t size, off_t offset, STRCB * callback, void *callback_data);
-    void write(char const *buf, size_t size, off_t offset, FREE * free_func);
+    virtual bool write(char const *buf, size_t size, off_t offset, FREE * free_func);
     virtual void close(int);
     void doCallback(int errflag);
     void lockMemBuf();

diff --git a/src/fs/coss/store_io_coss.cc b/src/fs/coss/store_io_coss.cc
index 8aac78ee1295d8269c76219e258cec3e2cd15736..f90c9c66668b4f083936dc65a10daeca49f75321 100644 (file)
--- a/src/fs/coss/store_io_coss.cc
+++ b/src/fs/coss/store_io_coss.cc
@@ -334,7 +334,7 @@ CossState::read_(char *buf, size_t size, off_t offset, STRCB * callback, void *c
     }
 }
 
-void
+bool
 CossState::write(char const *buf, size_t size, off_t offset, FREE * free_func)
 {
     char *dest;
@@ -360,6 +360,7 @@ CossState::write(char const *buf, size_t size, off_t offset, FREE * free_func)
         (free_func) ((char *)buf);
 
     ++ StoreFScoss::GetInstance().stats.write.success;
+    return true;
 }
 
 off_t

diff --git a/src/fs/ufs/UFSStoreState.cc b/src/fs/ufs/UFSStoreState.cc
index 74ba8b8800e2856ce3ad6d10ed142854f9db0ddd..849c4029cb12357c02f6276e7cf08d008cdacc3c 100644 (file)
--- a/src/fs/ufs/UFSStoreState.cc
+++ b/src/fs/ufs/UFSStoreState.cc
@@ -190,7 +190,7 @@ Fs::Ufs::UFSStoreState::read_(char *buf, size_t size, off_t aOffset, STRCB * aCa
  * writes and just do the write directly.  But for now we'll keep the
  * code simpler and always go through the pending_writes queue.
  */
-void
+bool
 Fs::Ufs::UFSStoreState::write(char const *buf, size_t size, off_t aOffset, FREE * free_func)
 {
     debugs(79, 3, "UFSStoreState::write: dirn " << swap_dirn  << ", fileno "<<
@@ -200,11 +200,12 @@ Fs::Ufs::UFSStoreState::write(char const *buf, size_t size, off_t aOffset, FREE
         debugs(79, DBG_IMPORTANT,HERE << "avoid write on theFile with error");
         debugs(79, DBG_IMPORTANT,HERE << "calling free_func for " << (void*) buf);
         free_func((void*)buf);
-        return;
+        return false;
     }
 
     queueWrite(buf, size, aOffset, free_func);
     drainWriteQueue();
+    return true;
 }
 
 /*

diff --git a/src/fs/ufs/UFSStoreState.h b/src/fs/ufs/UFSStoreState.h
index 4be8139702b833e84f8a05680f38adb3595caa36..701744f0598a3bc4f3bdbe5a1bc3723c752c0a91 100644 (file)
--- a/src/fs/ufs/UFSStoreState.h
+++ b/src/fs/ufs/UFSStoreState.h
@@ -58,8 +58,9 @@ public:
     bool closing;
     bool reading;
     bool writing;
+    /* StoreIOState API */
     void read_(char *buf, size_t size, off_t offset, STRCB * callback, void *callback_data);
-    void write(char const *buf, size_t size, off_t offset, FREE * free_func);
+    virtual bool write(char const *buf, size_t size, off_t offset, FREE * free_func);
 
 protected:
     virtual void doCloseCallback (int errflag);

diff --git a/src/store_swapout.cc b/src/store_swapout.cc
index f148c68aa170eac9532cb971cf4597232b40fec2..5c61634d36d79c26d51001d120eb22e4133663c4 100644 (file)
--- a/src/store_swapout.cc
+++ b/src/store_swapout.cc
@@ -123,7 +123,7 @@ storeSwapOutFileNotify(void *data, int errflag, StoreIOState::Pointer self)
     e->swap_dirn = mem->swapout.sio->swap_dirn;
 }
 
-static void
+static bool
 doPages(StoreEntry *anEntry)
 {
     MemObject *mem = anEntry->mem_obj;
@@ -134,7 +134,7 @@ doPages(StoreEntry *anEntry)
             mem->data_hdr.getBlockContainingLocation(mem->swapout.queue_offset);
 
         if (!page)
-            return; // wait for more data to become available
+            break; // wait for more data to become available
 
         // memNodeWriteComplete() and absence of buffer offset math below
         // imply that we always write from the very beginning of the page
@@ -158,15 +158,16 @@ doPages(StoreEntry *anEntry)
 
         mem->swapout.queue_offset += swap_buf_len;
 
-        storeIOWrite(mem->swapout.sio,
+        // Quit if write() fails. Sio is going to call our callback, and that
+        // will cleanup, but, depending on the fs, that call may be async.
+        const bool ok = mem->swapout.sio->write(
                      mem->data_hdr.NodeGet(page),
                      swap_buf_len,
                      -1,
                      memNodeWriteComplete);
 
-        /* the storeWrite() call might generate an error */
-        if (anEntry->swap_status != SWAPOUT_WRITING)
-            break;
+        if (!ok || anEntry->swap_status != SWAPOUT_WRITING)
+            return false;
 
         int64_t swapout_size = mem->endOffset() - mem->swapout.queue_offset;
 
@@ -175,8 +176,11 @@ doPages(StoreEntry *anEntry)
                 break;
 
         if (swapout_size <= 0)
-            return;
+            break;
     } while (true);
+
+    // either wait for more data or call swapOutFileClose()
+    return true;
 }
 
 /* This routine is called every time data is sent to the client side.
@@ -267,9 +271,7 @@ StoreEntry::swapOut()
     if (mem_obj->swapout.sio == NULL)
         return;
 
-    doPages(this);
-
-    if (mem_obj->swapout.sio == NULL)
+    if (!doPages(this))
         /* oops, we're not swapping out any more */
         return;