Patchwork [12/47] block: sort BlockDeviceIoStatus errors by severity

login
register
mail settings
Submitter Paolo Bonzini
Date July 24, 2012, 11:03 a.m.
Message ID <1343127865-16608-13-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/172871/
State New
Headers show

Comments

Paolo Bonzini - July 24, 2012, 11:03 a.m.
This does not let a "failed" (EIO) status override a "nospace" status.
When several concurrent asynchronous operations fail, management will
always observe the most severe condition.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block.c          |   11 ++++++++---
 qapi-schema.json |    2 +-
 2 files changed, 9 insertions(+), 4 deletions(-)
Paolo Bonzini - Aug. 1, 2012, 9:44 a.m.
Il 24/07/2012 13:03, Paolo Bonzini ha scritto:
> This does not let a "failed" (EIO) status override a "nospace" status.
> When several concurrent asynchronous operations fail, management will
> always observe the most severe condition.

Patch dropped; Kevin noted on IRC that if you have both errors you need to
take separate actions to fix them.  So when one is fixed the requeued
request will always fail and it's not important which error is signaled
first.

Paolo
Kevin Wolf - Aug. 1, 2012, 9:44 a.m.
Am 24.07.2012 13:03, schrieb Paolo Bonzini:
> This does not let a "failed" (EIO) status override a "nospace" status.
> When several concurrent asynchronous operations fail, management will
> always observe the most severe condition.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

As discussed in IRC, the commit message doesn't agree with the patch.
Probably the patch isn't needed at all because resuming after fixing the
first error will always give you the second one, and it doesn't really
matter which one you fix first.

Kevin

Patch

diff --git a/block.c b/block.c
index 333a8fd..dce07b3 100644
--- a/block.c
+++ b/block.c
@@ -3883,9 +3883,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 136ce5e..2dee7c3 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -435,7 +435,7 @@ 
 #
 # Since: 1.0
 ##
-{ 'enum': 'BlockDeviceIoStatus', 'data': [ 'ok', 'failed', 'nospace' ] }
+{ 'enum': 'BlockDeviceIoStatus', 'data': [ 'ok', 'nospace', 'failed' ] }
 
 ##
 # @BlockInfo: