Page MenuHomeFreeBSD

geom_part: auto commit after GEOM_PART has been automatically resized.
Needs ReviewPublic

Authored by rew on Jan 12 2022, 8:44 AM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 31 2022, 4:03 AM
Unknown Object (File)
Dec 24 2022, 1:32 AM
Unknown Object (File)
Dec 14 2022, 6:13 PM
Unknown Object (File)
Dec 14 2022, 2:04 AM
Subscribers

Details

Summary

This is useful so that user intervention is not required to commit
changes to disk after a resize has occurred.

For example, when running a bhyve VM, the guest does not have to
manually commit the changes to disk after the virtio-blk device was
resized from the host.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 43879
Build 40767: arc lint + arc unit

Event Timeline

rew requested review of this revision.Jan 12 2022, 8:44 AM

Any feedback here?

The idea of resize changes being committed automatically was also brought up here: https://reviews.freebsd.org/D9603

This patch drops the requirement for a bhyve guest vm to manually do a 'gpart commit' after a guest block device has been resized from the host.

I both love and hate this idea.
For drives that change size based on an external command , it's absolutely the right thing, assuming that the resize is more or less permanent.
For others, I'm less sure we want to automatically, though I'm struggling to provide a concrete counter example.

For many GEOM classes with metadata in last sector automatic resize handling is almost required to not get lost on reboot. I don't know about some of partition tables supported by gpart, but at least GPT and MBR should be OK without it, that is why I suppose it was made flexible. Plus table resize without partition resize won't buy much except restoring redundancy for GPT. So I also feel split about this change. Plus I guess g_part_commit() may commit some other pending changes, that may upset somebody.

I think you can go through using of some sysctl/tunable first.
I.e. by default system will use old behavior, but you can change it for your system.

In D33863#768971, @ae wrote:

I think you can go through using of some sysctl/tunable first.
I.e. by default system will use old behavior, but you can change it for your system.

I'd agree. This would let us get more experience with this.

In D33863#768994, @imp wrote:
In D33863#768971, @ae wrote:

I think you can go through using of some sysctl/tunable first.
I.e. by default system will use old behavior, but you can change it for your system.

I'd agree. This would let us get more experience with this.

To me sysctl here would feel more like an excuse, not something practically useful. Issues in this area I'd expect pretty rare (generally disk resizes is a rare thing), and by the time somebody hit a need to tune it, it will likely be too late already.

At very least I suppose we should do something about the pending changes commit. I've never used the pending changes myself, but if some people really do, they'd be unhappy from auto-commit triggered by this. BTW, does the patch really means g_part_ctl_commit(), or there are some other patches in flight, because I can't find g_part_commit() anywhere in head now.

In D33863#768995, @mav wrote:

To me sysctl here would feel more like an excuse, not something practically useful.

yea, I'm not a fan of adding another sysctl here. I'm from the frame of mind where, if you want auto resize, then you get auto commit as well. But that's not what kern.geom.part.auto_resize means.

I've never used the pending changes myself, but if some people really do, they'd be unhappy from auto-commit triggered by this.

I'm curious how many people use pending changes in this context, considering the default behavior of gpart is to autocommit. Wonder if its worth sending an email out to get feedback from people that might be using this?

BTW, does the patch really means g_part_ctl_commit(), or there are some other patches in flight, because I can't find g_part_commit() anywhere in head now.

Here's the patch that brings g_part_commit() in: https://reviews.freebsd.org/D33862