[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH-for-5.2 v2 2/2] block/block-backend: Let blk_attach_dev()
From: |
Daniel P . Berrangé |
Subject: |
Re: [RFC PATCH-for-5.2 v2 2/2] block/block-backend: Let blk_attach_dev() provide helpful error |
Date: |
Thu, 16 Jul 2020 14:04:40 +0100 |
User-agent: |
Mutt/1.14.5 (2020-06-23) |
On Thu, Jul 16, 2020 at 02:37:04PM +0200, Philippe Mathieu-Daudé wrote:
> Let blk_attach_dev() take an Error* object to return helpful
> information. Adapt the callers.
>
> $ qemu-system-arm -M n800
> qemu-system-arm: sd_init failed: cannot attach blk 'sd0' to device 'sd-card'
> blk 'sd0' is already attached by device 'omap2-mmc'
> Drive 'sd0' is already in use because it has been automatically connected
> to another device
> (do you need 'if=none' in the drive options?)
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> v2: Rebased after 668f62ec62 ("error: Eliminate error_propagate()")
> ---
> include/sysemu/block-backend.h | 2 +-
> block/block-backend.c | 11 ++++++++++-
> hw/block/fdc.c | 4 +---
> hw/block/swim.c | 4 +---
> hw/block/xen-block.c | 5 +++--
> hw/core/qdev-properties-system.c | 16 +++++++++-------
> hw/ide/qdev.c | 4 +---
> hw/scsi/scsi-disk.c | 4 +---
> 8 files changed, 27 insertions(+), 23 deletions(-)
>
> diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
> index 8203d7f6f9..118fbad0b4 100644
> --- a/include/sysemu/block-backend.h
> +++ b/include/sysemu/block-backend.h
> @@ -113,7 +113,7 @@ BlockDeviceIoStatus blk_iostatus(const BlockBackend *blk);
> void blk_iostatus_disable(BlockBackend *blk);
> void blk_iostatus_reset(BlockBackend *blk);
> void blk_iostatus_set_err(BlockBackend *blk, int error);
> -int blk_attach_dev(BlockBackend *blk, DeviceState *dev);
> +int blk_attach_dev(BlockBackend *blk, DeviceState *dev, Error **errp);
> void blk_detach_dev(BlockBackend *blk, DeviceState *dev);
> DeviceState *blk_get_attached_dev(BlockBackend *blk);
> char *blk_get_attached_dev_id(BlockBackend *blk);
> diff --git a/block/block-backend.c b/block/block-backend.c
> index 63ff940ef9..b7be0a4619 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -884,12 +884,21 @@ void blk_get_perm(BlockBackend *blk, uint64_t *perm,
> uint64_t *shared_perm)
>
> /*
> * Attach device model @dev to @blk.
> + *
> + * @blk: Block backend
> + * @dev: Device to attach the block backend to
> + * @errp: pointer to NULL initialized error object
> + *
> * Return 0 on success, -EBUSY when a device model is attached already.
> */
> -int blk_attach_dev(BlockBackend *blk, DeviceState *dev)
> +int blk_attach_dev(BlockBackend *blk, DeviceState *dev, Error **errp)
> {
> trace_blk_attach_dev(blk_name(blk), object_get_typename(OBJECT(dev)));
> if (blk->dev) {
> + error_setg(errp, "cannot attach blk '%s' to device '%s'",
> + blk_name(blk), object_get_typename(OBJECT(dev)));
> + error_append_hint(errp, "blk '%s' is already attached by device
> '%s'\n",
> + blk_name(blk),
> object_get_typename(OBJECT(blk->dev)));
I would have a preference for expanding the main error message and not
using a hint. Any hint is completely thrown away when using QMP :-(
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|