Opened 10 years ago

Closed 8 years ago

Last modified 8 years ago

#3833 closed Enhancement (fixed)

'freespace' argument for 'session-get' RPC method

Reported by: taem Owned by: jordan
Priority: Normal Milestone: 2.80
Component: Transmission Version: 2.13
Severity: Normal Keywords: patch
Cc: christophe.drevet@…

Description

Hello,

Please take a look at the proposed implementation of 'freespace' argument for 'session-get' RPC method. This argument returns available disk space (in KiB) for the default download directory for new torrents.

Am I going the right way?

Thanks.

Attachments (22)

0002-Display-available-disk-space-in-the-web-interface.patch (1.7 KB) - added by taem 10 years ago.
Display available disk space in the web interface
0001-Add-freespace-argument-for-session-get-RPC-method.patch (2.9 KB) - added by taem 10 years ago.
Add 'freespace' argument for 'session-get' RPC method
freespace.patch (3.4 KB) - added by reardon 10 years ago.
freespace.diff (3.1 KB) - added by reardon 10 years ago.
wrong diff, re-up
freespace2.diff (7.0 KB) - added by jordan 10 years ago.
0001-Add-check-for-disk-quota.patch (4.0 KB) - added by taem 10 years ago.
freespacequota.patch (3.4 KB) - added by taem 10 years ago.
freespacetorrent.patch (2.8 KB) - added by taem 10 years ago.
freespacequota2.patch (3.2 KB) - added by taem 10 years ago.
freespacetorrent2.patch (2.8 KB) - added by taem 10 years ago.
freespacequota3.patch (3.8 KB) - added by taem 10 years ago.
freespacetorrent3.patch (2.5 KB) - added by taem 10 years ago.
0001-Save-download-dir-block-device-for-the-next-tr_sessi.patch (4.0 KB) - added by taem 10 years ago.
0001-Save-download-dir-block-device-for-the-next-tr_torre.patch (2.2 KB) - added by taem 10 years ago.
freespace4.patch (9.9 KB) - added by taem 10 years ago.
0001-Check-for-available-quota-when-getting-free-disk-spa.patch (4.8 KB) - added by taem 8 years ago.
Check for available quota when getting free disk space
0002-Add-XFS-quota-support.patch (2.9 KB) - added by taem 8 years ago.
Add XFS quota support
0003-Cache-download-dir-s-block-device-and-FS-type.patch (4.9 KB) - added by taem 8 years ago.
Cache download dir's block device and FS type
0001-Port-quota-handling-to-BSD-and-Mac-OS-X.patch (1.7 KB) - added by taem 8 years ago.
0001-Fix-FTBFS-for-OS-X-and-uClibc.patch (1.3 KB) - added by taem 8 years ago.
0001-Headers-clean-up.patch (777 bytes) - added by taem 8 years ago.
0001-In-OS-X-disk-quota-s-info-already-in-bytes.patch (1.1 KB) - added by taem 8 years ago.

Download all attachments as: .zip

Change History (104)

Changed 10 years ago by taem

Display available disk space in the web interface

Changed 10 years ago by taem

Add 'freespace' argument for 'session-get' RPC method

comment:1 Changed 10 years ago by taem

  • Summary changed from Add 'freespace' argument for 'session-get' RPC method to 'freespace' argument for 'session-get' RPC method

comment:2 Changed 10 years ago by reardon

This is my current top feature request--I already wrote my own patch but not as clean (via platform.c) as this.

But why not use the POSIX function on Darwin? Then for WIN32 use GetDiskFreeSpaceEx?().

In rpcimpl.c, do a check for illegal value for freespace and only add the response term if you have a legal value (ie not -1).

Suggest you change the dictionary term to "download-dir-freespace" or "download-dir-df". Also recommend you just return the value in bytes and let the local formatter decide what to do. Though since you have to go thru JSON you will be constained to 253 instead of 264

Last edited 10 years ago by reardon (previous) (diff)

comment:3 Changed 10 years ago by reardon

