Page MenuHomeFreeBSD

Allow to specify a fs type for a vnode-backed memory disk
ClosedPublic

Authored by ak on Dec 5 2015, 10:58 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Mar 12, 2:58 AM
Unknown Object (File)
Tue, Mar 12, 2:58 AM
Unknown Object (File)
Tue, Mar 12, 2:58 AM
Unknown Object (File)
Tue, Mar 12, 2:53 AM
Unknown Object (File)
Tue, Mar 12, 2:53 AM
Unknown Object (File)
Fri, Mar 8, 4:39 AM
Unknown Object (File)
Jan 6 2024, 4:10 PM
Unknown Object (File)
Jan 4 2024, 2:03 PM
Subscribers

Details

Reviewers
mav
Group Reviewers
manpages
Summary

Allow to specify a file system type for a vnode-backed memory disk.

This will simplify mounting images from command line:

#mdmfs -T cd9660 -P -F foo.iso md /mnt/cdrom
instead of
#mdconfig -a -t vnode -f foo.iso -u 10000
#mount_cd9660 /dev/md10000 /mnt/cdrom

And allows to mount images from /etc/fstab:
echo 'md /media mfs rw,-Tmsdosfs,-P,-F/path/image.img 0 0' >> /etc/fstab

Test Plan

Apply the patch, rebuild mdmfs and mount/unmount some images.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

ak retitled this revision from to Allow to specify a fs type for a vnode-backed memory disk.
ak updated this object.
ak edited the test plan for this revision. (Show Details)
ak set the repository for this revision to rS FreeBSD src repository - subversion.
wblock added inline comments.
sbin/mdmfs/mdmfs.8
234 ↗(On Diff #10775)

Is there a line missing here? (I know, it's not part of your patch.)

243 ↗(On Diff #10775)

Why not UFS?

244 ↗(On Diff #10775)
.Fl T
only applies when
.Fl F
and
.Fl P
are used.
sbin/mdmfs/mdmfs.8
234 ↗(On Diff #10775)

Actually, I'd implemented that option too. I think it should had been "on created filesystem which allows return of". This sentence is a little awkward, so I will be grateful for any suggestions.

243 ↗(On Diff #10775)

Sure, you can specify -T ufs, but ufs is default, you don't need -T to select it.

244 ↗(On Diff #10775)

Noted.

sbin/mdmfs/mdmfs.8
234 ↗(On Diff #10775)

Line 231 has a "The", which I think can just be deleted. Then for the rest:

.Xr md 4
supports the BIO_DELETE command, also known as TRIM.
When used with a filesystem that supports TRIM,
.Xr md 4
returns freed blocks to the system memory pool.
sbin/mdmfs/mdmfs.8
234 ↗(On Diff #10775)

This is slightly technically incorrect: BIO_DELETE is a geom command, which indicates that a certain range of data is no longer used and can be erased, trim is an implementation specific for ata, for scsi, for example, it is called unmap, so they are not equivalent in general. Also trim on fs needs to be explicitly enabled, while md(4) supports it passively, so IMHO previous sentence was a little more clear.

sbin/mdmfs/mdmfs.8
234 ↗(On Diff #10775)

I'd remove the 'aka TRIM bit, and then say

When used with a filesystem that issue BIO_DELETE bio requests,
.Xr md 4
returns deleted blocks to the system memory pool.

or

.Xr md 4 returns blocks to the system memory pool in response
to BIO_DELETE commands.

ak edited edge metadata.
  • Bump manpage's date
  • Rephrase -t option description
  • Clarify -T option usage

One note, but the man page looks good.

head/sbin/mdmfs/mdmfs.8
311

Please just split the sentence here:

with the same letter.
The
.Fl T
ak edited edge metadata.
  • Split long sentences.

I generally have no objections against adding another flag. It looks useful. But I have couple related comments.

head/sbin/mdmfs/mdmfs.c
514

Why only those file systems, and why this tool should at all bother what file system types are supported by OS/mount tool?

sbin/mdmfs/mdmfs.8
243 ↗(On Diff #10775)

Default may change at some point, who knows, and it is good to be able to explicitly specify things even if they are default. Inability to do it create more problems for other code that has also know about this UFS default, and that this options must not be specified in case of UFS.

ak edited edge metadata.
  • Don't check for valid fstype, rely on mount(8) for this
  • Update -T option description
mav added a reviewer: mav.

Looks good to me.

This revision is now accepted and ready to land.Mar 9 2016, 6:37 PM