Add VIRTIO_BLK_T_DISCARD support to the virtio-blk driver

[allanjude]

Summary

If the hypervisor advertises support for the DISCARD command then the
guest can perform TRIM commands, freeing space on the backing store.

If VIRTIO_BLK_F_DISCARD is enabled, advertise DISKFLAG_CANDELETE

[bryanv]

sys/dev/virtio/block/virtio_blk.c

737

Spec says this is in 512-byte units.

...max_discard_sectors discard_sector_alignment are expressed in 512-byte units if the VIRTIO_BLK_F_DISCARD feature bit...

738

Is there a corresponding disk field for discard_sector_alignment?

954

This will go out of scope

sys/dev/virtio/block/virtio_blk.h

38–39

I'd prefer to you keep the existing style (and it is more or less consistent with the other VirtIO drivers defs)

[allanjude]

Update to fix some issues and catch up with HEAD

[cem]

sys/dev/virtio/block/virtio_blk.c

557–560

You should also check for VIRTIO_BLK_F_DISCARD before submitting the DELETE bio to the queue.

696

This refactoring (512 -> VTBLK_BSIZE) can go in as a separate non-functional change.

889–894

Should these still be in units of 512 if VIRTIO_BLK_F_BLK_SIZE? (Orthogonal to the TRIM change, I know.)

959–960

We've already done this math once in req->vbr_hdr.type, etc, so it's a bit odd to do it again. I'd use the req values here, since that seems to be the primary interface.

1127–1128

I'd suggest assigning NULL to the pointer after free as well.

sys/dev/virtio/block/virtio_blk.h

47–48

These comments seem backwards? Also are the bits really both 0x200, or should one be 0x100?

76–83

style nit: Capitalize the first letter of each sentence.

[allanjude]

Address most of cem's feedback

sys/dev/virtio/block/virtio_blk.c

889–894

I think so, everything in the virtio protocol is specifically defined in 512 bytes

959–960

The req object only knows the sector number, it does not know the length (num_sectors)

sys/dev/virtio/block/virtio_blk.h

47–48

I have improved the comment. They are both 0x200, the flag was renamed from WCE to FLUSH in virtio 1.0

[allanjude]

Fix typo

[jhb]

Please do test against a non-bhyve hypervisor (KVM or QEMU)

[allanjude]

I've created some images for people to test in other hypervisors. A build of -current from this morning, with this patch applied:

http://trooper.hml3.scaleengine.net/freebsd_virtio_trim_r362891.vmdk

http://trooper.hml3.scaleengine.net/freebsd_virtio_trim_r362891.raw

[freqlabs]

I haven't reviewed the code, but I did a basic test in bhyve and it put zeros on the disk in an instant :)