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

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

[Mark Syms via]

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

[Roger Pau Monné]

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

[Mark Syms]

> 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