Sorry I guess this really belongs at #1212. I for one do not need an elaborate solution that includes quota info. Rather I just want available space in the download-dir, as is proposed here.

Last edited 10 years ago by reardon (previous) (diff)

comment:4 Changed 10 years ago by charles

reardon, do you want to revise this ticket's patch to add the WIN32 and POSIX changes you described? Seems like win32 support in this ticket would be a nice-to-have. I'm not sure which function you're referring to for Darwin.

comment:5 Changed 10 years ago by reardon

Agree, WIN32 is nice-to-have. I don't have Win32 compilation ability, so I will only write pseudo-code and submit as comments.

By POSIX function, I meant the same statvfs() that is used for Linux et al. Darwin supports it so I don't see a reason to special-case Darwin.

I will modify taem's patch tonight if it hasn't been done already.

Changed 10 years ago by reardon

comment:6 Changed 10 years ago by reardon

Please see revised patch. This includes support in transmission-remote but does not update rpc-spec

comment:7 Changed 10 years ago by jordan

  • Cc rberber@… added

comment:8 follow-up: Changed 10 years ago by jordan

adding rb07 the CC list. he's been doing the Qt client builds on Windows and might be able to verify or correct the GetDiskFreeSpaceEx?() part of this patch.

comment:9 in reply to: ↑ 8 Changed 10 years ago by rb07

Replying to jordan:

adding rb07 the CC list. he's been doing the Qt client builds on Windows and might be able to verify or correct the GetDiskFreeSpaceEx() part of this patch.

The commented part looks fine, I would only add the missing include and change the variable type to uint64_t (Linux uses unsigned long on struct statvfs).

Answering the questions on the code's comments:

  • You don't need to convert to UTF-16, unless path is really a UTF-8 string, but then something else has to be done with the build (i.e. outside the code, to switch Windows API). Currently there is no handling of UTF-8 on Windows (other than what Qt does), so there are no conversions to UTF-16 on libtransmission; this could change when full support for foreign locales is added (Linux doesn't have it either).
  • AFAIK the path doesn't need the trailing back slash, that is building with MinGW; I haven't tested using shared drives or other UNC paths.

Since MinGW doesn't have statvfs() your solution is fine.

#ifndef WIN32 /* POSIX */ 
 #include <sys/statvfs.h> 
#else
 #include <windows.h>   /* or specifically <winbase.h> */
#endif

...

static uint64_t 
getFreeSpace( const char * path )
{
#ifdef WIN32

 uint64_t freeBytesAvailable = 0;

 if ( GetDiskFreeSpaceEx(path, &freeBytesAvailable, NULL, NULL) )
 {
     return freeByteAvailable;
 }
 else
 {
     return -1;
 }
#else /* POSIX */

...

comment:10 follow-up: Changed 10 years ago by reardon

<windows.h> is already included in platform.c so I think that would be redundant.

comment:11 Changed 10 years ago by jordan

very nice.

rb07: thanks for letting yourself be volunteered :)

comment:12 Changed 10 years ago by jordan

  • Component changed from libtransmission to Transmission
  • Milestone changed from None Set to 2.20
  • Owner changed from charles to jordan
  • Status changed from new to assigned

comment:13 in reply to: ↑ 10 Changed 10 years ago by rb07

There's an unreachable "return -1" at the end of the WIN32 section on the latest patch, and you may get a warning for returning uint64_t when the function is still defined as int64_t.

comment:14 Changed 10 years ago by reardon

If we want to maintain inline results then we are stuck with the uint->int conversion. gcc does not complain, fwiw.

Changed 10 years ago by reardon

wrong diff, re-up

comment:15 Changed 10 years ago by jordan

Another revision. Changes this time:

  • Added rpc spec documentation
  • Instead of omitting the rpc key, send it with a value of -1 when it can't be calculated
  • Added autoconf check for statvfs()
  • Avoid possible overflow when multiplying buf.f_bavail and buf.f_bsize

