Hi!

On Tue, May 14, 2013 at 7:14 PM, Bernardo Dal Seno <[email protected]>wrote:

> On 14 May 2013 18:30, Helga Velroyen <[email protected]> wrote:
> > This adds functionality to retrieve disk space information
> > for file storage. It calls the 'df' tool and parses its
> > output to extract the total and free amount of disk space
> > on the disk where the given path is located.
> > The code is not integrated yet, but thoroughly unit-tested.
> >
> > Signed-off-by: Helga Velroyen <[email protected]>
>
> > diff --git a/lib/storage/filestorage.py b/lib/storage/filestorage.py
> > new file mode 100644
> > index 0000000..47152c8
> > --- /dev/null
> > +++ b/lib/storage/filestorage.py
> > @@ -0,0 +1,78 @@
> > +#
> > +#
> > +
> > +# Copyright (C) 2011, 2012 Google Inc.
>
> This should be just 2013.
>

Sure.


>
> > +def GetSpaceInfo(path, _parsefn=_ParseDfResult):
> > +  """Retrieves the free and total space of the device where the file is
> > +     located.
> > +
> > +     @type path: string
> > +     @param path: Path of the file whose embracing device's capacity is
> > +       reported.
> > +     @type: _parse_fn: function
> > +     @param: _parse_fn: Function that parses the output of the 'df'
> command;
> > +       given as parameter to make this code more testable.
> > +     @return: a JSON dictionary containing 'vg_size' and 'vg_free'
>
> Why don't you return just a normal dictionary? Where do you plan to use it?
>

In the RPC call NodeInfo. You are right, I guess it should be a normal dict
at this point. I'll send an interdiff.


>
> > diff --git a/test/py/ganeti.storage.filestorage_unittest.py b/test/py/
> ganeti.storage.filestorage_unittest.py
> > new file mode 100755
> > index 0000000..90d5c26
> > --- /dev/null
> > +++ b/test/py/ganeti.storage.filestorage_unittest.py
> > @@ -0,0 +1,96 @@
> > +#!/usr/bin/python
> > +#
> > +
> > +# Copyright (C) 2006, 2007, 2010, 2012, 2013 Google Inc.
>
> Again, just 2013.
>

Sure.


>
> > +  def testParseDfOutputValidInput(self):
> > +    """Tests that parsing of the out put of 'df' works correctly.
>
> s/out put/output/
>

Sure.


>
> > +  def testParseDfOutputInvalidInput(self):
> > +    """Tests that parsing of the out put of 'df' works correctly.
>
> s/out put/output/
>

Sure.


>
> > +if __name__ == "__main__":
> > +  testutils.GanetiTestProgram()
> > +
> > +if __name__ == "__main__":
> > +  testutils.GanetiTestProgram()
>
> These lines are needlessly repeated.
>

Oops, true.

Cheers,
Helga

Reply via email to