diff mbox

block/quorum: make quorum_getlength error message user friendly

Message ID 1405064065-10021-1-git-send-email-namei.unix@gmail.com
State New
Headers show

Commit Message

Liu Yuan July 11, 2014, 7:34 a.m. UTC
When start quorum driver with 2 different sized images, we get:

qemu-system-x86_64: -drive if=virtio,driver=quorum...: Could not refresh total \
sector count: Input/output error

EIO would confuse users. With this patch, the error message goes like

Children images are not in the same size
qemu-system-x86_64: -drive if=virtio,driver=quorum...: Could not refresh total \
sector count: Invalid argument

Cc: Benoit Canet <benoit@irqsave.net>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Liu Yuan <namei.unix@gmail.com>
---
 block/quorum.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Kevin Wolf July 11, 2014, 10:26 a.m. UTC | #1
Am 11.07.2014 um 09:34 hat Liu Yuan geschrieben:
> When start quorum driver with 2 different sized images, we get:
> 
> qemu-system-x86_64: -drive if=virtio,driver=quorum...: Could not refresh total \
> sector count: Input/output error
> 
> EIO would confuse users. With this patch, the error message goes like
> 
> Children images are not in the same size
> qemu-system-x86_64: -drive if=virtio,driver=quorum...: Could not refresh total \
> sector count: Invalid argument
> 
> Cc: Benoit Canet <benoit@irqsave.net>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Liu Yuan <namei.unix@gmail.com>

I think it would be better to do this check in quorum_open() instead of
relying on bdrv_open() to perform a bdrv_getlength(). As a bonus,
quorum_open() has an Error argument, so we wouldn't have to use
something like error_printf(), which doesn't work well for QMP commands.

Kevin
Liu Yuan July 12, 2014, 3:42 a.m. UTC | #2
On Fri, Jul 11, 2014 at 12:26:58PM +0200, Kevin Wolf wrote:
> Am 11.07.2014 um 09:34 hat Liu Yuan geschrieben:
> > When start quorum driver with 2 different sized images, we get:
> > 
> > qemu-system-x86_64: -drive if=virtio,driver=quorum...: Could not refresh total \
> > sector count: Input/output error
> > 
> > EIO would confuse users. With this patch, the error message goes like
> > 
> > Children images are not in the same size
> > qemu-system-x86_64: -drive if=virtio,driver=quorum...: Could not refresh total \
> > sector count: Invalid argument
> > 
> > Cc: Benoit Canet <benoit@irqsave.net>
> > Cc: Kevin Wolf <kwolf@redhat.com>
> > Cc: Stefan Hajnoczi <stefanha@redhat.com>
> > Signed-off-by: Liu Yuan <namei.unix@gmail.com>
> 
> I think it would be better to do this check in quorum_open() instead of
> relying on bdrv_open() to perform a bdrv_getlength(). As a bonus,
> quorum_open() has an Error argument, so we wouldn't have to use
> something like error_printf(), which doesn't work well for QMP commands.
> 
> Kevin

Make sense. I'll do it after we settle down my 'add simple read pattern' patch
since they are conflicted.

Yuan
diff mbox

Patch

diff --git a/block/quorum.c b/block/quorum.c
index 2f18755..51437ad 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -736,7 +736,8 @@  static int64_t quorum_getlength(BlockDriverState *bs)
             return value;
         }
         if (value != result) {
-            return -EIO;
+            error_printf("Children images are not in the same size\n");
+            return -EINVAL;
         }
     }