Please let me know if I've added any bugs in this patch. If it looks good to everyone I'll check it in.

Changed 10 years ago by jordan

comment:16 Changed 10 years ago by reardon

Small glitch in remote.c, I think you changed all of the other references to 'download-dir-free-space' (with hyphen) except here. Works fine with the change.

comment:17 Changed 10 years ago by jordan

  • Resolution set to fixed
  • Status changed from assigned to closed

04:41:22 < CIA-44> jordan * r11627 /trunk/ (8 files in 4 dirs): (trunk, daemon) #3833 "'freespace' argument for 'session-get' RPC method" -- committing patch from taem, reardon, and rb07

comment:18 Changed 10 years ago by taem

Hi all,

Sorry. I'm just returned from the holidays. Thanks for applying.

comment:19 follow-ups: Changed 10 years ago by Elbandi

Hi,

I'd like only to indicate, this report wrong info if the quota is set. but quota query is a bit complex :(

comment:20 in reply to: ↑ 19 Changed 10 years ago by taem

Replying to Elbandi:

Hi,

I'd like only to indicate, this report wrong info if the quota is set. but quota query is a bit complex :(

Hi,

You are right. I'm currently investigating this issue.

comment:21 in reply to: ↑ 19 Changed 10 years ago by taem

Replying to Elbandi:

Hi,

I'd like only to indicate, this report wrong info if the quota is set. but quota query is a bit complex :(

Elbandi, can you please test patch. I've added check for user quota. I'm not sure what to do when quota isn't enabled or there is error on getting quota info:

int64_t
tr_sessionGetDownloadDirFreeSpace( const tr_session * session )
{
    assert( tr_isSession( session ) );

    /* Check user quota first */
    {
        int64_t freespace;

        freespace = tr_getQuotaFreeSpace( session->downloadDir );
        /* If quota isn't enabled or there is error on getting its info */
        /* proceed to check available space by the usual way */
        if (freespace >= 0)
        {
            return freespace;
        }
    }

    return tr_getFreeSpace( session->downloadDir );
}

Also there are some dirty code to determine special block device, which is needed by the quotactl().

Changed 10 years ago by taem

comment:22 Changed 10 years ago by taem

Tested on Linux with local file system only.

comment:23 Changed 10 years ago by livings124

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:24 follow-up: Changed 10 years ago by Elbandi

i got some semester exams, i cannot test now. I added some tweak to your patch (malloc+strcpy -> tr_strdup): http://transmission.pastebin.com/WJzMGXgJ

But man getmntent says:

The getmntent() function reads the next line from the file system description file
fp and returns a pointer to a structure containing the broken out fields from
a line in the file. The pointer points to a static area of memory which is
overwritten by subsequent calls to getmntent(). 

so the returned mnt_fsname can used as device: http://transmission.pastebin.com/CqMu7jT1

