[pacman-dev] Checking free space before transaction
Hi, I have implemented some basic disk space checking in pacman. The code is still a bit rough and there are some TODOs in there (especially related to output). But the basics are all there, so it seems time to get some comments. See my diskspace branch: http://projects.archlinux.org/users/allan/pacman.git/log/?h=diskspace Obviously I need to do some rebasing there, so it is probably easiest to look at the diff between that and my working branch: http://projects.archlinux.org/users/allan/pacman.git/diff/?h=diskspace&id2=working Here are some basic examples of what is implemented so far: > sudo src/pacman/pacman -S grub --dbpath /home/allan/tmp/db/ ... (1/1) checking for file conflicts [######################] 100% Partition: / - blocks needed: 112 - blocks free: 2635828 Partition: /boot - blocks needed: 1 - blocks free: 109046 (1/1) installing grub [######################] 100% > sudo src/pacman/pacman -S grub --dbpath /home/allan/tmp/db/ ... (1/1) checking for file conflicts [######################] 100% Partition: / - blocks needed: 0 - blocks free: 2635684 Partition: /boot - blocks needed: 0 - blocks free: 109044 (1/1) upgrading grub [######################] 100% It also handles the space freed by removing conflicts etc. Obviously the space calculations could be improved, e.g. - for /boot on installing grub, the /boot/grub directory should require a block in addition to the one used by the menu.lst file - directories get counted for removal even when they are not being removed... and that multiplies across packages - .PKGINFO etc count towards / (rather than to dbpath ???) - space for .pacnew/.pacsave files is ignored But it is close enough... In its current form the calculations should catch major stupidity. Allan
On Wed, Nov 10, 2010 at 01:47:52PM +1000, Allan McRae wrote:
Hi,
I have implemented some basic disk space checking in pacman. The code is still a bit rough and there are some TODOs in there (especially related to output). But the basics are all there, so it seems time to get some comments.
Didn't test this. But on that topic, may I suggest this: http://gitorious.org/pacman-bsd/pacman-bsd/commit/8b367d4441ba85b5548285d987... and this: http://gitorious.org/pacman-bsd/pacman-bsd/commit/cf3341fe591293a493bc925e04...
On Tue, Nov 9, 2010 at 10:47 PM, Allan McRae <allan@archlinux.org> wrote:
Hi,
I have implemented some basic disk space checking in pacman. The code is still a bit rough and there are some TODOs in there (especially related to output). But the basics are all there, so it seems time to get some comments.
I only glanced at the code so this might not be relevant. -Does it work if you have other partitions like /usr, /var , etc... ? -I like the idea of putting option in pacman.conf. If someone has an exotic partitionning scheme that makes pacman unable to calculate the actual disk space, then they can disable the disk space checking. -If you haven't done it: Checking the DBPath partition to see if it has enough space to put the db entries.
On Wed, Nov 10, 2010 at 1:16 PM, Eric Bélanger <snowmaniscool@gmail.com> wrote:
On Tue, Nov 9, 2010 at 10:47 PM, Allan McRae <allan@archlinux.org> wrote:
Hi,
I have implemented some basic disk space checking in pacman. The code is still a bit rough and there are some TODOs in there (especially related to output). But the basics are all there, so it seems time to get some comments.
I only glanced at the code so this might not be relevant.
-Does it work if you have other partitions like /usr, /var , etc... ?
-I like the idea of putting option in pacman.conf. If someone has an exotic partitionning scheme that makes pacman unable to calculate the actual disk space, then they can disable the disk space checking.
Your glance was much too quick. The whole point of this was to get rid of the dumbassness of "dur I have magic number size to hit" type checking. For this, you don't have to map partitions yourself, you don't do anything- it gets your partition map, figures out where each file is going to end up, sums up the necessary blocks per partition gained/lost (not even bytes which could still be wrong), and then finally decides whether the install can proceed. -Dan
On Wed, Nov 10, 2010 at 02:43:16PM -0600, Dan McGee wrote:
On Wed, Nov 10, 2010 at 1:16 PM, Eric Bélanger <snowmaniscool@gmail.com> wrote:
On Tue, Nov 9, 2010 at 10:47 PM, Allan McRae <allan@archlinux.org> wrote:
Hi,
I have implemented some basic disk space checking in pacman. The code is still a bit rough and there are some TODOs in there (especially related to output). But the basics are all there, so it seems time to get some comments.
I only glanced at the code so this might not be relevant.
-Does it work if you have other partitions like /usr, /var , etc... ?
-I like the idea of putting option in pacman.conf. If someone has an exotic partitionning scheme that makes pacman unable to calculate the actual disk space, then they can disable the disk space checking.
Your glance was much too quick. The whole point of this was to get rid of the dumbassness of "dur I have magic number size to hit" type checking. For this, you don't have to map partitions yourself, you don't do anything- it gets your partition map, figures out where each file is going to end up, sums up the necessary blocks per partition gained/lost (not even bytes which could still be wrong), and then finally decides whether the install can proceed.
-Dan
Good luck with that. If It eventually works, It would be great. I made some minor changes to make Allan's code compile in FreeBSD. I'm not sure the changes I made were correct(After all, I don't know much beyond hello world in C). But with those changes pacman did not like my ZFS pool. So, this code might only work in GNU or with only classic FSes.
On 11/11/10 07:29, Nezmer wrote:
On Wed, Nov 10, 2010 at 02:43:16PM -0600, Dan McGee wrote:
On Wed, Nov 10, 2010 at 1:16 PM, Eric Bélanger<snowmaniscool@gmail.com> wrote:
On Tue, Nov 9, 2010 at 10:47 PM, Allan McRae<allan@archlinux.org> wrote:
Hi,
I have implemented some basic disk space checking in pacman. The code is still a bit rough and there are some TODOs in there (especially related to output). But the basics are all there, so it seems time to get some comments.
I only glanced at the code so this might not be relevant.
-Does it work if you have other partitions like /usr, /var , etc... ?
-I like the idea of putting option in pacman.conf. If someone has an exotic partitionning scheme that makes pacman unable to calculate the actual disk space, then they can disable the disk space checking.
Your glance was much too quick. The whole point of this was to get rid of the dumbassness of "dur I have magic number size to hit" type checking. For this, you don't have to map partitions yourself, you don't do anything- it gets your partition map, figures out where each file is going to end up, sums up the necessary blocks per partition gained/lost (not even bytes which could still be wrong), and then finally decides whether the install can proceed.
-Dan
Good luck with that. If It eventually works, It would be great.
I made some minor changes to make Allan's code compile in FreeBSD. I'm not sure the changes I made were correct(After all, I don't know much beyond hello world in C). But with those changes pacman did not like my ZFS pool.
So, this code might only work in GNU or with only classic FSes.
This should be portable... although I have not tested on non-Linux systems yet. Any chance you want to give some actual error messages from the compilation? Or details of "did not like my ZFS pool"? Allan
On 11/11/10 07:29, Nezmer wrote:
But with those changes pacman did not like my ZFS pool.
From a bit of google, it looks like the statvfs call is probably returning EOVERFLOW on that so we can probably just assume LOTS of space and move on in those conditions... Allan
diff --git a/lib/libalpm/diskspace.c b/lib/libalpm/diskspace.c index 51c25ed..f382d3c 100644 --- a/lib/libalpm/diskspace.c +++ b/lib/libalpm/diskspace.c @@ -80,7 +80,7 @@ static alpm_list_t *mount_point_list() endmntent(fp); #elif defined HAVE_GETMNTINFO int entries; - struct statvfs *fsp; + struct statfs *fsp; entries = getmntinfo(&fsp, MNT_NOWAIT); @@ -89,12 +89,12 @@ static alpm_list_t *mount_point_list() } for(; entries-- > 0; fsp++) { - if((&fsp)->f_bfree != 0) { + if(fsp->f_bfree != 0) { MALLOC(mp, sizeof(alpm_mountpoint_t), RET_ERR(PM_ERR_MEMORY, NULL)); - mp->mount_dir = strdup(fsp->f_mntonname) + mp->mount_dir = strdup(fsp->f_mntonname); - MALLOC(mp->fsp, sizeof(struct statvfs), RET_ERR(PM_ERR_MEMORY, NULL)); - memcpy((void *)(mp->fsp), (void *)fsp, sizeof(struct statvfs)); + MALLOC(mp->fsp, sizeof(struct statfs), RET_ERR(PM_ERR_MEMORY, NULL)); + memcpy((void *)(mp->fsp), (void *)fsp, sizeof(struct statfs)); mp->blocks_needed = 0; mp->max_blocks_needed = 0;
On Thu, Nov 11, 2010 at 04:46:44PM +0200, Nezmer wrote:
On Thu, Nov 11, 2010 at 09:09:27AM +1000, Allan McRae wrote:
On 11/11/10 07:29, Nezmer wrote:
But with those changes pacman did not like my ZFS pool.
From a bit of google, it looks like the statvfs call is probably returning EOVERFLOW on that so we can probably just assume LOTS of space and move on in those conditions...
Allan
That's over-simplification. This is the quick patch I needed to make this even compile.
diff --git a/lib/libalpm/diskspace.c b/lib/libalpm/diskspace.c index 51c25ed..f382d3c 100644 --- a/lib/libalpm/diskspace.c +++ b/lib/libalpm/diskspace.c @@ -80,7 +80,7 @@ static alpm_list_t *mount_point_list() endmntent(fp); #elif defined HAVE_GETMNTINFO int entries; - struct statvfs *fsp; + struct statfs *fsp;
entries = getmntinfo(&fsp, MNT_NOWAIT);
from /usr/include/sys/mount.h: int getmntinfo(struct statfs **, int);
@@ -89,12 +89,12 @@ static alpm_list_t *mount_point_list() }
for(; entries-- > 0; fsp++) { - if((&fsp)->f_bfree != 0) { + if(fsp->f_bfree != 0) { MALLOC(mp, sizeof(alpm_mountpoint_t), RET_ERR(PM_ERR_MEMORY, NULL)); - mp->mount_dir = strdup(fsp->f_mntonname) + mp->mount_dir = strdup(fsp->f_mntonname);
- MALLOC(mp->fsp, sizeof(struct statvfs), RET_ERR(PM_ERR_MEMORY, NULL)); - memcpy((void *)(mp->fsp), (void *)fsp, sizeof(struct statvfs)); + MALLOC(mp->fsp, sizeof(struct statfs), RET_ERR(PM_ERR_MEMORY, NULL)); + memcpy((void *)(mp->fsp), (void *)fsp, sizeof(struct statfs));
mp->blocks_needed = 0; mp->max_blocks_needed = 0;
On 12/11/10 00:49, Nezmer wrote:
On Thu, Nov 11, 2010 at 04:46:44PM +0200, Nezmer wrote:
On Thu, Nov 11, 2010 at 09:09:27AM +1000, Allan McRae wrote:
On 11/11/10 07:29, Nezmer wrote:
But with those changes pacman did not like my ZFS pool.
From a bit of google, it looks like the statvfs call is probably returning EOVERFLOW on that so we can probably just assume LOTS of space and move on in those conditions...
Allan
That's over-simplification. This is the quick patch I needed to make this even compile.
diff --git a/lib/libalpm/diskspace.c b/lib/libalpm/diskspace.c index 51c25ed..f382d3c 100644 --- a/lib/libalpm/diskspace.c +++ b/lib/libalpm/diskspace.c @@ -80,7 +80,7 @@ static alpm_list_t *mount_point_list() endmntent(fp); #elif defined HAVE_GETMNTINFO int entries; - struct statvfs *fsp; + struct statfs *fsp;
entries = getmntinfo(&fsp, MNT_NOWAIT);
from /usr/include/sys/mount.h: int getmntinfo(struct statfs **, int);
And here comes the joys of various BSDs doing whatever they feel like... I have programmed based on the NetBSD man page which does use a statvfs struct...
On Thu, Nov 11, 2010 at 9:05 AM, Allan McRae <allan@archlinux.org> wrote:
On 12/11/10 00:49, Nezmer wrote:
On Thu, Nov 11, 2010 at 04:46:44PM +0200, Nezmer wrote:
On Thu, Nov 11, 2010 at 09:09:27AM +1000, Allan McRae wrote:
On 11/11/10 07:29, Nezmer wrote:
But with those changes pacman did not like my ZFS pool.
From a bit of google, it looks like the statvfs call is probably returning EOVERFLOW on that so we can probably just assume LOTS of space and move on in those conditions...
Allan
That's over-simplification. This is the quick patch I needed to make this even compile.
diff --git a/lib/libalpm/diskspace.c b/lib/libalpm/diskspace.c index 51c25ed..f382d3c 100644 --- a/lib/libalpm/diskspace.c +++ b/lib/libalpm/diskspace.c @@ -80,7 +80,7 @@ static alpm_list_t *mount_point_list() endmntent(fp); #elif defined HAVE_GETMNTINFO int entries; - struct statvfs *fsp; + struct statfs *fsp;
entries = getmntinfo(&fsp, MNT_NOWAIT);
from /usr/include/sys/mount.h: int getmntinfo(struct statfs **, int);
And here comes the joys of various BSDs doing whatever they feel like... I have programmed based on the NetBSD man page which does use a statvfs struct...
This seems to explain it well: http://stackoverflow.com/questions/1653163/difference-between-statvfs-and-st... FreeBSD also seems to define the function and type elsewhere, so we might just need to be smarter about what header we include: http://www.cygwin.com/ml/cygwin-developers/2003-12/msg00020.html So we shouldn't be using statfs() if possible. What was the error, Nezmer? Compile output would have been nice; is it that the type and function aren't even defined? Can you grep /usr/include/ for us to see if statvfs is defined but needs some sort of #define to turn it on? -Dan
On Thu, Nov 11, 2010 at 09:42:13AM -0600, Dan McGee wrote:
On Thu, Nov 11, 2010 at 9:05 AM, Allan McRae <allan@archlinux.org> wrote:
On 12/11/10 00:49, Nezmer wrote:
On Thu, Nov 11, 2010 at 04:46:44PM +0200, Nezmer wrote:
On Thu, Nov 11, 2010 at 09:09:27AM +1000, Allan McRae wrote:
On 11/11/10 07:29, Nezmer wrote:
But with those changes pacman did not like my ZFS pool.
From a bit of google, it looks like the statvfs call is probably returning EOVERFLOW on that so we can probably just assume LOTS of space and move on in those conditions...
Allan
That's over-simplification. This is the quick patch I needed to make this even compile.
diff --git a/lib/libalpm/diskspace.c b/lib/libalpm/diskspace.c index 51c25ed..f382d3c 100644 --- a/lib/libalpm/diskspace.c +++ b/lib/libalpm/diskspace.c @@ -80,7 +80,7 @@ static alpm_list_t *mount_point_list() endmntent(fp); #elif defined HAVE_GETMNTINFO int entries; - struct statvfs *fsp; + struct statfs *fsp;
entries = getmntinfo(&fsp, MNT_NOWAIT);
from /usr/include/sys/mount.h: int getmntinfo(struct statfs **, int);
And here comes the joys of various BSDs doing whatever they feel like... I have programmed based on the NetBSD man page which does use a statvfs struct...
This seems to explain it well: http://stackoverflow.com/questions/1653163/difference-between-statvfs-and-st...
FreeBSD also seems to define the function and type elsewhere, so we might just need to be smarter about what header we include: http://www.cygwin.com/ml/cygwin-developers/2003-12/msg00020.html
So we shouldn't be using statfs() if possible. What was the error, Nezmer? Compile output would have been nice; is it that the type and function aren't even defined? Can you grep /usr/include/ for us to see if statvfs is defined but needs some sort of #define to turn it on?
-Dan
I'm probably not the best person to answer those questions (Did I mention I don't know much in C ?). The 1st error was as I already mentioned in getmntinfo expecting statfs struct. statfs is defined in sys/mount.h : struct statfs { uint32_t f_version; /* structure version number */ uint32_t f_type; /* type of filesystem */ uint64_t f_flags; /* copy of mount exported flags */ uint64_t f_bsize; /* filesystem fragment size */ uint64_t f_iosize; /* optimal transfer block size */ uint64_t f_blocks; /* total data blocks in filesystem */ uint64_t f_bfree; /* free blocks in filesystem */ int64_t f_bavail; /* free blocks avail to non-superuser */ uint64_t f_files; /* total file nodes in filesystem */ int64_t f_ffree; /* free nodes avail to non-superuser */ uint64_t f_syncwrites; /* count of sync writes since mount */ uint64_t f_asyncwrites; /* count of async writes since mount */ uint64_t f_syncreads; /* count of sync reads since mount */ uint64_t f_asyncreads; /* count of async reads since mount */ uint64_t f_spare[10]; /* unused spare */ uint32_t f_namemax; /* maximum filename length */ uid_t f_owner; /* user that mounted the filesystem */ fsid_t f_fsid; /* filesystem id */ char f_charspare[80]; /* spare string space */ char f_fstypename[MFSNAMELEN]; /* filesystem type name */ char f_mntfromname[MNAMELEN]; /* mounted filesystem */ char f_mntonname[MNAMELEN]; /* directory on which mounted */ }; statvfs is defined in sys/statvfs.h : struct statvfs { fsblkcnt_t f_bavail; /* Number of blocks */ fsblkcnt_t f_bfree; fsblkcnt_t f_blocks; fsfilcnt_t f_favail; /* Number of files (e.g., inodes) */ fsfilcnt_t f_ffree; fsfilcnt_t f_files; unsigned long f_bsize; /* Size of blocks counted above */ unsigned long f_flag; unsigned long f_frsize; /* Size of fragments */ unsigned long f_fsid; /* Not meaningful */ unsigned long f_namemax; /* Same as pathconf(_PC_NAME_MAX) */ };
On Thu, Nov 11, 2010 at 10:03 AM, Nezmer <git@nezmer.info> wrote:
On Thu, Nov 11, 2010 at 09:42:13AM -0600, Dan McGee wrote:
On Thu, Nov 11, 2010 at 9:05 AM, Allan McRae <allan@archlinux.org> wrote:
On 12/11/10 00:49, Nezmer wrote:
On Thu, Nov 11, 2010 at 04:46:44PM +0200, Nezmer wrote:
On Thu, Nov 11, 2010 at 09:09:27AM +1000, Allan McRae wrote:
On 11/11/10 07:29, Nezmer wrote: > > But with those changes pacman did not like my ZFS pool.
From a bit of google, it looks like the statvfs call is probably returning EOVERFLOW on that so we can probably just assume LOTS of space and move on in those conditions...
Allan
That's over-simplification. This is the quick patch I needed to make this even compile.
diff --git a/lib/libalpm/diskspace.c b/lib/libalpm/diskspace.c index 51c25ed..f382d3c 100644 --- a/lib/libalpm/diskspace.c +++ b/lib/libalpm/diskspace.c @@ -80,7 +80,7 @@ static alpm_list_t *mount_point_list() endmntent(fp); #elif defined HAVE_GETMNTINFO int entries; - struct statvfs *fsp; + struct statfs *fsp;
entries = getmntinfo(&fsp, MNT_NOWAIT);
from /usr/include/sys/mount.h: int getmntinfo(struct statfs **, int);
And here comes the joys of various BSDs doing whatever they feel like... I have programmed based on the NetBSD man page which does use a statvfs struct...
This seems to explain it well: http://stackoverflow.com/questions/1653163/difference-between-statvfs-and-st...
FreeBSD also seems to define the function and type elsewhere, so we might just need to be smarter about what header we include: http://www.cygwin.com/ml/cygwin-developers/2003-12/msg00020.html
So we shouldn't be using statfs() if possible. What was the error, Nezmer? Compile output would have been nice; is it that the type and function aren't even defined? Can you grep /usr/include/ for us to see if statvfs is defined but needs some sort of #define to turn it on?
-Dan
I'm probably not the best person to answer those questions (Did I mention I don't know much in C ?).
The 1st error was as I already mentioned in getmntinfo expecting statfs struct.
statfs is defined in sys/mount.h :
struct statfs { uint32_t f_version; /* structure version number */ uint32_t f_type; /* type of filesystem */ uint64_t f_flags; /* copy of mount exported flags */ uint64_t f_bsize; /* filesystem fragment size */ uint64_t f_iosize; /* optimal transfer block size */ uint64_t f_blocks; /* total data blocks in filesystem */ uint64_t f_bfree; /* free blocks in filesystem */ int64_t f_bavail; /* free blocks avail to non-superuser */ uint64_t f_files; /* total file nodes in filesystem */ int64_t f_ffree; /* free nodes avail to non-superuser */ uint64_t f_syncwrites; /* count of sync writes since mount */ uint64_t f_asyncwrites; /* count of async writes since mount */ uint64_t f_syncreads; /* count of sync reads since mount */ uint64_t f_asyncreads; /* count of async reads since mount */ uint64_t f_spare[10]; /* unused spare */ uint32_t f_namemax; /* maximum filename length */ uid_t f_owner; /* user that mounted the filesystem */ fsid_t f_fsid; /* filesystem id */ char f_charspare[80]; /* spare string space */ char f_fstypename[MFSNAMELEN]; /* filesystem type name */ char f_mntfromname[MNAMELEN]; /* mounted filesystem */ char f_mntonname[MNAMELEN]; /* directory on which mounted */ };
statvfs is defined in sys/statvfs.h :
struct statvfs { fsblkcnt_t f_bavail; /* Number of blocks */ fsblkcnt_t f_bfree; fsblkcnt_t f_blocks; fsfilcnt_t f_favail; /* Number of files (e.g., inodes) */ fsfilcnt_t f_ffree; fsfilcnt_t f_files; unsigned long f_bsize; /* Size of blocks counted above */ unsigned long f_flag; unsigned long f_frsize; /* Size of fragments */ unsigned long f_fsid; /* Not meaningful */ unsigned long f_namemax; /* Same as pathconf(_PC_NAME_MAX) */ };
More potentially useful info to make this work a little better everywhere: http://www.gnu.org/software/hello/manual/autoconf/Particular-Functions.html#... -Dan
So, looking into this further: getmntent: used by glibc, uclibc, cygwin getmntinfo with statfs: used by bsd4.4 (FreeBSD, OpenBSD, OSX) getmntinfo with statvfs: NetBSD I can work with that... the mount point struct can hold a statfs or statvfs object depending on what function we are using. Given the entries that we need to access are available in both structs, the rest should be transparent. So what I need to do is: 1) figure out how to distinguish between the two getmntinfo versions... 2) add a code path for the second getmntinfo 3) review what headers need included in each path I could do with some help figuring out #1 as that is hitting the limits of my autoconf knowledge. Any ideas? Allan
On Fri, Nov 12, 2010 at 11:07:14AM +1000, Allan McRae wrote:
So, looking into this further:
getmntent: used by glibc, uclibc, cygwin
getmntinfo with statfs: used by bsd4.4 (FreeBSD, OpenBSD, OSX)
getmntinfo with statvfs: NetBSD
I can work with that... the mount point struct can hold a statfs or statvfs object depending on what function we are using. Given the entries that we need to access are available in both structs, the rest should be transparent.
So what I need to do is: 1) figure out how to distinguish between the two getmntinfo versions...
Wouldn't this work?: #ifndef __NetBSD__ /* use statfs */ #else /* use statvfs */ #endif
2) add a code path for the second getmntinfo 3) review what headers need included in each path
I could do with some help figuring out #1 as that is hitting the limits of my autoconf knowledge. Any ideas?
Allan
On 12/11/10 12:22, Nezmer wrote:
On Fri, Nov 12, 2010 at 11:07:14AM +1000, Allan McRae wrote:
So, looking into this further:
getmntent: used by glibc, uclibc, cygwin
getmntinfo with statfs: used by bsd4.4 (FreeBSD, OpenBSD, OSX)
getmntinfo with statvfs: NetBSD
I can work with that... the mount point struct can hold a statfs or statvfs object depending on what function we are using. Given the entries that we need to access are available in both structs, the rest should be transparent.
So what I need to do is: 1) figure out how to distinguish between the two getmntinfo versions...
Wouldn't this work?:
#ifndef __NetBSD__ /* use statfs */ #else /* use statvfs */ #endif
Sure, but that would be broken if some other BSD uses the same method as NetBSD so we are better off just doing the check properly in the first place.
On Fri, Nov 12, 2010 at 07:09:42PM +1000, Allan McRae wrote:
On 12/11/10 12:22, Nezmer wrote:
On Fri, Nov 12, 2010 at 11:07:14AM +1000, Allan McRae wrote:
So, looking into this further:
getmntent: used by glibc, uclibc, cygwin
getmntinfo with statfs: used by bsd4.4 (FreeBSD, OpenBSD, OSX)
getmntinfo with statvfs: NetBSD
I can work with that... the mount point struct can hold a statfs or statvfs object depending on what function we are using. Given the entries that we need to access are available in both structs, the rest should be transparent.
So what I need to do is: 1) figure out how to distinguish between the two getmntinfo versions...
Wouldn't this work?:
#ifndef __NetBSD__ /* use statfs */ #else /* use statvfs */ #endif
Sure, but that would be broken if some other BSD uses the same method as NetBSD so we are better off just doing the check properly in the first place.
Obviously, you know this stuff better than me. Using __FreeBSD__ __OpenBSD__ __NetBSD__ and __DragonFly__ seems to be the default way to distinguish BSDs though. Look at 'lib/isc/unix/net.c' in BIND sources for an example.
On Friday, November 12, 2010, Nezmer <git@nezmer.info> wrote:
On Fri, Nov 12, 2010 at 07:09:42PM +1000, Allan McRae wrote:
On 12/11/10 12:22, Nezmer wrote:
On Fri, Nov 12, 2010 at 11:07:14AM +1000, Allan McRae wrote:
So, looking into this further:
getmntent: used by glibc, uclibc, cygwin
getmntinfo with statfs: used by bsd4.4 (FreeBSD, OpenBSD, OSX)
getmntinfo with statvfs: NetBSD
I can work with that... the mount point struct can hold a statfs or statvfs object depending on what function we are using. Given the entries that we need to access are available in both structs, the rest should be transparent.
So what I need to do is: 1) figure out how to distinguish between the two getmntinfo versions...
Wouldn't this work?:
#ifndef __NetBSD__ /* use statfs */ #else /* use statvfs */ #endif
Sure, but that would be broken if some other BSD uses the same method as NetBSD so we are better off just doing the check properly in the first place.
Obviously, you know this stuff better than me.
Using __FreeBSD__ __OpenBSD__ __NetBSD__ and __DragonFly__ seems to be the default way to distinguish BSDs though. Look at 'lib/isc/unix/net.c' in BIND sources for an example.
You're addressing the wrong problem though. We could care less what OS we have, we care only about what data type is returned/expected by a function. If you look in the git history for pacman you'll see we've ripped such junk out of our code. -Dan
On 12/11/10 11:07, Allan McRae wrote:
So, looking into this further:
getmntent: used by glibc, uclibc, cygwin
getmntinfo with statfs: used by bsd4.4 (FreeBSD, OpenBSD, OSX)
getmntinfo with statvfs: NetBSD
I can work with that... the mount point struct can hold a statfs or statvfs object depending on what function we are using. Given the entries that we need to access are available in both structs, the rest should be transparent.
So what I need to do is: 1) figure out how to distinguish between the two getmntinfo versions...
So having a peak at how coreutils does this... Restating the prototype for a function does not create a compiler error. But stating a modified prototype for a function causes an error. So I need one of those "if this compiles" tests. Should be manageable.
2) add a code path for the second getmntinfo 3) review what headers need included in each path
Just to put my notes on the list: NetBSD: #include <sys/types.h> #include <sys/statvfs.h> FreeBSD/OSX: #include <sys/param.h> #include <sys/ucred.h> #include <sys/mount.h> OpenBSD: #include <sys/param.h> #include <sys/mount.h> Glibc: #include <stdio.h> #include <mntent.h> #include <sys/statvfs.h> Fun stuff... Allan
Hi Nezmer, Can you run test the updated autoconf stuff on my diskspace branch? Specifically, I would like to see the output from configure to do with getmntinfo to see if it detects the correct version to use. The actual building will still be broken... Thanks, Allan
On Sat, Nov 13, 2010 at 10:48:33PM +1000, Allan McRae wrote:
Hi Nezmer,
Can you run test the updated autoconf stuff on my diskspace branch? Specifically, I would like to see the output from configure to do with getmntinfo to see if it detects the correct version to use.
The actual building will still be broken...
Thanks, Allan
checking for getmntinfo... yes checking parameter style for getmntinfo... statfs
On 13/11/10 23:16, Nezmer wrote:
On Sat, Nov 13, 2010 at 10:48:33PM +1000, Allan McRae wrote:
Hi Nezmer,
Can you run test the updated autoconf stuff on my diskspace branch? Specifically, I would like to see the output from configure to do with getmntinfo to see if it detects the correct version to use.
The actual building will still be broken...
Thanks, Allan
checking for getmntinfo... yes checking parameter style for getmntinfo... statfs
Thanks. I have installed a FreeBSD VM and the diskspace checking is now working... sort of (the calculation of blocks used has issues exposed due to types being used and using ceil() requires a -lm added). But the basics are fine. As an aside, it was quite annoying to find that even the current pacman does not compile on FreeBSD but no bug has ever been reported here. So I had to hack around those issues before testing. (for those that are interested, FreeBSD does its own thing with time_t definitions, so the formatting for printf is not quite right...). Allan
On Sun, Nov 14, 2010 at 02:40:10AM +1000, Allan McRae wrote:
On 13/11/10 23:16, Nezmer wrote:
On Sat, Nov 13, 2010 at 10:48:33PM +1000, Allan McRae wrote:
Hi Nezmer,
Can you run test the updated autoconf stuff on my diskspace branch? Specifically, I would like to see the output from configure to do with getmntinfo to see if it detects the correct version to use.
The actual building will still be broken...
Thanks, Allan
checking for getmntinfo... yes checking parameter style for getmntinfo... statfs
Thanks. I have installed a FreeBSD VM and the diskspace checking is now working... sort of (the calculation of blocks used has issues exposed due to types being used and using ceil() requires a -lm added). But the basics are fine.
I forgot to mention that I used "LIBS='-lm'". My bad.
As an aside, it was quite annoying to find that even the current pacman does not compile on FreeBSD but no bug has ever been reported here. So I had to hack around those issues before testing. (for those that are interested, FreeBSD does its own thing with time_t definitions, so the formatting for printf is not quite right...).
Allan
What? pacman's C code *never* failed to compile on FreeBSD since I started using it (in January I think). I use gcc45 though so my environment is not exactly the same as the official one. Maybe GCC's include-fixed headers are doing magic. I can share ISOs, images If anyone needs a nice working FreeBSD/pacman environment.
On 14/11/10 09:03, Nezmer wrote:
On Sun, Nov 14, 2010 at 02:40:10AM +1000, Allan McRae wrote:
On 13/11/10 23:16, Nezmer wrote:
On Sat, Nov 13, 2010 at 10:48:33PM +1000, Allan McRae wrote:
Hi Nezmer,
Can you run test the updated autoconf stuff on my diskspace branch? Specifically, I would like to see the output from configure to do with getmntinfo to see if it detects the correct version to use.
The actual building will still be broken...
Thanks, Allan
checking for getmntinfo... yes checking parameter style for getmntinfo... statfs
Thanks. I have installed a FreeBSD VM and the diskspace checking is now working... sort of (the calculation of blocks used has issues exposed due to types being used and using ceil() requires a -lm added). But the basics are fine.
I forgot to mention that I used "LIBS='-lm'". My bad.
As an aside, it was quite annoying to find that even the current pacman does not compile on FreeBSD but no bug has ever been reported here. So I had to hack around those issues before testing. (for those that are interested, FreeBSD does its own thing with time_t definitions, so the formatting for printf is not quite right...).
What? pacman's C code *never* failed to compile on FreeBSD since I started using it (in January I think). I use gcc45 though so my environment is not exactly the same as the official one. Maybe GCC's include-fixed headers are doing magic.
I can share ISOs, images If anyone needs a nice working FreeBSD/pacman environment.
You do not compile with --enable-debug which turns on some more strict error checking (turns warnings into errors). Allan
On Sun, Nov 14, 2010 at 10:05:18AM +1000, Allan McRae wrote:
On 14/11/10 09:03, Nezmer wrote:
On Sun, Nov 14, 2010 at 02:40:10AM +1000, Allan McRae wrote:
On 13/11/10 23:16, Nezmer wrote:
On Sat, Nov 13, 2010 at 10:48:33PM +1000, Allan McRae wrote:
Hi Nezmer,
Can you run test the updated autoconf stuff on my diskspace branch? Specifically, I would like to see the output from configure to do with getmntinfo to see if it detects the correct version to use.
The actual building will still be broken...
Thanks, Allan
checking for getmntinfo... yes checking parameter style for getmntinfo... statfs
Thanks. I have installed a FreeBSD VM and the diskspace checking is now working... sort of (the calculation of blocks used has issues exposed due to types being used and using ceil() requires a -lm added). But the basics are fine.
I forgot to mention that I used "LIBS='-lm'". My bad.
As an aside, it was quite annoying to find that even the current pacman does not compile on FreeBSD but no bug has ever been reported here. So I had to hack around those issues before testing. (for those that are interested, FreeBSD does its own thing with time_t definitions, so the formatting for printf is not quite right...).
What? pacman's C code *never* failed to compile on FreeBSD since I started using it (in January I think). I use gcc45 though so my environment is not exactly the same as the official one. Maybe GCC's include-fixed headers are doing magic.
I can share ISOs, images If anyone needs a nice working FreeBSD/pacman environment.
You do not compile with --enable-debug which turns on some more strict error checking (turns warnings into errors).
Allan
With gcc45, I get: (1) --enable-debug: success (2) CFLAGS+=" -Werror": error util.c:883:15: error: static declaration of ‘strnlen’ follows non-static declaration /usr/include/string.h:105:9: note: previous declaration of ‘strnlen’ was here gmake[2]: *** [util.o] Error 1 gmake[2]: Leaving directory `<pacman-tree>/src/pacman' (3) CFLAGS+=" -Werror -DHAVE_STRNDUP": success
On 12/11/10 00:46, Nezmer wrote:
On Thu, Nov 11, 2010 at 09:09:27AM +1000, Allan McRae wrote:
On 11/11/10 07:29, Nezmer wrote:
But with those changes pacman did not like my ZFS pool.
From a bit of google, it looks like the statvfs call is probably returning EOVERFLOW on that so we can probably just assume LOTS of space and move on in those conditions...
That's over-simplification.
OK... care to explain is some detail that actually might be useful to fixing this?
On 11/11/10 05:16, Eric Bélanger wrote:
-If you haven't done it: Checking the DBPath partition to see if it has enough space to put the db entries.
I really do not care if someone does not have the ~1M total on their system to download the repo dbs... And we should get an error trying to open them directly from the tarbarll anyway. Allan
On Wed, Nov 10, 2010 at 5:30 PM, Allan McRae <allan@archlinux.org> wrote:
On 11/11/10 05:16, Eric Bélanger wrote:
-If you haven't done it: Checking the DBPath partition to see if it has enough space to put the db entries.
I really do not care if someone does not have the ~1M total on their system to download the repo dbs...
And we should get an error trying to open them directly from the tarbarll anyway.
Allan
I was refering to the local db entries that pacman writes to after installing/upgrading a package, not the sync ones. After re-reading your first email, you might have included it in the " - .PKGINFO etc count towards / (rather than to dbpath ???)" TODO item.
participants (4)
-
Allan McRae
-
Dan McGee
-
Eric Bélanger
-
Nezmer