On Thu, Dec 5, 2013 at 9:43 AM, Thomas Thrainer <[email protected]> wrote:
>> + def Create(cls, path, size, create_folders=False,
>> + _file_path_acceptance_fn=None):
>
>
> CreateFile would be a better name, as that's what is being created
> primarily.
>> + return FileDeviceHelper(path,
>> +
>> _file_path_acceptance_fn=_file_path_acceptance_fn)
>
>
> Why return a FileDeviceHelper here? The return value is nowhere actually
> used, right? And if the method is named CreateFile, returning a
> FileDeviceHelper would be weird anyway...
My idea is for the FileDeviceHelper to be essentially an object
representing the file itself in ways that are useful to the disk
template implementations. I think of this method as an alternative
constructor for files that don't yet exist (a "factory" if you will),
and as such I think it should return the "file object" that's been
created.
That said, no, I can't find a place where the return object is used
now. Please let me know if you still think it should return None and
be named CreateFile instead.
> I would perform this check in Create() and don't care about this in
> __init__. This way it's easier to test this class, and it's enough to verify
> the right path when we're creating the file, right?
You are right that Create must also check the path, but I'd rather
check it every single time as well in order to honor changes in the
allowed paths file.
> Didn't we agree on splitting this test method into a bunch of smaller ones
> with the common initialization logic in a setUp() method?
Yes; that's coming in at least in a second interdiff.
~~~
Here's the current interdiff before I administer the changes to the
respective commits.
diff --git a/lib/storage/filestorage.py b/lib/storage/filestorage.py
index d544363..185fdf4 100644
--- a/lib/storage/filestorage.py
+++ b/lib/storage/filestorage.py
@@ -19,7 +19,7 @@
# 02110-1301, USA.
-"""Disk-based disk templates and functions.
+"""Filesystem-based access functions and disk templates.
"""
@@ -37,6 +37,9 @@ from ganeti.storage import base
class FileDeviceHelper(object):
+ """Model a file in a file system for use as instance storage.
+
+ """
@classmethod
def Create(cls, path, size, create_folders=False,
@@ -46,10 +49,17 @@ class FileDeviceHelper(object):
@param size: the size in MiBs the file should be truncated to.
@param create_folders: create the directories for the path if necessary
(using L{ganeti.utils.io.Makedirs})
+
+ @rtype FileDeviceHelper
+ @return The FileDeviceHelper object representing the object.
@raise errors.FileStoragePathError: if the file path is
disallowed by policy
"""
+ if not _file_path_acceptance_fn:
+ _file_path_acceptance_fn = CheckFileStoragePathAcceptance
+ _file_path_acceptance_fn(path)
+
if create_folders:
folder = os.path.dirname(path)
io.Makedirs(folder)
@@ -74,8 +84,8 @@ class FileDeviceHelper(object):
"""
if not _file_path_acceptance_fn:
_file_path_acceptance_fn = CheckFileStoragePathAcceptance
-
_file_path_acceptance_fn(path)
+
self.path = path
def Exists(self, assert_exists=None):
@@ -126,7 +136,7 @@ class FileDeviceHelper(object):
except OSError as err:
base.ThrowError("%s: can't stat: %s", self.path, err)
- def Grow(self, amount):
+ def Grow(self, amount, dryrun, backingstore, excl_stor):
"""Grow the file
@param amount: the amount (in mebibytes) to grow by.
@@ -134,6 +144,12 @@ class FileDeviceHelper(object):
"""
# Check that the file exists
self.Exists(assert_exists=True)
+
+ if not backingstore:
+ return
+ if dryrun:
+ return
+
current_size = self.Size()
if amount < 0:
base.ThrowError("%s: can't grow by negative amount", self.path)
@@ -228,7 +244,7 @@ class FileStorage(base.BlockDev):
return
if dryrun:
return
- self.file.Grow(amount)
+ self.file.Grow(amount, amount, dryrun, backingstore, excl_stor)
def Attach(self):
"""Attach to an existing file.
diff --git a/lib/storage/gluster.py b/lib/storage/gluster.py
index 26ab201..fa1532b 100644
--- a/lib/storage/gluster.py
+++ b/lib/storage/gluster.py
@@ -359,11 +359,7 @@ class GlusterStorage(base.BlockDev):
@param amount: the amount (in mebibytes) to grow with
"""
- if not backingstore:
- return
- if dryrun:
- return
- self.file.Grow(amount)
+ self.file.Grow(amount, dryrun, backingstore, excl_stor)
def Attach(self):
"""Attach to an existing file.
--
Raffa Santi
Google Germany GmbH
Dienerstr. 12
80331 München
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
