Message ID | 1325698703-15370-4-git-send-email-stefanha@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
On Wed, 4 Jan 2012 17:38:23 +0000 Stefan Hajnoczi <stefanha@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 <stefanha@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 } }" >
On Wed, Jan 4, 2012 at 7:59 PM, Luiz Capitulino <lcapitulino@redhat.com> wrote: > On Wed, 4 Jan 2012 17:38:23 +0000 > Stefan Hajnoczi <stefanha@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? Sure, fixed in v2.
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 } }"
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 Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> --- blockdev.c | 26 ++++++++++++++++++-------- qerror.c | 8 ++++++++ qerror.h | 6 ++++++ 3 files changed, 32 insertions(+), 8 deletions(-)