- User Since
- Oct 24 2014, 7:17 PM (292 w, 4 d)
Tue, May 19
Finally, this looks good to go.
Fri, May 15
I believe that this change is correct as it stands.
Can you run one more test run over this code just to be sure that it patches cleanly and that none of our final changes broke anything?
Wed, May 13
This looks good to me. I am fine with not supporting the old behavior.
Sat, May 9
May 3 2020
This rather long block is a summary of the discussion that Kostik
and I had in email. Dropping it in here so that it is part of the
public record. Regular font are my comments; italics font are
Kostik's response to my comments.
As imp has noted, we have had readdir(3) and the getdirents(2) for nearly 40 years and many filesystems do not support directly reading directories. While I personally still use od(1) to inspect UFS directories, that is a pretty lame reason for keeping the functionality. And returning EISDIR will make us more compatible with Linux and remain in compliance with POSIX. So, all told I am in favor of this change. Adding a sysctl to revert to the old behavior seems reasonable so long as it defaults to the new (EISDIR) behavior.
Apr 20 2020
I concur with this change.
It should be MFC'ed to stable/12 and stable/11.
Apr 15 2020
Apr 10 2020
Apr 9 2020
Apr 6 2020
Apr 4 2020
Apr 3 2020
Changes moved to D24210.
These changes will be done as part of D24210 so that all the updates to make the dump and restore utilities compile with -fno-common are in one place.
Rather than using EXTERN, place each global declaration into the file in which it is primarily used.
Taking over with Kyle Evans blessing.
Mar 31 2020
Now looks good.
Rather than doing EXTERN, move the explicit declarations into file as this:
mapsize - main.c
usedinomap - main.c
dumpdirmap - main.c
dumpinomap - main.c
disk - main.c
tape - main.c
popenout - main.c
dumpdates - itime.c
temp - unused, remove from dump.h, main.c, and definition of _PATH_DTMP from pathnames.h
lastlevel - itime.c
level - main.c
uflag - main.c
diskfd - main.c
tapefd - tape.c
pipeout - main.c
curino - tape.c
newtape - tape.c
density - already declared in main.c
tapesize - main.c
tsize - main.c
asize - tape.c
etapes - main.c
nonodump - main.c
unlimited - main.c
cachesize - already declared in main.c
rsync_friendly - main.c
notify - already declared in main.c
blockswritten - already declared in main.c
tapeno - already declared in main.c
tstart_writing - main.c
tend_writing - main.c
passno - main.c
sblock - main.c
dev_bsize - already declared in main.c
dev_bshift - main.c
tp_bshift - main.c
dumpdates - leave in itime.c
nddates - leave in itime.c
If you would like me to take over this project, I am willing to do so.
Mar 30 2020
I prefer just using extern and then declaring the actual variables in the function in which they are most prominent, but using EXTERN as you have done does work fine.
While u_spcl is used only in dump/tape.c, it is used in four different files in restore and is not meant to be a separate instance in each one. So, you will have to make it an extern in the header file and then declare it in dump/tape.c and restore/dump.c. I prefer that you do it that way as opposed to the much uglier EXTERN approach.
This looks complete. I had missed the VOP_* manual pages, good catch.
I am happy to have you clean up the duplicate EIOs.
Thanks for doing this.
Mar 24 2020
While read(2) and write(2) are the most likely system calls to return EINTEGRITY, really any system call that accesses a filesystem can potentially return EINTEGRITY. Using the metric that if it can return EIO, then it can now return EINTEGRITY leads to this list of system calls needing EINTEGRITY added: access.2, acct.2, bind.2, chdir.2, chflags.2, chmod.2, chown.2, chroot.2, copy_file_range.2, execve.2, fhlink.2, fhreadlink.2, fsync.2, getdirentries.2, getfh.2, getfsstat.2, intro.2, ktrace.2, link.2, lio_listio.2, mkdir.2, mkfifo.2, mknod.2, mount.2, msync.2, open.2, pathconf.2, posix_fallocate.2, quotactl.2, readlink.2, rename.2, rmdir.2, sendfile.2, stat.2, statfs.2, swapon.2, symlink.2, truncate.2, undelete.2, unlink.2, utimensat.2, and utimes.2.
Mar 20 2020
This is a good example of a place that where EINTEGRITY is intended to be used.
At the moment mount(2) is the only system call that lists EINTEGRITY as a possible error. If your change is going to cause EINTEGRITY to be returned to other system calls (like read(2) or wirite(2)) then you should add it to their list of possible error returns.
Mar 17 2020
With the above suggested changes, I agree with this proposal.
Mar 11 2020
Feb 27 2020
I am happy with this revision.
Feb 18 2020
Jan 28 2020
The logic is easier for me to follow.
Jan 23 2020
Jan 22 2020
Jan 20 2020
I am really happy to see these changes as it will be especially helpful for vnodes that waste many Megabytes of space.
I concur with Jeff that the new code is difficult to follow, though do not have any suggestions on how to simplify it.
Jan 17 2020
After consulting with kib@ I have concluded that it is acceptable to MFC -r343111 to stable/12. You should MFC your change separately (i.e., in a second checkin).
Jan 15 2020
Jan 14 2020
Jan 13 2020
This change seems like an improvement over what we have now.
I have never been a fan of short-circuiting the layers, so am fine with keeping them intact.
Looks good to go.
Jan 11 2020
Jan 10 2020
OK, looks good to go.
I agree with mac@ that it looks like EFRAGS is only in an unused code path. But it does not hurt to map it to a FreeBSD error in the event that the code path gets brought back to life.
Jan 7 2020
I noted I don't know what guaratees are needed for quota files, thus I for safety I regressed the quota-induced scan to go over all vnodes in the mount point. Getting rid of the extra loop is a laudable goal of course, but I don't know how it should look like on a kernel with my patch. If the current places which put the vnode on the dirty list are sufficient that's great, but chances are excellent quota code needs extra calls
My confusion is what "clusters" represent. Apparently a cluster is a 32KB block. Based on your clarification, I now agree with your calculation and printing of free space, but suggest that you clarify that clusters are 32KB blocks (either by replacing the word "clusters" with "32KB blocks", or if clusters is meaningful to administrators change it to "32KB clusters").
Overall I like these changes. I have one specific comment here and also a suggested change to qsync/qsyncvp to mitigate the increased cost of qsync().
What is the reason for adding this macro to set i_flags?
The heavy use of qsync() is in ffs_sync_lazy() and ffs_sync(). These should be using qsyncvp() in their loops rather than separately calling qsync() as show in my attached diff.
This change is independent of your changes, so I should just check it in as a separate change.
Jan 6 2020
The current output is number of free blocks and number of bad blocks. You are changing it to number of free bytes and number of bad bytes.
Since fsck for all other filesystems reports blocks this is a non-standard semantic change, so I recommend that you restore the / 1024 to output block counts.
If you think that it is useful to have byte counts then should make it clear you are reporting bytes and not blocks (e.g., %sB free bytes).
Adding a new error number requires changes to a lot more files than just lib/libc/gen/errlst.c. See for example -r343111 when I added EINTEGRITY which touched 16 different files in the system. And that is just to add the base error. It then has to be added to every system call manual page for which it can be returned.
Dec 25 2019
Dec 24 2019
Dec 22 2019
Dec 13 2019
A couple of minor nits noted inline.
Dec 3 2019
Nov 20 2019
Oct 27 2019
Oct 24 2019
Oct 22 2019
Oct 17 2019
This request is entirely reasonable.
Oct 11 2019
I concur that VFS_STATFS should be done here.
Oct 4 2019
Oct 1 2019
Sep 17 2019
Sep 12 2019
With the minor changes requested, looks good to me.
Sep 9 2019
What scenario would ever have v_writecount be less than zero?
Aug 28 2019
Removing the function is right approach. All looks good.
Aug 27 2019
My inline comment seems to have been lost. I requested that ufs_prepare_reclaim() be made static.
This change looks good to me modulo minor inline comment.
Aug 17 2019
Aug 15 2019
Aug 14 2019
Any progress on getting this done?