On Sun, Sep 18, 2022 at 05:01:53PM +0300, Alvaro Karsz wrote: > Thanks for the reply. > > > why minimum? > > > why is that? > > This was discussed in the previous version > (https://www.spinics.net/lists/linux-virtualization/msg58232.html). > As far as I know, the Linux kernel uses the same "max segments" value > for a discard and a secure erase command. > In the first version, I ignored the max_secure_erase_seg and > secure_erase_sector_alignment config fields (just like > max_write_zeroes_seg and write_zeroes_may_unmap are ignored in the > write zeros command implementation). > > It was suggested to use the minimum "max segments" value if both > VIRTIO_BLK_F_SECURE_ERASE and VIRTIO_BLK_F_DISCARD are negotiated. > The same is true for the sector alignment values. sounds good. Add a code comment? > > is this logic repeating code from below? > > I'm not sure what you mean. > The idea is: > At this point, the VIRTIO_BLK_F_DISCARD fields were read from the > virtio config (if VIRTIO_BLK_F_DISCARD is negotiated). > If max_discard_segs is 0, VIRTIO_BLK_F_DISCARD is not negotiated (or > set to 0), so we should use the max_secure_erase_seg value as > max_discard_segs. yes but I now see two places that seem to include this logic. > > > Always? What's going on here? > > which versions handled max_secure_erase_seg == 0? > > This comment is from the VIRTIO_BLK_F_DISCARD implementation. > I added the max_secure_erase_seg part since I could not find how to > handle the case when max_secure_erase_seg is 0 in the spec. > So, like with the VIRTIO_BLK_F_DISCARD implementation, I'm setting the > value to sg_elems. I am not 100% sure. Two options: 1- Add a validate callback and clear VIRTIO_BLK_F_SECURE_ERASE. 2- Alternatively, fail probe. which is preferable depends on how bad is it if host sets VIRTIO_BLK_F_SECURE_ERASE but guest does not use it. -- MST _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
