>> class FileDeviceHelper(object):
>> + """Model a file in a file system for use as instance storage.
>> +
>> + """
>
>
> FileDeviceHelper is what it says it is: a helper class dealing with file
> devices. If it was modeling a file, the class would be named File, right? So
> I don't quite like this misleading comment...
Okay, I'll drop the docstring then.
>> - def Grow(self, amount):
>> + def Grow(self, amount, dryrun, backingstore, excl_stor):
>
> Why excl_stor?
I figured that if we wanted to merge the method implementation between
the FileStorage and GlusterStorage methods, including the handling of
additional parameters, then it made sense to pass *all* parameters to
the Helper even if excl_storage is ignored.
Second interdiff, also to be split in two fixups:
diff --git a/lib/storage/filestorage.py b/lib/storage/filestorage.py
index 185fdf4..1bafef0 100644
--- a/lib/storage/filestorage.py
+++ b/lib/storage/filestorage.py
@@ -37,13 +37,10 @@ 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,
- _file_path_acceptance_fn=None):
+ def CreateFile(cls, path, size, create_folders=False,
+ _file_path_acceptance_fn=CheckFileStoragePathAcceptance):
"""Create a new file and its file device helper.
@param size: the size in MiBs the file should be truncated to.
@@ -56,8 +53,6 @@ class FileDeviceHelper(object):
"""
- if not _file_path_acceptance_fn:
- _file_path_acceptance_fn = CheckFileStoragePathAcceptance
_file_path_acceptance_fn(path)
if create_folders:
@@ -76,14 +71,12 @@ class FileDeviceHelper(object):
_file_path_acceptance_fn=_file_path_acceptance_fn)
def __init__(self, path,
- _file_path_acceptance_fn=None):
+ _file_path_acceptance_fn=_file_path_acceptance_fn):
"""Create a new file device helper.
@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)
self.path = path
@@ -145,14 +138,15 @@ class FileDeviceHelper(object):
# Check that the file exists
self.Exists(assert_exists=True)
+ if amount < 0:
+ base.ThrowError("%s: can't grow by negative amount", self.path)
+
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)
new_size = current_size + amount * 1024 * 1024
try:
f = open(self.path, "a+")
@@ -286,7 +280,7 @@ class FileStorage(base.BlockDev):
dev_path = unique_id[1]
- FileDeviceHelper.Create(dev_path, size)
+ FileDeviceHelper.CreateFile(dev_path, size)
return FileStorage(unique_id, children, size, params, dyn_params)
diff --git a/lib/storage/gluster.py b/lib/storage/gluster.py
index 9ce76c3..d4f9650 100644
--- a/lib/storage/gluster.py
+++ b/lib/storage/gluster.py
@@ -431,6 +431,6 @@ class GlusterStorage(base.BlockDev):
# Possible optimization: defer actual creation to first Attach, rather
# than mounting and unmounting here, then remounting immediately after.
with volume_obj:
- FileDeviceHelper.Create(full_path, size, create_folders=True)
+ FileDeviceHelper.CreateFile(full_path, size, create_folders=True)
return GlusterStorage(unique_id, children, size, params, dyn_params)
--
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
