[PATCH] [xen-block] Return BLKIF_RSP_EOPNOTSUPP for unknown operation

Mark Syms via posted 1 patch 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250827160841.351707-1-mark.syms@cloud.com
Maintainers: Stefano Stabellini <sstabellini@kernel.org>, Anthony PERARD <anthony@xenproject.org>, Paul Durrant <paul@xen.org>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Stefan Hajnoczi <stefanha@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
hw/block/dataplane/xen-block.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH] [xen-block] Return BLKIF_RSP_EOPNOTSUPP for unknown operation
Posted by Mark Syms via 1 month ago
Returning BLKIF_RSP_ERROR if an operation is not supoprted does not
allow the frontend to exercise any discretion on how to handle the
response and may lead to an operating system crash. As different
backends may support different feature sets and we might, during a
migration, switch backends, an in-flight request might be issued (or
reissued) which is then not supported by this backend.

Signed-off-by: Mark Syms <mark.syms@cloud.com>
---
 hw/block/dataplane/xen-block.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c
index 48c2e315f3..32cf919a97 100644
--- a/hw/block/dataplane/xen-block.c
+++ b/hw/block/dataplane/xen-block.c
@@ -167,7 +167,8 @@ static int xen_block_parse_request(XenBlockRequest *request)
         return 0;
     default:
         error_report("error: unknown operation (%d)", request->req.operation);
-        goto err;
+        request->status = BLKIF_RSP_EOPNOTSUPP;
+        return -1;
     };
 
     if (request->req.operation != BLKIF_OP_READ &&
-- 
2.50.1
Re: [PATCH] [xen-block] Return BLKIF_RSP_EOPNOTSUPP for unknown operation
Posted by Roger Pau Monné 1 month ago
On Wed, Aug 27, 2025 at 05:08:41PM +0100, Mark Syms via wrote:
> Returning BLKIF_RSP_ERROR if an operation is not supoprted does not
> allow the frontend to exercise any discretion on how to handle the
> response and may lead to an operating system crash. As different
> backends may support different feature sets and we might, during a
> migration, switch backends, an in-flight request might be issued (or
> reissued) which is then not supported by this backend.

Linux blkfront at least will empty the ring on resume, and re-process
and queue the requests after having negotiated the (possibly) new set
of features with the backend.  That however doesn't avoid finding
flush requests that might not longer be supported by the new backend,
in which case Linux blkfront will drop such requests.

I assume the fixes done here are not targeted at dealing with a Linux
blkfront?

> Signed-off-by: Mark Syms <mark.syms@cloud.com>
> ---
>  hw/block/dataplane/xen-block.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c
> index 48c2e315f3..32cf919a97 100644
> --- a/hw/block/dataplane/xen-block.c
> +++ b/hw/block/dataplane/xen-block.c
> @@ -167,7 +167,8 @@ static int xen_block_parse_request(XenBlockRequest *request)
>          return 0;
>      default:
>          error_report("error: unknown operation (%d)", request->req.operation);
> -        goto err;
> +        request->status = BLKIF_RSP_EOPNOTSUPP;
> +        return -1;

The comment in blkif.h contains:

 /* Operation not supported (only happens on barrier writes). */
#define BLKIF_RSP_EOPNOTSUPP  -2

So in principle BLKIF_RSP_EOPNOTSUPP is only to be used as a response
for BLKIF_OP_WRITE_BARRIER or BLKIF_OP_FLUSH_DISKCACHE requests,
however blkback already uses it as a response to unknown request
types (like you propose here).

Would you mind also sending a patch to adjust blkif.h in Xen to remove
the "(only happens on barrier writes)" part of the comment?

Thanks Roger.
Re: [PATCH] [xen-block] Return BLKIF_RSP_EOPNOTSUPP for unknown operation
Posted by Mark Syms 1 month ago
> The comment in blkif.h contains:
>
>  /* Operation not supported (only happens on barrier writes). */
> #define BLKIF_RSP_EOPNOTSUPP  -2
>
> So in principle BLKIF_RSP_EOPNOTSUPP is only to be used as a response
> for BLKIF_OP_WRITE_BARRIER or BLKIF_OP_FLUSH_DISKCACHE requests,
> however blkback already uses it as a response to unknown request
> types (like you propose here).
>
> Would you mind also sending a patch to adjust blkif.h in Xen to remove
> the "(only happens on barrier writes)" part of the comment?

Sure, no problem