On Mon, Jun 05 2023 at 5:14P -0400,
Sarthak Kukreti [off-list ref] wrote:
On Sat, Jun 3, 2023 at 8:57 AM Mike Snitzer [off-list ref] wrote:
quoted
We all just need to focus on your proposal and Joe's dm-thin
reservation design...
[Sarthak: FYI, this implies that it doesn't really make sense to add
dm-thinp support before Joe's design is implemented. Otherwise we'll
have 2 different responses to REQ_OP_PROVISION. The one that is
captured in your patchset isn't adequate to properly handle ensuring
upper layer (like XFS) can depend on the space being available across
snapshot boundaries.]
Ack. Would it be premature for the rest of the series to go through
(REQ_OP_PROVISION + support for loop and non-dm-thinp device-mapper
targets)? I'd like to start using this as a reference to suggest
additions to the virtio-spec for virtio-blk support and start looking
at what an ext4 implementation would look like.
Please drop the dm-thin.c and dm-snap.c changes. dm-snap.c would need
more work to provide the type of guarantee XFS requires across
snapshot boundaries. I'm inclined to _not_ add dm-snap.c support
because it is best to just use dm-thin.
And FYI even your dm-thin patch will be the starting point for the
dm-thin support (we'll keep attribution to you for all the code in a
separate patch).
Fair points, I certainly don't want to derail this conversation; I'd
be happy to see this work merged sooner rather than later.
Once those dm target changes are dropped I think the rest of the
series is fine to go upstream now. Feel free to post a v8.
For posterity, I'll distill what I said above into the following: I'd like
a capability for userspace to create thin snapshots that ignore the
thin volume's provisioned areas. IOW, an opt-in flag which makes
snapshots fallback to what they do today to provide flexibility to
userspace to decide the space requirements for the above mentioned
scenarios, and at the same time, not adding separate corner case
handling for filesystems. But to reiterate, my intent isn't to pile
this onto the work you, Mike and Joe have planned; just some insight
into why I'm in favor of ideas that reduce the snapshot size.
I think it'd be useful to ignore a thin device's reservation for
read-only snapshots. Adding the ability to create read-only thin
snapshots could make sense (later activations don't necessarily need
to impose read-only, doing so would require some additional work).
Mike