Skip to content

Commit

Permalink
Replace messy disk_usage with existing method
Browse files Browse the repository at this point in the history
  • Loading branch information
@medariox
medariox committed Mar 30, 2016
1 parent fd59bdc commit 19efea6
Showing 1 changed file with 9 additions and 32 deletions.
41 changes: 9 additions & 32 deletions sickbeard/helpers.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1574,38 +1574,12 @@ def verify_freespace(src, dest, oldfile=None):

logger.log(u"Trying to determine free space on destination drive", logger.DEBUG)

if platform.system() == 'Windows':
import sys

def disk_usage(path):
_, total, free = ctypes.c_ulonglong(), ctypes.c_ulonglong(), ctypes.c_ulonglong()
if sys.version_info >= (3,) or isinstance(path, unicode):
method = ctypes.windll.kernel32.GetDiskFreeSpaceExW
else:
method = ctypes.windll.kernel32.GetDiskFreeSpaceExA
ret = method(path, ctypes.byref(_), ctypes.byref(total), ctypes.byref(free))
if ret == 0:
logger.log(u"Unable to determine free space, something went wrong", logger.WARNING)
raise ctypes.WinError()
return free.value

elif hasattr(os, 'statvfs'): # POSIX
def disk_usage(path):
import subprocess
call = subprocess.Popen(["df", path], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
output = call.communicate()[0]
return output.split("\n")[1].split()[3]

else:
logger.log(u"Unable to determine free space on your OS")
return True

if not ek(os.path.isfile, src):
logger.log("A path to a file is required for the source. {0} is not a file.".format(src), logger.WARNING)
return True

try:
diskfree = disk_usage(dest)
diskfree = getDiskSpaceUsage(dest, None)
except Exception:
logger.log(u"Unable to determine free space, so I will assume there is enough.", logger.WARNING)
return True
Expand All @@ -1620,8 +1594,8 @@ def disk_usage(path):
if diskfree > neededspace:
return True
else:
logger.log(u"Not enough free space: Needed: %s bytes ( %s ), found: %s bytes ( %s )"
% (neededspace, pretty_file_size(neededspace), diskfree, pretty_file_size(diskfree)), logger.WARNING)
logger.log(u"Not enough free space: Needed: {0} bytes ({1}), found: {2} bytes ({3})".format
(neededspace, pretty_file_size(neededspace), diskfree, pretty_file_size(diskfree)), logger.WARNING)
return False


Expand Down Expand Up @@ -1683,19 +1657,22 @@ def isFileLocked(checkfile, writeLockCheck=False):
return False


def getDiskSpaceUsage(diskPath=None):
def getDiskSpaceUsage(diskPath=None, pretty=True):
"""
returns the free space in human readable bytes for a given path or False if no path given
:param diskPath: the filesystem path being checked
:param pretty: return as Bytes if None
"""

if diskPath and ek(os.path.exists, diskPath):
if platform.system() == 'Windows':
free_bytes = ctypes.c_ulonglong(0)
ctypes.windll.kernel32.GetDiskFreeSpaceExW(ctypes.c_wchar_p(diskPath), None, None, ctypes.pointer(free_bytes))
return pretty_file_size(free_bytes.value)
return pretty_file_size(free_bytes.value) if pretty else free_bytes.value
else:
st = ek(os.statvfs, diskPath)
return pretty_file_size(st.f_bavail * st.f_frsize) # pylint: disable=no-member
file_size = st.f_bavail * st.f_frsize
return pretty_file_size(file_size) if pretty else file_size # pylint: disable=no-member
else:
return False

Expand Down

10 comments on commit 19efea6

@a10kiloham
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This definitely breaks on OSX in certain cases now without the special casing to use 'df'

@medariox
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bobbysteel
That's strange, statvfs should work just fine under OSX.
Do you know which OSX versions are affected? Do you have any debug log we can look at?

@a10kiloham
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately it's easily testable without a debug log. There seems to be some sort of overflow issue with the POSIX specification of a 32 bit long int. With 4TB+ storage volumes this causes (expected) unpredictable behavior. There doesn't seem to be a 64-bit version of statvfs on OSX (and there's not statvfs64) that I can find. 'df' works properly.

Example:

import ctypes
import os
import platform
import sys
print os.name
st = os.statvfs('/mnt/resources/TV')
print "Fr Size:",st.f_frsize
print "Bavail:",st.f_bavail
print "Bsize:",st.f_bsize
print "F_Frsize * F_Bavail / 1024:",st.f_frsize * st.f_bavail / 1024
import subprocess
call = subprocess.Popen(["df", "-k", "/mnt/Resources/TV"], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
output = call.communicate()[0]
print "df -k output:"
print output.split("\n")[1].split()[3]

Result:

Fr Size: 512
Bavail: 699520960
Bsize: 1048576
F_Frsize * F_Bavail / 1024: 349760480
df -k output:
4644727776

On this particular machine, the 4.6TB value is the correct space. This is a mount point to a Synology share.

@OmgImAlexis
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is with blocks and actual free space. This is caused by the implementation of SAMBA you're using.

Same issue here, we check Sickrage's issues to.

@a10kiloham
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep not trying to argue with you. But this is all plain vanilla OSX. Apple ignores bugs like this from what I've seen. How would you recommend this be fixed?

@OmgImAlexis
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since like I said on the other thread this isn't an issue at all for me and there are other people with the same issue as you with really custom Samba installs mainly which are from NAS devices like yours... It sounds more like an issue with the statvfs library than with OSX or anything else.

Also if it was a "normal" OSX issue there would be a LOT more people complaining. I'm running OSX on my laptop and there are no issues at all like that, even with 5TB+ of space free which is more than 4TB and more than the apparently "overflow" issue you keep talking about even though it states in the statvfs documentation that they use the underflow to handle over 4TB's worth of bytes.

@a10kiloham
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alexis - can you suggest what else is going on? This is Synology which is pretty well known and tested, and OSX. I can't explain it better than I have - I'm happy to troubleshoot further but these are pretty standard pieces of equipment, not hacky third party Samba installs. Can you suggest an alternate cause for this or another way to determine where in Apple's Samba implementation this is being caused? From my perspective yes it's an Apple bug. Dunno why statvfs would behave differently with network drives vs local drives but it is...

@OmgImAlexis
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not statvfs but the way Synology's Samba install is reporting free space vs free blocks. I'm guessing since Synology allows thin provisioning with that way the drives work it's not showing a correct amount of free blocks vs virtual free space.

Maybe contact them?

@a10kiloham
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strangely though statvfs works perfectly on the synology itself and no other systems except OSX seem to have this issue

@a10kiloham
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also mounting with NFS generates the same result so it doesn't seem like only the Synology's Samba install.

Please sign in to comment.