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

[Stefan Hajnoczi <stefanha@redhat.com>]

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

On Wed, May 06, 2026 at 08:10:15AM +0200, Markus Armbruster wrote:
> Stefan Hajnoczi <stefanha@redhat.com> writes:
> 
> > 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
> 
> I see "Error: Node drive1 is in use".
> 
> It's from qmp_blockdev_del():
> 
>     bs = bdrv_find_node(node_name);
>     if (!bs) {
>         error_setg(errp, "Failed to find node with node-name='%s'", node_name);
>         return;
>     }
>     if (bdrv_has_blk(bs)) {
>         error_setg(errp, "Node %s is in use", node_name);
>         return;
>     }
> 
> Called from hmp_drive_del():
> 
>     bs = bdrv_find_node(id);
>     if (bs) {
>         qmp_blockdev_del(id, &err);
>         goto unlock;
>     }
> 
> > But when I change --blockdev to --drive, it succeeds. I think this
> > second scenario is the one that you have been testing.
> 
> I *guess* you changed it to something like
> 
>     -drive if=none,file=test.img,id=drive1

Yes.

> 
> hmp_drive_del()'s call of bdrv_find_node() then fails, and we take a
> completely different path, which succeeds:
> 
>     (qemu) info block
>     drive1 (#block195): test.img (raw)
>         Attached to:      nvme-ns0
>         Cache mode:       writeback
>     (qemu) drive_del drive1
>     (qemu) info block
>     nvme-ns0: [not inserted]
>         Attached to:      nvme-ns0
> 
> This rips nvme-ns0's drive right off.  Feels *unsafe*: the frontend
> (here: nvme-ns) may reasonably assume that a non-removable medium stays
> put, use it without checking it's still there, and crash.

I/O requests fail with -ENOMEDIUM when blk->root.bs is NULL but it's
still a bit concerning that the medium can be removed without the device
emulation being aware.

> 
> > 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.
> 
> I suspect this is simply a bug, and likely a regression.  Can you find
> out?

Okay. I thought maybe there is some backwards compatibility reason that
requires -drive if=none to behave like this.

It's a long-standing bug:

v3.0.0  Affected
v4.0.0  Affected
v6.0.0  Affected
v8.0.0  Affected
master  Affected

I didn't go back beyond v3.0.0 because Python 2 is required to build
really old versions of QEMU.

Stefan

> 
> > 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>
> 

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