We rely on md(4) in our build process heavily. However, if the build goes haywire the allocated resources (i.e. swap and memory-backed md(4)'s) need to be purged. It is extremely useful to have ability to attach arbitrary labels to each of the virtual disks so that they can be identified and GC'ed if neecessary.
Details
Details
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Love this idea, but the padding issue needs to be resolved.
| sys/sys/mdioctl.h | ||
|---|---|---|
| 52 ↗ | (On Diff #27626) | This changes the ABI on 64-bit architectures, since an int is only 32-bits there, but a pointer is 64 so the sizeof md_ioctl grows by 4 bytes. |
Comment Actions
Hmm, are you sure? I think I've actually checked it goes down by 8 bytes. There is alignment, the structure is not packed, so that compiler I think aligns everything to the natural word size on 64-bits. At least on intel it's what's happening.
-Max
Comment Actions
I've checked both forward and backward compatibility (3-way actually, old mdconfig+new kernel, old kernel + new mdconfig, new mdconfig+new kernel) everything seems to be working properly.
| sys/sys/mdioctl.h | ||
|---|---|---|
| 52 ↗ | (On Diff #27626) | I think there is alignment in play, the struct is not packed. |
| sbin/mdconfig/mdconfig.8 | ||
|---|---|---|
| 197 ↗ | (On Diff #27626) | I believe the rule for manpages is that sentences start on a new line. Also, "can be then" should be "can then be". Finally, the second sentence is a fragment. I suggest this instead: .It Fl L Ar label Associate a label (arbitrary string) with the new memory disk. The label can then be inspected with the .Nm .Fl l v command. |
| sys/dev/md/md.c | ||
| 1703 ↗ | (On Diff #27626) | I think you want copystr(). |
Comment Actions
Igor suggestion for the man page change.
| sbin/mdconfig/mdconfig.8 | ||
|---|---|---|
| 197 ↗ | (On Diff #27626) | That's right. Use textproc/igor to check the man page for common errors. |
Comment Actions
On all supported architectures?
Don't forget to bump .Dd in the man page when committing.
| sbin/mdconfig/mdconfig.8 | ||
|---|---|---|
| 197 ↗ | (On Diff #27626) | I would prefer "can then be instpected with |
| sbin/mdconfig/mdconfig.8 | ||
|---|---|---|
| 197 ↗ | (On Diff #27626) | Yeah, that's probably better way to phrase it. |
| sys/dev/md/md.c | ||
|---|---|---|
| 1703 ↗ | (On Diff #27626) | Are you sure? Reading copystr(8): DESCRIPTION
The copy functions are designed to copy contiguous data from one address
to another. All but copystr() copy data from user-space to kernel-space
or vice-versa.
...
The copystr() function copies a NUL-terminated string, at most len bytes
long, from kernel-space address kfaddr to kernel-space address kdaddr.
The number of bytes actually copied, including the terminating NUL, is
returned in *done (if done is non-NULL). |
| sys/sys/mdioctl.h | ||
|---|---|---|
| 52 ↗ | (On Diff #27626) | I've verified again, the sizeof(struct md_ioctl) remains 448 bytes before and after change on x64. I don't have access to any other supported 64-bit arches to verify. |
| sys/dev/md/md.c | ||
|---|---|---|
| 1703 ↗ | (On Diff #27626) | You're right. I need new glasses. :-p |
| sbin/mdconfig/mdconfig.8 | ||
|---|---|---|
| 197 ↗ | (On Diff #27626) | I feel like this example command should be separated from the rest of the sentence. Maybe: The label can then be inspected with .Bd -literal -offset indent .Nm .Fl l v .Ed (Might need a .Pp after that.) |
| sbin/mdconfig/mdconfig.8 | ||
|---|---|---|
| 197 ↗ | (On Diff #27626) | Done in the latest revision. I have had to merge Nm + Fl... into a single line, though otherwise it would wrap it up separating -l -v to the next line. |
Comment Actions
Man page changes LGTM.
| sbin/mdconfig/mdconfig.8 | ||
|---|---|---|
| 40 ↗ | (On Diff #28650) | Remember to bump this on commit. |