Message ID | 20240306131055.2460378-2-armbru@redhat.com |
---|---|
State | New |
Headers | show |
Series | blockdev: Fix block_resize error reporting for op blockers | expand |
On 6/3/24 14:10, Markus Armbruster wrote: > When block_resize() runs into an op blocker, it creates an error like > this: > > error_setg(errp, "Device '%s' is in use", device); > > Trouble is @device can be null. My system formats null as "(null)", > but other systems might crash. Reproducer: > > 1. Create two block devices > > -> {"execute": "blockdev-add", "arguments": {"driver": "file", "node-name": "blk0", "filename": "64k.img"}} > <- {"return": {}} > -> {"execute": "blockdev-add", "arguments": {"driver": "file", "node-name": "blk1", "filename": "m.img"}} > <- {"return": {}} > > 2. Put a blocker on one them > > -> {"execute": "blockdev-mirror", "arguments": {"job-id": "job0", "device": "blk0", "target": "blk1", "sync": "full"}} > {"return": {}} > -> {"execute": "job-pause", "arguments": {"id": "job0"}} > {"return": {}} > -> {"execute": "job-complete", "arguments": {"id": "job0"}} > {"return": {}} > > Note: job events elided for brevity. > > 3. Attempt to resize > > -> {"execute": "block_resize", "arguments": {"node-name": "blk1", "size":32768}} > <- {"error": {"class": "GenericError", "desc": "Device '(null)' is in use"}} > > Broken when commit 3b1dbd11a60 made @device optional. Fixed in commit > ed3d2ec98a3 (block: Add errp to b{lk,drv}_truncate()), except for this > one instance. > > Fix it by using the error message provided by the op blocker instead, > so it fails like this: > > <- {"error": {"class": "GenericError", "desc": "Node 'blk1' is busy: block device is in use by block job: mirror"}} > > Fixes: 3b1dbd11a60d (qmp: Allow block_resize to manipulate bs graph nodes.) > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > blockdev.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
diff --git a/blockdev.c b/blockdev.c index f8bb0932f8..d8fb3399f5 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2252,8 +2252,7 @@ void coroutine_fn qmp_block_resize(const char *device, const char *node_name, } bdrv_graph_co_rdlock(); - if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_RESIZE, NULL)) { - error_setg(errp, QERR_DEVICE_IN_USE, device); + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_RESIZE, errp)) { bdrv_graph_co_rdunlock(); return; }
When block_resize() runs into an op blocker, it creates an error like this: error_setg(errp, "Device '%s' is in use", device); Trouble is @device can be null. My system formats null as "(null)", but other systems might crash. Reproducer: 1. Create two block devices -> {"execute": "blockdev-add", "arguments": {"driver": "file", "node-name": "blk0", "filename": "64k.img"}} <- {"return": {}} -> {"execute": "blockdev-add", "arguments": {"driver": "file", "node-name": "blk1", "filename": "m.img"}} <- {"return": {}} 2. Put a blocker on one them -> {"execute": "blockdev-mirror", "arguments": {"job-id": "job0", "device": "blk0", "target": "blk1", "sync": "full"}} {"return": {}} -> {"execute": "job-pause", "arguments": {"id": "job0"}} {"return": {}} -> {"execute": "job-complete", "arguments": {"id": "job0"}} {"return": {}} Note: job events elided for brevity. 3. Attempt to resize -> {"execute": "block_resize", "arguments": {"node-name": "blk1", "size":32768}} <- {"error": {"class": "GenericError", "desc": "Device '(null)' is in use"}} Broken when commit 3b1dbd11a60 made @device optional. Fixed in commit ed3d2ec98a3 (block: Add errp to b{lk,drv}_truncate()), except for this one instance. Fix it by using the error message provided by the op blocker instead, so it fails like this: <- {"error": {"class": "GenericError", "desc": "Node 'blk1' is busy: block device is in use by block job: mirror"}} Fixes: 3b1dbd11a60d (qmp: Allow block_resize to manipulate bs graph nodes.) Signed-off-by: Markus Armbruster <armbru@redhat.com> --- blockdev.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)