Add support to vioblk for DISCARD operation

Review Request #2534 - Created April 10, 2020 and updated

Information
Jason King
illumos-gate
12506
Reviewers
general

Adds support for vioblk DISCARD operation

zfs TRIM tests, manually issuing various DKIOCFREE calls while observing the resulting guest and host calls

Issues

  • 19
  • 22
  • 0
  • 41
Description From Last Updated
This suggests that the interface is really unsafe for an application to use as written, if they care about reliable ... Robert Mustacchi Robert Mustacchi
Why is this opaque? The client is going to need to know the size and layout of the structure so ... Robert Mustacchi Robert Mustacchi
I think it's worth spending some time documenting this in more detail. For example, it took me a while to ... Robert Mustacchi Robert Mustacchi
So, right now the only member of the bd_xfer_t that a client driver uses is the additional segment list when ... Robert Mustacchi Robert Mustacchi
Does the system already have a constraint on power of 2 block sizes? Robert Mustacchi Robert Mustacchi
I think the flow of control is clearer when the thing that does the allocation is the same place that ... Robert Mustacchi Robert Mustacchi
I wonder if we want a different way of a driver being able to indicate the support for trim / ... Robert Mustacchi Robert Mustacchi
Is there any particular reason you chose ENXIO here? Robert Mustacchi Robert Mustacchi
Are we guaranteed that the longlong_t in the form of the diskaddr_t values that we're getting won't be negative? Robert Mustacchi Robert Mustacchi
I assume that this is being verified / checked as something that won't overflow by callers? Robert Mustacchi Robert Mustacchi
Should we sanity check these at all? If the guest gave us a bad value, that would cause us to ... Robert Mustacchi Robert Mustacchi
These kinds of comments make me feel like we should fix this. Certainly it's an easier driver design when we ... Robert Mustacchi Robert Mustacchi
So, reading this it doesn't immediately clarify things for me to the point that this is clear, fwiw. It's not ... Robert Mustacchi Robert Mustacchi
Should we assume that dfi_bshift is well formed and correct and that this isn't a value that'll cause this to ... Robert Mustacchi Robert Mustacchi
You store bsize as a uint_t above in dfl_iter(), but as a uint64_t here. Is there a reason for the ... Robert Mustacchi Robert Mustacchi
It does seem that ENXIO is more documented for this case, though it is overloaded. Robert Mustacchi Robert Mustacchi
I've been looking at this and there's something that's not settling for me. If we've been given a starting and ... Robert Mustacchi Robert Mustacchi
size_t / uint64_t comparison. You probably want to make idx a uint64_t. Should this instead be a check that we ... Robert Mustacchi Robert Mustacchi
Is there value in making this a const pointer? Robert Mustacchi Robert Mustacchi
Yuri Pankov
Robert Mustacchi
Jason King
Robert Mustacchi
Loading...