On Mon, Jun 05 2023 at 5:14P -0400, Sarthak Kukreti <sarthakkukreti@xxxxxxxxxxxx> wrote: > On Sat, Jun 3, 2023 at 8:57 AM Mike Snitzer <snitzer@xxxxxxxxxx> wrote: > > > > 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