On Wed, 4 Jan 2012 17:38:23 +0000
Stefan Hajnoczi <stefa...@linux.vnet.ibm.com> wrote:
> Let's report specific errors so that management tools and users can
> identify the problem.
>
> Two new qerrors are needed:
> * QERR_DEVICE_HAS_NO_MEDIUM for ENOMEDIUM
> * QERR_DEVICE_IS_READ_ONLY for EACCES
Great series, the number of complaints about generic errors have increased
lately. It's to fix this.
There's a missing bit though, would you mind to update the block_resize's
command documentation in the schema?
Thanks!
>
> Signed-off-by: Stefan Hajnoczi <stefa...@linux.vnet.ibm.com>
> ---
> blockdev.c | 26 ++++++++++++++++++--------
> qerror.c | 8 ++++++++
> qerror.h | 6 ++++++
> 3 files changed, 32 insertions(+), 8 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index c832782..8c2c8cc 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -841,11 +841,6 @@ int do_drive_del(Monitor *mon, const QDict *qdict,
> QObject **ret_data)
> return 0;
> }
>
> -/*
> - * XXX: replace the QERR_UNDEFINED_ERROR errors with real values once the
> - * existing QERR_ macro mess is cleaned up. A good example for better
> - * error reports can be found in the qemu-img resize code.
> - */
> void qmp_block_resize(const char *device, int64_t size, Error **errp)
> {
> BlockDriverState *bs;
> @@ -857,12 +852,27 @@ void qmp_block_resize(const char *device, int64_t size,
> Error **errp)
> }
>
> if (size < 0) {
> - error_set(errp, QERR_UNDEFINED_ERROR);
> + error_set(errp, QERR_INVALID_PARAMETER_VALUE, "size", "a >0 size");
> return;
> }
>
> - if (bdrv_truncate(bs, size)) {
> + switch (bdrv_truncate(bs, size)) {
> + case 0:
> + break;
> + case -ENOMEDIUM:
> + error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
> + break;
> + case -ENOTSUP:
> + error_set(errp, QERR_UNSUPPORTED);
> + break;
> + case -EACCES:
> + error_set(errp, QERR_DEVICE_IS_READ_ONLY, device);
> + break;
> + case -EBUSY:
> + error_set(errp, QERR_DEVICE_IN_USE, device);
> + break;
> + default:
> error_set(errp, QERR_UNDEFINED_ERROR);
> - return;
> + break;
> }
> }
> diff --git a/qerror.c b/qerror.c
> index 2979b3e..3d95383 100644
> --- a/qerror.c
> +++ b/qerror.c
> @@ -80,6 +80,10 @@ static const QErrorStringTable qerror_table[] = {
> .desc = "Migration is disabled when using feature '%(feature)'
> in device '%(device)'",
> },
> {
> + .error_fmt = QERR_DEVICE_HAS_NO_MEDIUM,
> + .desc = "Device '%(device)' has no medium",
> + },
> + {
> .error_fmt = QERR_DEVICE_INIT_FAILED,
> .desc = "Device '%(device)' could not be initialized",
> },
> @@ -88,6 +92,10 @@ static const QErrorStringTable qerror_table[] = {
> .desc = "Device '%(device)' is in use",
> },
> {
> + .error_fmt = QERR_DEVICE_IS_READ_ONLY,
> + .desc = "Device '%(device)' is read only",
> + },
> + {
> .error_fmt = QERR_DEVICE_LOCKED,
> .desc = "Device '%(device)' is locked",
> },
> diff --git a/qerror.h b/qerror.h
> index c34674e..a693d49 100644
> --- a/qerror.h
> +++ b/qerror.h
> @@ -81,12 +81,18 @@ QError *qobject_to_qerror(const QObject *obj);
> #define QERR_DEVICE_FEATURE_BLOCKS_MIGRATION \
> "{ 'class': 'DeviceFeatureBlocksMigration', 'data': { 'device': %s,
> 'feature': %s } }"
>
> +#define QERR_DEVICE_HAS_NO_MEDIUM \
> + "{ 'class': 'DeviceHasNoMedium', 'data', { 'name': %s } }"
> +
> #define QERR_DEVICE_INIT_FAILED \
> "{ 'class': 'DeviceInitFailed', 'data': { 'device': %s } }"
>
> #define QERR_DEVICE_IN_USE \
> "{ 'class': 'DeviceInUse', 'data': { 'device': %s } }"
>
> +#define QERR_DEVICE_IS_READ_ONLY \
> + "{ 'class': 'DeviceIsReadOnly', 'data': { 'device': %s } }"
> +
> #define QERR_DEVICE_LOCKED \
> "{ 'class': 'DeviceLocked', 'data': { 'device': %s } }"
>
