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?
signature.asc
Description: PGP signature