Patchwork [RFC,12/36] block: sort BlockDeviceIoStatus errors by severity

login
register
mail settings
Submitter Paolo Bonzini
Date June 15, 2012, 3:05 p.m.
Message ID <1339772759-31004-13-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/165197/
State New
Headers show

Comments

Paolo Bonzini - June 15, 2012, 3:05 p.m.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block.c          |   11 ++++++++---
 qapi-schema.json |    2 +-
 2 files changed, 9 insertions(+), 4 deletions(-)
Eric Blake - June 15, 2012, 5:45 p.m.
On 06/15/2012 09:05 AM, Paolo Bonzini wrote:

> +++ b/qapi-schema.json
> @@ -429,7 +429,7 @@
>  #
>  # Since: 1.0
>  ##
> -{ 'enum': 'BlockDeviceIoStatus', 'data': [ 'ok', 'failed', 'nospace' ] }
> +{ 'enum': 'BlockDeviceIoStatus', 'data': [ 'ok', 'nospace', 'failed' ] }

Isn't this an ABI change?  Do we have any generated or client code where
'failed' mapping to 1 (pre-patch) vs. 2 (post-patch) would cause us
back-compat grief?
Paolo Bonzini - July 11, 2012, 4:03 p.m.
Il 15/06/2012 19:45, Eric Blake ha scritto:
>> > +++ b/qapi-schema.json
>> > @@ -429,7 +429,7 @@
>> >  #
>> >  # Since: 1.0
>> >  ##
>> > -{ 'enum': 'BlockDeviceIoStatus', 'data': [ 'ok', 'failed', 'nospace' ] }
>> > +{ 'enum': 'BlockDeviceIoStatus', 'data': [ 'ok', 'nospace', 'failed' ] }
> Isn't this an ABI change?  Do we have any generated or client code where
> 'failed' mapping to 1 (pre-patch) vs. 2 (post-patch) would cause us
> back-compat grief?

No, we do not generate any code (yet?) for consumption outside QEMU
itself.  In particular we do not install any generated header nor do we
use the generated enums in a (static or shared) library.

Paolo

Patch

diff --git a/block.c b/block.c
index 4011ce4..2f6384a 100644
--- a/block.c
+++ b/block.c
@@ -4133,9 +4133,14 @@  void bdrv_iostatus_reset(BlockDriverState *bs)
 void bdrv_iostatus_set_err(BlockDriverState *bs, int error)
 {
     assert(bdrv_iostatus_is_enabled(bs));
-    if (bs->iostatus == BLOCK_DEVICE_IO_STATUS_OK) {
-        bs->iostatus = error == ENOSPC ? BLOCK_DEVICE_IO_STATUS_NOSPACE :
-                                         BLOCK_DEVICE_IO_STATUS_FAILED;
+    BlockDeviceIoStatus new_status =
+        (error == ENOSPC ? BLOCK_DEVICE_IO_STATUS_NOSPACE :
+                           BLOCK_DEVICE_IO_STATUS_FAILED);
+
+    /* iostatus values are sorted from less severe to most severe
+     * (ok, nospace, failed).  */
+    if (bs->iostatus < new_status) {
+        bs->iostatus = new_status;
     }
 }
 
diff --git a/qapi-schema.json b/qapi-schema.json
index b760bc8..d09d267 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -429,7 +429,7 @@ 
 #
 # Since: 1.0
 ##
-{ 'enum': 'BlockDeviceIoStatus', 'data': [ 'ok', 'failed', 'nospace' ] }
+{ 'enum': 'BlockDeviceIoStatus', 'data': [ 'ok', 'nospace', 'failed' ] }
 
 ##
 # @BlockInfo: