Re: [PATCH] block: add blockdev-attach QMP command

[Stefan Hajnoczi <stefanha@redhat.com>]

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

On Wed, Apr 15, 2026 at 07:39:05PM +0200, mr-083 wrote:
> Add a blockdev-attach QMP command that attaches a block driver state
> tree to a device's block backend.  Unlike blockdev-insert-medium, this
> works for non-removable devices such as NVMe namespaces.
> 
> After drive_del removes a device's backing store, the BlockBackend
> remains attached to the guest device but has no BlockDriverState.
> blockdev-attach reconnects a block node (previously created with
> blockdev-add) to the device's BlockBackend via blk_insert_bs().
> 
> This separates the two concerns as recommended: blockdev-add creates
> the block node, blockdev-attach associates it with the device.
> 
> Example usage with NVMe namespace hot-swap:
>   drive_del drv0
>   blockdev-add node-name=node0 driver=qcow2 file.driver=file \
>                file.filename=disk.qcow2
>   blockdev-attach id=ns0 node-name=node0

Hi Matthieu,
I came across a quirk here:

  $ qemu-system-x86_64 \
      --blockdev file,filename=my-lun.img,node-name=drive1 \
      --device nvme,serial=nvme0 \
      --device nvme-ns,id=nvme-ns0,drive=drive1
  (qemu) drive_del drive1
  Error: drive drive1 is in use

But when I change --blockdev to --drive, it succeeds. I think this
second scenario is the one that you have been testing.

I also tried the QMP blockdev-del command together with --blockdev, and
it fails:

  $ qemu-system-x86_64 ... \
      -qmp unix:/tmp/qmp.sock,server=on,wait=off
  $ qmp-shell /tmp/qmp.sock
  (QEMU) blockdev-del node-name=drive1
  {"error": {"class": "GenericError", "desc": "Node drive1 is in use"}}

It seems logical that a disk image that's in use by NVMe emulation
cannot be removed. So now I'm wondering if there is a bug or a
historical reason why drive_del allows drives to the removed at runtime
even when media change is not supported.

CCing Kevin Wolf and Markus Amrbruster as they may know the answer.

Depending on the answer this may influence the test workflow in this
patch. It's unclear to me what is being tested here since this does not
simulate a real error that a physical NVMe drive could raise?

I think the intention is to fail I/O so a test can simulate a period
where the drive is down. QEMU has the blkdebug driver which could be
inserted into the block graph to fail I/O requests - maybe that is
appropriate for your test and may not require new monitor commands (see
blkdebug examples in tests/)?

Stefan

