Re: [RFC v2 2/4] virtio-blk: Add VIRTIO_BLK_T_OUT_FUA command support

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1435 bytes --]

On Thu, May 08, 2025 at 05:20:42PM +0100, Alberto Faria wrote:
> @@ -828,13 +832,16 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
>  
>      type = virtio_ldl_p(vdev, &req->out.type);
>  
> -    /* VIRTIO_BLK_T_OUT defines the command direction. VIRTIO_BLK_T_BARRIER
> -     * is an optional flag. Although a guest should not send this flag if
> -     * not negotiated we ignored it in the past. So keep ignoring it. */
> -    switch (type & ~(VIRTIO_BLK_T_OUT | VIRTIO_BLK_T_BARRIER)) {
> +    /* VIRTIO_BLK_T_BARRIER is an optional flag. Although a guest should not
> +     * send this flag if not negotiated we ignored it in the past. So keep
> +     * ignoring it. */
> +    switch (type & ~VIRTIO_BLK_T_BARRIER) {

This changes the behavior of the device. VIRTIO_BLK_T_FLUSH |
VIRTIO_BLK_T_OUT is now treated as an unsupported command instead of a
flush. The same is true for the other command types as well (like zoned
devices, discard, etc).

Buggy guest drivers might depend on this behavior. From a user
perspective it's QEMU's fault if existing guests break, even if the
guest driver violates the VIRTIO specification. I would treat this as a
stable ABI that third-party virtio-blk drivers depend on unless there is
a strong reason to change existing behavior.

Can you add "case VIRTIO_BLK_T_OUT_FUA & ~VIRTIO_BLK_T_OUT:" instead of
changing existing behavior?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]