Thanks for the review. Please see inline. Shih-Hao
----- Original Message ----- > From: "Ben Pfaff" <b...@nicira.com> > To: "Shih-Hao Li" <shi...@nicira.com> > Cc: dev@openvswitch.org, "Shih-Hao Li" <shi...@vmware.com> > Sent: Tuesday, September 24, 2013 9:58:16 AM > Subject: Re: [ovs-dev] [PATCH] Check disk space in ovs-bugtool > On Tue, Sep 24, 2013 at 09:37:18AM -0700, Shih-Hao Li wrote: > > From: Shih-Hao Li <shi...@vmware.com> > > > > When output to a file, only allow 90% of the free disk space to be used. > This seems like a good idea. > The code to check for free space and then subtract the amount used if it > is allowable is repeated a couple of times. A helper function might be > a good idea. One could also improve the error message in the failure > case (the user will likely want to know how big an amount was attempted > and the size of the cap). I will refactor this. > It might be worth documenting how to avoid the cap if the user really > wants to (it looks like outputting to a fd instead of a file avoids > it?). How about adding a new parameter, such as the percentage of free disk space allowed to use? If it is 0, then no check. By default, always check and use 90 regardless output_fd or output_file cases. > ... > > +def get_free_disk_space(path): > > + path = os.path.abspath(path) > > + while not os.path.ismount(path): > > + path = os.path.dirname(path) > > + s = os.statvfs(path) > > + return s.f_frsize * s.f_bfree > What is the reason for chaining up to a mount point? All the > documentation I see says that statvfs returns information about the file > system containing the provided path and doesn't mention special > importance of mount points I actually borrowed the code from another NVP script. I think ismount may not do much here, but it does help tracking down to a valid path because we haven't created the tar file at this time.
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