(not tested, it's only a idea)

For quota man quotactl:

ESRCH 
No disc quota is found for the indicated user. 
Quotas have not been turned on for this filesystem. 

the second else in getquota is not nessesary.

comment:25 in reply to: ↑ 24 Changed 10 years ago by taem

Replying to Elbandi:

i got some semester exams, i cannot test now. I added some tweak to your patch (malloc+strcpy -> tr_strdup): http://transmission.pastebin.com/WJzMGXgJ

Cool. Less code. Better code.

But man getmntent says:

The getmntent() function reads the next line from the file system description file
fp and returns a pointer to a structure containing the broken out fields from
a line in the file. The pointer points to a static area of memory which is
overwritten by subsequent calls to getmntent(). 

so the returned mnt_fsname can used as device: http://transmission.pastebin.com/CqMu7jT1

(not tested, it's only a idea)

It's works. Thanks.

For quota man quotactl:

ESRCH 
No disc quota is found for the indicated user. 
Quotas have not been turned on for this filesystem. 

the second else in getquota is not nessesary.

Unfortunaly, both dqb_bsoftlimit=0 and dqb_bhardlimit=0 are valid. I guess it's relates to

The dqb_valid field is a bit mask that is set to indicate the entries in the dqblk structure that are valid. Currently, the kernel fills in all entries of the dqblk structure and marks them as valid in the dqb_valid field.

From the quotactl(2) linux manpage. I suppose, zero = quota is not enabled.

Please find attached the refreshed patch.

Changed 10 years ago by taem

comment:26 follow-ups: Changed 10 years ago by jordan

getblkdev(), getdev(), and getquota() should all be static functions

Other than that, is everyone who's contributed to this happy with freespace2.diff + freespacequota.patch?

I'd like to get this in before the 2.20 freeze :)

comment:27 in reply to: ↑ 26 ; follow-up: Changed 10 years ago by rb07

Replying to jordan:

Other than that, is everyone who's contributed to this happy with freespace2.diff + freespacequota.patch?

The Windows part doesn't need anything on the freespacequota.patch, the tr_getFreeSpace() function already returns the amount available to the user.

Quoting from the Microsoft manual:

"A pointer to a variable that receives the total number of free bytes on a disk that are available to the user who is associated with the calling thread.

...

If per-user quotas are being used, this value may be less than the total number of free bytes on a disk."

comment:28 in reply to: ↑ 26 Changed 10 years ago by taem

Replying to jordan:

Other than that, is everyone who's contributed to this happy with freespace2.diff + freespacequota.patch?

Yes.

comment:29 in reply to: ↑ 27 Changed 10 years ago by taem

Replying to rb07:

The Windows part doesn't need anything on the freespacequota.patch, the tr_getFreeSpace() function already returns the amount available to the user.

Perhaps, '#ifndef WIN32' should be moved to tr_sessionGetDownloadDirFreeSpace() from tr_getQuotaFreeSpace().

comment:30 Changed 10 years ago by taem

I just found a bug in daemon/remote.c:

if( tr_bencDictFindInt( args,  "download-dir-free-space", &i ) ) 
    printf( "  Download directory free space: %s\n",  strlsize( buf, i, sizeof buf ) );
static char*
strlsize( char * buf, int64_t bytes, size_t buflen )
{
    if( !bytes )
        tr_strlcpy( buf, "None", buflen );
    else
        tr_formatter_size_B( buf, bytes, buflen );

    return buf;
}

strlsize() passes int64_t type value to tr_formatter_size_B()'s uint64_t type argument (bytes).

comment:31 Changed 10 years ago by taem

Also, what do you think about to add available disk space info for incomplete directory?

comment:32 follow-up: Changed 10 years ago by Elbandi

Two things came to my mind:
first, if the torrentdir is a long path (with lots of '/'), the getdev reads the mtab/mounts many times. (mean: /this/is/the/torrent/path => 5 reads).
seconds, all sessionget with download-dir-free-space params need to resolve the quota device. this is unnecessary. the quota device is change, only when torrentdir is changed. (so we sould "cache" this device path, for example a RO variable near torrentdir).

comment:33 Changed 10 years ago by taem

Please find attached proposed patch which adds free space info for torrent download directory.

Rationale: the torrent and the default download directories can be different.

Last edited 10 years ago by taem (previous) (diff)

Changed 10 years ago by taem

comment:34 in reply to: ↑ 32 Changed 10 years ago by taem

Replying to Elbandi:

Two things came to my mind:
first, if the torrentdir is a long path (with lots of '/'), the getdev reads the mtab/mounts many times. (mean: /this/is/the/torrent/path => 5 reads).
seconds, all sessionget with download-dir-free-space params need to resolve the quota device. this is unnecessary. the quota device is change, only when torrentdir is changed. (so we sould "cache" this device path, for example a RO variable near torrentdir).

True.

comment:35 follow-up: Changed 10 years ago by jordan

Could someone attach a single diff that has all the changes needed to apply this to trunk? There are several revisions here which overlap.

comment:36 in reply to: ↑ 35 Changed 10 years ago by taem

Replying to jordan:

Could someone attach a single diff that has all the changes needed to apply this to trunk? There are several revisions here which overlap.

I can do that. Add freespacetorrent.patch to the diff? This patch adds new torrent-get argument.

comment:37 Changed 10 years ago by taem

Please find attached refreshed patch freespacequota.patch - freespacequota2.patch and refreshed freespacetorrent.patch - freespacetorrent2.patch. These patches refreshed against the trunk.

An issue described in comment:32 is in my todo for the next release.

Thanks.

Changed 10 years ago by taem

Changed 10 years ago by taem

comment:38 Changed 10 years ago by jordan

A couple of thoughts/suggestions

  • http://osdir.com/ml/apple.fink.devel/2003-03/msg00309.html says "OS X doesn't have a getmntent; use getmntinfo() instead. Autoconf provides a AC_FUNT_GETMNTENT macro to help find getmntent or let you know if it's not present." Not building properly on BSD / OS X is a showstopper... can this be fixed?
  • Since both session.c and torrent.c use tr_getFreeSpace() and tr_getQuotaFreeSpace() in the exact same way, I suggest we rename tr_getFreeSpace() as tr_getDiskFreeSpace(), making both it and tr_getQuotaFreeSpace() private to platform.c, and adding a public wrapper function tr_getFreeSpace():
platform.h:

    /** If the disk quota is enabled and readable, this returns how much is available in the quota.
        Otherwise, it returns how much is available on the disk, or -1 on error. */
    int64_t tr_getFreeSpace( const char * path );

platform.c:

    int64_t
    tr_getFreeSpace( const char * path )
    {
        int64_t i = tr_getQuotaFreeSpace( path );
        if( i < 0 )
            i = tr_getDiskFreeSpace( path );
        return i;
    }

torrent.c:

    int64_t
    tr_torrentGetDownloadDirFreeSpace( const tr_torrent * tor )
    {
        assert( tr_isTorrent( tor  ) );

        return tr_getFreeSpace( tor->downloadDir );
    }

session.c:

    int64_t
    tr_sessionGetDownloadDirFreeSpace( const tr_session * session )
    {
        assert( tr_isSession( session ) );

        return tr_getFreeSpace( session->downloadDir );
    }

comment:39 Changed 10 years ago by taem

Not building properly on BSD / OS X I hope it fixed.
Add a public wrapper function tr_getFreeSpace() fixed.

Please find attached patches.

Changed 10 years ago by taem

Changed 10 years ago by taem

comment:40 Changed 10 years ago by taem

  • Cc taem@… added

comment:41 Changed 10 years ago by jordan

  • Milestone changed from 2.20 to 2.30

bah, this should have gone in 2.20 but it's too late now.

comment:42 Changed 10 years ago by jordan

  • Milestone changed from 2.30 to Sometime

comment:43 Changed 10 years ago by taem

Hi,

Please find attached patches that fix an issue described in the comment:32.

0001-Save-download-dir-block-device-for-the-next-tr_sessi.patch depends on freespacequota3.patch. 0001-Save-download-dir-block-device-for-the-next-tr_torre.patch depends on freespacetorrent3.patch.

Changed 10 years ago by taem

comment:44 Changed 10 years ago by taem

Please find attached patch freespace4.patch. It's combines freespacequota3.patch, freespacetorrent3.patch, 0001-Save-download-dir-block-device-for-the-next-tr_sessi.patch and 0001-Save-download-dir-block-device-for-the-next-tr_torre.patch.

comment:45 Changed 10 years ago by taem

Hi,

Any comments on this ticket?

Thanks.

comment:46 follow-up: Changed 10 years ago by Opperpanter

Looks like we're almost there to get a working solution? I am in favour of scheduling it for the next release. Do you need me to test something?

comment:47 in reply to: ↑ 46 Changed 10 years ago by taem

Replying to Opperpanter:

Looks like we're almost there to get a working solution? I am in favour of scheduling it for the next release. Do you need me to test something?

Hi,

It needs XFS quota support. I suppose to create the patch this week.

Thanks.

comment:48 Changed 9 years ago by livings124

taem: ping

comment:49 Changed 8 years ago by dr4Ke

  • Cc christophe.drevet@… added

Changed 8 years ago by taem

Check for available quota when getting free disk space

Changed 8 years ago by taem

Add XFS quota support

Changed 8 years ago by taem

Cache download dir's block device and FS type

comment:50 Changed 8 years ago by taem

Hi,

Please find attached patches. Please ignore previous patches.

Thanks.

comment:51 Changed 8 years ago by jordan

Applied as-is in r13696, r13697, r13698.

Copyediting for whitespace/indentation/tabs in r13699.

comment:52 Changed 8 years ago by jordan

Replaced new use of PATH_MAX with TR_PATH_MAX in r13700.

comment:53 Changed 8 years ago by jordan

Looks like r13696 broke the Mac build: https://build.transmissionbt.com/job/trunk-mac/5511/console ... taem, do you have a fix for this?

comment:54 Changed 8 years ago by x190

Last edited 8 years ago by x190 (previous) (diff)

comment:55 follow-up: Changed 8 years ago by jordan

taem, another FTBFS from this: #5197

comment:56 Changed 8 years ago by taem

jordan, r13701 fixed Mac build?

comment:57 in reply to: ↑ 55 Changed 8 years ago by taem

Replying to jordan:

taem, another FTBFS from this: #5197

Ok.

comment:58 follow-up: Changed 8 years ago by jordan

Two more build errors on Mac,

  1. btodb() isn't being found
  1. the arguments to quotactl() differ between BSD and Linux.

Latest log's at https://build.transmissionbt.com/job/trunk-mac/5516/console

Last edited 8 years ago by livings124 (previous) (diff)

comment:59 in reply to: ↑ 58 Changed 8 years ago by taem

Replying to jordan:

Two more build errors on Mac,

  1. btodb() isn't being found
  2. the arguments to quotactl() differ between BSD and Linux.

Please find attached patch.

comment:60 Changed 8 years ago by taem

  • Cc taem@… removed

comment:61 Changed 8 years ago by jordan

r13714: 0001-Port-quota-handling-to-BSD-and-Mac-OS-X.patch from taem

comment:62 follow-up: Changed 8 years ago by jordan

taem, you un-cc'ed yourself on this and the other FTBFS ticket. Are you still here?

Still not building on OS X. Ain't platform code fun? :)

https://build.transmissionbt.com/job/trunk-mac/5525/console

comment:64 follow-up: Changed 8 years ago by JJTagy

dq.dqb_curblocks ?

comment:65 in reply to: ↑ 62 Changed 8 years ago by taem

Replying to jordan:

taem, you un-cc'ed yourself on this and the other FTBFS ticket. Are you still here?

Yes, I'm still receiving notifications :)

Still not building on OS X.

Ok.

Ain't platform code fun? :)

Yep :)

comment:66 in reply to: ↑ 64 Changed 8 years ago by taem

Replying to JJTagy:

dq.dqb_curblocks ?

Yes, there is typo.

comment:67 Changed 8 years ago by taem

Please find attached patch. I hope it fixed FTBFS.

Changed 8 years ago by taem

comment:68 Changed 8 years ago by taem

Please find attached patch that clean ups headers include.

Changed 8 years ago by taem

comment:69 follow-up: Changed 8 years ago by x190

For SYS_DARWIN return freespace; (should be non-bitshifted and is already in bytes?)

return (freespace < 0) ? 0 : freespace * 1024;

Last edited 8 years ago by x190 (previous) (diff)

comment:70 in reply to: ↑ 69 ; follow-up: Changed 8 years ago by taem

Replying to x190:

For SYS_DARWIN return freespace; (already in bytes?)

return (freespace < 0) ? 0 : freespace * 1024;

Maybe. I can't find reference for this. Can anyone test the patch?

comment:71 Changed 8 years ago by jordan

r13715: 0001-Fix-FTBFS-for-OS-X-and-uClibc.patch

r13716: 0001-Headers-clean-up.patch

comment:72 Changed 8 years ago by rb07

  • Cc rberber@… removed

comment:73 in reply to: ↑ 70 ; follow-up: Changed 8 years ago by x190

Replying to taem:

Replying to x190:

For SYS_DARWIN return freespace; (already in bytes?)

return (freespace < 0) ? 0 : freespace * 1024;

Maybe. I can't find reference for this. Can anyone test the patch?

OS X quota.h: (all in bytes)

struct dqblk {
	u_int64_t dqb_bhardlimit;	/* absolute limit on disk bytes alloc */
	u_int64_t dqb_bsoftlimit;	/* preferred limit on disk bytes */
	u_int64_t dqb_curbytes;	        /* current byte count */

Also, why the bitshift (div2?), if I may ask?

Last edited 8 years ago by x190 (previous) (diff)

comment:74 in reply to: ↑ 73 ; follow-up: Changed 8 years ago by taem

Replying to x190:

Replying to taem:

Replying to x190:

For SYS_DARWIN return freespace; (already in bytes?)

return (freespace < 0) ? 0 : freespace * 1024;

Maybe. I can't find reference for this. Can anyone test the patch?

OS X quota.h: (all in bytes)

struct dqblk {
	u_int64_t dqb_bhardlimit;	/* absolute limit on disk bytes alloc */
	u_int64_t dqb_bsoftlimit;	/* preferred limit on disk bytes */
	u_int64_t dqb_curbytes;	        /* current byte count */

Also, why the bitshift (div2?), if I may ask?

Do you run OS X? Would you please test attached patch?

comment:75 in reply to: ↑ 74 ; follow-up: Changed 8 years ago by x190

Replying to taem:

Do you run OS X? Would you please test attached patch?

Sorry, I can't build on 10.6. If you intend to return in KiB, then freespace / 1024 for SYS_DARWIN. FreeBSD bitshift---why? For all others---isn't the expected blocksize 512 KiB?

comment:76 in reply to: ↑ 75 Changed 8 years ago by taem

Replying to x190:

If you intend to return in KiB, then freespace / 1024 for SYS_DARWIN.

I guess, freespace should be returned in bytes like for other archs.

FreeBSD bitshift---why? For all others---isn't the expected blocksize 512 KiB?

Yes, thats why variables should be bitshifted (divided by 2) to get correct values, because, as you have mentioned, quota stored in 512-bytes blocks. In FreeBSD, quota implemented the same way as in Linux - http://lists.freebsd.org/pipermail/freebsd-hackers/2008-February/023346.html.

comment:77 follow-up: Changed 8 years ago by taem

And I'm not sure how OS X stores quota, in 512 or 1024 bytes.

Last edited 8 years ago by taem (previous) (diff)

comment:78 in reply to: ↑ 77 Changed 8 years ago by x190

Replying to taem:

And I'm not sure how OS X stores quota, in 512 or 1024 bytes.

I think you have it right as per your last patch. It was my mistake to think in terms of KiB instead of just bytes for blocksize and as shown by quota.h, OS X stores both limit and curbytes in bytes.

comment:79 follow-up: Changed 8 years ago by jordan

  • Resolution set to fixed
  • Status changed from reopened to closed

r13754: 0001-Headers-clean-up.patch

taem, if this finishes the ticket, go ahead and mark it as 'fixed' for 2.80 :)

comment:80 Changed 8 years ago by taem

jordan, ok :)

comment:81 in reply to: ↑ 79 Changed 8 years ago by x190

Replying to jordan:

r13754: 0001-Headers-clean-up.patch

taem, if this finishes the ticket, go ahead and mark it as 'fixed' for 2.80 :)

r13754 refers to 0001-In-OS-X-disk-quota-s-info-already-in-bytes.patch.

comment:82 Changed 8 years ago by livings124

  • Milestone changed from Sometime to 2.80
Note: See TracTickets for help on using tickets.