From: Brian Warner Date: Mon, 9 Mar 2009 03:07:32 +0000 (-0700) Subject: storage/immutable: raise a specific error upon seeing a bad version number, instead... X-Git-Tag: allmydata-tahoe-1.4.0~74 X-Git-Url: https://git.rkrishnan.org/banana.xhtml?a=commitdiff_plain;h=1a98521c3d0acc8b08c1e9365247d89277c3f9df;p=tahoe-lafs%2Ftahoe-lafs.git storage/immutable: raise a specific error upon seeing a bad version number, instead of using assert. Also wrap to 80cols. --- diff --git a/src/allmydata/storage/common.py b/src/allmydata/storage/common.py index 70f8fed0..865275bc 100644 --- a/src/allmydata/storage/common.py +++ b/src/allmydata/storage/common.py @@ -6,6 +6,8 @@ class DataTooLargeError(Exception): pass class UnknownMutableContainerVersionError(Exception): pass +class UnknownImmutableContainerVersionError(Exception): + pass def si_b2a(storageindex): diff --git a/src/allmydata/storage/immutable.py b/src/allmydata/storage/immutable.py index a6da31f0..363dc0dc 100644 --- a/src/allmydata/storage/immutable.py +++ b/src/allmydata/storage/immutable.py @@ -7,7 +7,8 @@ from allmydata.interfaces import RIBucketWriter, RIBucketReader from allmydata.util import base32, fileutil, log from allmydata.util.assertutil import precondition from allmydata.storage.lease import LeaseInfo -from allmydata.storage.common import DataTooLargeError +from allmydata.storage.common import UnknownImmutableContainerVersionError, \ + DataTooLargeError # each share file (in storage/shares/$SI/$SHNUM) contains lease information # and share data. The share data is accessed by RIBucketWriter.write and @@ -26,11 +27,13 @@ from allmydata.storage.common import DataTooLargeError # B+0x44: expiration time, 4 bytes big-endian seconds-since-epoch # B+0x48: next lease, or end of record -# Footnote 1: as of Tahoe v1.3.0 this field is not used by storage servers, but it is still -# filled in by storage servers in case the storage server software gets downgraded from >= Tahoe -# v1.3.0 to < Tahoe v1.3.0, or the share file is moved from one storage server to another. The -# value stored in this field is truncated, so If the actual share data length is >= 2**32, then -# the value stored in this field will be the actual share data length modulo 2**32. +# Footnote 1: as of Tahoe v1.3.0 this field is not used by storage servers, +# but it is still filled in by storage servers in case the storage server +# software gets downgraded from >= Tahoe v1.3.0 to < Tahoe v1.3.0, or the +# share file is moved from one storage server to another. The value stored in +# this field is truncated, so if the actual share data length is >= 2**32, +# then the value stored in this field will be the actual share data length +# modulo 2**32. class ShareFile: LEASE_SIZE = struct.calcsize(">L32s32sL") @@ -41,8 +44,8 @@ class ShareFile: self.home = filename self._max_size = max_size if create: - # touch the file, so later callers will see that we're working on it. - # Also construct the metadata. + # touch the file, so later callers will see that we're working on + # it. Also construct the metadata. assert not os.path.exists(self.home) fileutil.make_dirs(os.path.dirname(self.home)) f = open(self.home, 'wb') @@ -64,7 +67,10 @@ class ShareFile: filesize = os.path.getsize(self.home) (version, unused, num_leases) = struct.unpack(">LLL", f.read(0xc)) f.close() - assert version == 1, version + if version != 1: + msg = "sharefile %s had version %d but we wanted 1" % \ + (filename, version) + raise UnknownImmutableContainerVersionError(msg) self._num_leases = num_leases self._lease_offset = filesize - (num_leases * self.LEASE_SIZE) self._data_offset = 0xc @@ -74,9 +80,9 @@ class ShareFile: def read_share_data(self, offset, length): precondition(offset >= 0) - # reads beyond the end of the data are truncated. Reads that start beyond the end of the - # data return an empty string. - # I wonder why Python doesn't do the following computation for me? + # reads beyond the end of the data are truncated. Reads that start + # beyond the end of the data return an empty string. I wonder why + # Python doesn't do the following computation for me? seekpos = self._data_offset+offset fsize = os.path.getsize(self.home) actuallength = max(0, min(length, fsize-seekpos)) @@ -290,7 +296,9 @@ class BucketReader(Referenceable): self.shnum = shnum def __repr__(self): - return "<%s %s %s>" % (self.__class__.__name__, base32.b2a_l(self.storage_index[:8], 60), self.shnum) + return "<%s %s %s>" % (self.__class__.__name__, + base32.b2a_l(self.storage_index[:8], 60), + self.shnum) def remote_read(self, offset, length): start = time.time() diff --git a/src/allmydata/test/test_storage.py b/src/allmydata/test/test_storage.py index cff63037..f7b9783f 100644 --- a/src/allmydata/test/test_storage.py +++ b/src/allmydata/test/test_storage.py @@ -1,5 +1,5 @@ -import time, os.path, stat, re, simplejson +import time, os.path, stat, re, simplejson, struct from twisted.trial import unittest @@ -13,7 +13,7 @@ from allmydata.storage.server import StorageServer from allmydata.storage.mutable import MutableShareFile from allmydata.storage.immutable import BucketWriter, BucketReader from allmydata.storage.common import DataTooLargeError, storage_index_to_dir, \ - UnknownMutableContainerVersionError + UnknownMutableContainerVersionError, UnknownImmutableContainerVersionError from allmydata.storage.lease import LeaseInfo from allmydata.storage.crawler import BucketCountingCrawler from allmydata.storage.expirer import LeaseCheckingCrawler @@ -374,6 +374,24 @@ class Server(unittest.TestCase): for i,wb in writers.items(): wb.remote_abort() + def test_bad_container_version(self): + ss = self.create("test_bad_container_version") + a,w = self.allocate(ss, "si1", [0], 10) + w[0].remote_write(0, "\xff"*10) + w[0].remote_close() + + fn = os.path.join(ss.sharedir, storage_index_to_dir("si1"), "0") + f = open(fn, "rb+") + f.seek(0) + f.write(struct.pack(">L", 0)) # this is invalid: minimum used is v1 + f.close() + + b = ss.remote_get_buckets("allocate") + + e = self.failUnlessRaises(UnknownImmutableContainerVersionError, + ss.remote_get_buckets, "si1") + self.failUnless(" had version 0 but we wanted 1" in str(e), e) + def test_disconnect(self): # simulate a disconnection ss = self.create("test_disconnect")