> 
> An HMP wrapper is included for convenience.
> 
> Signed-off-by: Matthieu <matthieu@min.io>
> ---
>  block/monitor/block-hmp-cmds.c | 10 ++++++++++
>  block/qapi-system.c            | 36 ++++++++++++++++++++++++++++++++++
>  hmp-commands.hx                | 16 +++++++++++++++
>  include/block/block-hmp-cmds.h |  1 +
>  qapi/block.json                | 33 +++++++++++++++++++++++++++++++
>  5 files changed, 96 insertions(+)
> 
> diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
> index 1fd28d59eb..8a3d821e01 100644
> --- a/block/monitor/block-hmp-cmds.c
> +++ b/block/monitor/block-hmp-cmds.c
> @@ -195,6 +195,16 @@ unlock:
>      hmp_handle_error(mon, err);
>  }
>  
> +void hmp_blockdev_attach(Monitor *mon, const QDict *qdict)
> +{
> +    const char *id = qdict_get_str(qdict, "id");
> +    const char *node_name = qdict_get_str(qdict, "node-name");
> +    Error *err = NULL;
> +
> +    qmp_blockdev_attach(id, node_name, &err);
> +    hmp_handle_error(mon, err);
> +}
> +
>  void hmp_commit(Monitor *mon, const QDict *qdict)
>  {
>      const char *device = qdict_get_str(qdict, "device");
> diff --git a/block/qapi-system.c b/block/qapi-system.c
> index 54b7409b2b..ec89645bc1 100644
> --- a/block/qapi-system.c
> +++ b/block/qapi-system.c
> @@ -304,6 +304,42 @@ void qmp_blockdev_insert_medium(const char *id, const char *node_name,
>      blockdev_insert_medium(NULL, id, node_name, errp);
>  }
>  
> +void qmp_blockdev_attach(const char *id, const char *node_name,
> +                         Error **errp)
> +{
> +    BlockBackend *blk;
> +    BlockDriverState *bs;
> +    int ret;
> +
> +    GRAPH_RDLOCK_GUARD_MAINLOOP();
> +
> +    blk = qmp_get_blk(NULL, id, errp);
> +    if (!blk) {
> +        return;
> +    }
> +
> +    if (blk_bs(blk)) {
> +        error_setg(errp, "Device already has a medium inserted");
> +        return;
> +    }
> +
> +    bs = bdrv_find_node(node_name);
> +    if (!bs) {
> +        error_setg(errp, "Node '%s' not found", node_name);
> +        return;
> +    }
> +
> +    if (bdrv_has_blk(bs)) {
> +        error_setg(errp, "Node '%s' is already in use", node_name);
> +        return;
> +    }
> +
> +    ret = blk_insert_bs(blk, bs, errp);
> +    if (ret < 0) {
> +        return;
> +    }
> +}
> +
>  void qmp_blockdev_change_medium(const char *device,
>                                  const char *id,
>                                  const char *filename,
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 5cc4788f12..ce32ed33ab 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -207,6 +207,22 @@ SRST
>    actions (drive options rerror, werror).
>  ERST
>  
> +    {
> +        .name       = "blockdev-attach",
> +        .args_type  = "id:s,node-name:s",
> +        .params     = "id node-name",
> +        .help       = "attach a block node to a device (non-removable)",
> +        .cmd        = hmp_blockdev_attach,
> +    },
> +
> +SRST
> +``blockdev-attach`` *id* *node-name*
> +  Attach a block driver state tree (created with ``blockdev-add``) to a
> +  device's block backend. Unlike ``blockdev-insert-medium``, this works
> +  for non-removable devices such as NVMe namespaces. The device must
> +  have no medium inserted (e.g. after ``drive_del``).
> +ERST
> +
>      {
>          .name       = "change",
>          .args_type  = "device:B,force:-f,target:F,arg:s?,read-only-mode:s?",
> diff --git a/include/block/block-hmp-cmds.h b/include/block/block-hmp-cmds.h
> index 71113cd7ef..34d30915fc 100644
> --- a/include/block/block-hmp-cmds.h
> +++ b/include/block/block-hmp-cmds.h
> @@ -21,6 +21,7 @@ void hmp_drive_add(Monitor *mon, const QDict *qdict);
>  
>  void hmp_commit(Monitor *mon, const QDict *qdict);
>  void hmp_drive_del(Monitor *mon, const QDict *qdict);
> +void hmp_blockdev_attach(Monitor *mon, const QDict *qdict);
>  
>  void hmp_drive_mirror(Monitor *mon, const QDict *qdict);
>  void hmp_drive_backup(Monitor *mon, const QDict *qdict);
> diff --git a/qapi/block.json b/qapi/block.json
> index 46955bbb3e..c05d3b5ac1 100644
> --- a/qapi/block.json
> +++ b/qapi/block.json
> @@ -295,6 +295,39 @@
>    'data': { 'id': 'str',
>              'node-name': 'str'} }
>  
> +##
> +# @blockdev-attach:
> +#
> +# Attach a block driver state tree to a device's block backend.
> +# Unlike blockdev-insert-medium, this works for non-removable
> +# devices such as NVMe namespaces.  The device must currently have
> +# no medium inserted (e.g. after drive_del removed the backing).
> +#
> +# @id: The name or QOM path of the guest device
> +#
> +# @node-name: name of a node in the block driver state graph
> +#
> +# Since: 11.1
> +#
> +# .. qmp-example::
> +#
> +#     -> { "execute": "blockdev-add",
> +#          "arguments": {
> +#              "node-name": "node0",
> +#              "driver": "qcow2",
> +#              "file": { "driver": "file",
> +#                        "filename": "disk.qcow2" } } }
> +#     <- { "return": {} }
> +#
> +#     -> { "execute": "blockdev-attach",
> +#          "arguments": { "id": "ns0",
> +#                         "node-name": "node0" } }
> +#     <- { "return": {} }
> +##
> +{ 'command': 'blockdev-attach',
> +  'data': { 'id': 'str',
> +            'node-name': 'str'} }
> +
>  ##
>  # @BlockdevChangeReadOnlyMode:
>  #
> -- 
> 2.53.0
> 

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