7179: Tighten Put requirements when overwriting existing data.

diff --git a/services/keepstore/volume.go b/services/keepstore/volume.go
index e5964e2676e395d72ed89d680002ac2ccda93fd8..daf003e1956a6dd1d951a8b17394ae7ec792ee71 100644 (file)
--- a/services/keepstore/volume.go
+++ b/services/keepstore/volume.go
@@ -57,7 +57,13 @@ type Volume interface {
        //
        // If a block is already stored under the same name (loc) with
        // different content, Put must either overwrite the existing
-       // data with the new data or return a non-nil error.
+       // data with the new data or return a non-nil error. When
+       // overwriting existing data, it must never leave the storage
+       // device in an inconsistent state: a subsequent call to Get
+       // must return either the entire old block, the entire new
+       // block, or an error. (An implementation that cannot peform
+       // atomic updates must leave the old data alone and return an
+       // error.)
        //
        // Put also sets the timestamp for the given locator to the
        // current time.

diff --git a/services/keepstore/volume_generic_test.go b/services/keepstore/volume_generic_test.go
index f1b27bece2b4885a20aa0662768d9bea4b3bc71d..5f68ef87de4268f2cc0e669a109fef6be43bfb3c 100644 (file)
--- a/services/keepstore/volume_generic_test.go
+++ b/services/keepstore/volume_generic_test.go
@@ -157,14 +157,24 @@ func testPutBlockWithDifferentContent(t *testing.T, factory TestableVolumeFactor
                t.Errorf("Got err putting block %q: %q, expected nil", TEST_BLOCK, err)
        }
 
-       // Whether Put with the same loc with different content fails or succeeds
-       // is implementation dependent. So, just check loc exists after overwriting.
-       // We also do not want to see if loc has block1 or block2, for the same reason.
-       if err = v.Put(TEST_HASH, TEST_BLOCK_2); err != nil {
-               t.Errorf("Got err putting block with different content %q: %q, expected nil", TEST_BLOCK, err)
-       }
-       if _, err := v.Get(TEST_HASH); err != nil {
-               t.Errorf("Got err getting block %q: %q, expected nil", TEST_BLOCK, err)
+       putErr := v.Put(TEST_HASH, TEST_BLOCK_2)
+       buf, getErr := v.Get(TEST_HASH)
+       if putErr == nil {
+               // Put must not return a nil error unless it has
+               // overwritten the existing data.
+               if bytes.Compare(buf, TEST_BLOCK_2) != 0 {
+                       t.Errorf("Put succeeded but Get returned %+v, expected %+v", buf, TEST_BLOCK_2)
+               }
+       } else {
+               // It is permissible for Put to fail, but it must
+               // leave us with either the original data, the new
+               // data, or nothing at all.
+               if getErr == nil && bytes.Compare(buf, TEST_BLOCK) != 0 && bytes.Compare(buf, TEST_BLOCK_2) != 0 {
+                       t.Errorf("Put failed but Get returned %+v, which is neither %+v nor %+v", buf, TEST_BLOCK, TEST_BLOCK_2)
+               }
+       }
+       if getErr == nil {
+               bufs.Put(buf)
        }
 }