[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PULL 30/42] block: define get_block_status return valu

From: Peter Lieven
Subject: Re: [Qemu-devel] [PULL 30/42] block: define get_block_status return value
Date: Fri, 13 Sep 2013 10:51:36 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130804 Thunderbird/17.0.8

On 13.09.2013 10:26, Paolo Bonzini wrote:
Il 13/09/2013 10:04, Peter Lieven ha scritto:
On 06.09.2013 17:39, Stefan Hajnoczi wrote:
From: Paolo Bonzini <address@hidden>

Define the return value of get_block_status.  Bits 0, 1, 2 and 9-62
are valid; bit 63 (the sign bit) is reserved for errors.  Bits 3-8
are left for future extensions.

The return code is compatible with the old is_allocated API: if a driver
only returns 0 or 1 (aka BDRV_BLOCK_DATA) like is_allocated used to,
clients of is_allocated will not have any change in behavior.  Still,
we will return more precise information in the next patches and the
new definition of bdrv_is_allocated is already prepared for this.

Reviewed-by: Eric Blake <address@hidden>
Signed-off-by: Paolo Bonzini <address@hidden>
Signed-off-by: Stefan Hajnoczi <address@hidden>
   block.c               | 10 ++++++++--
   include/block/block.h | 26 ++++++++++++++++++++++++++
   2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index a5bfe74..97406ec 100644
--- a/block.c
+++ b/block.c
@@ -3093,7 +3093,7 @@ static int64_t coroutine_fn
bdrv_co_get_block_status(BlockDriverState *bs,
         if (!bs->drv->bdrv_co_get_block_status) {
           *pnum = nb_sectors;
-        return 1;
+        return BDRV_BLOCK_DATA;
         return bs->drv->bdrv_co_get_block_status(bs, sector_num,
nb_sectors, pnum);
@@ -3143,7 +3143,13 @@ int64_t bdrv_get_block_status(BlockDriverState
*bs, int64_t sector_num,
   int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t
                                      int nb_sectors, int *pnum)
-    return bdrv_get_block_status(bs, sector_num, nb_sectors, pnum);
+    int64_t ret = bdrv_get_block_status(bs, sector_num, nb_sectors,
+    if (ret < 0) {
sorry if I repeat myself.

isn't here a *pnum = 0 missing?
I think an earlier patch makes it unnecessary, because I'm checking the
return status of bdrv_get_block_status and bdrv_is_allocated.  In any
case the right place to put it would be in bdrv_get_block_status.
Ok, then I think for safety it should be added here:

@@ -3104,13 +3151,14 @@ static int64_t coroutine_fn 
bdrv_co_get_block_status(BlockDriverState *bs,

     ret = bs->drv->bdrv_co_get_block_status(bs, sector_num, nb_sectors, pnum);
     if (ret < 0) {
+        *pnum = 0;
         return ret;

because the new get_block_status api requires a ret < 0 if an error occurs. in 
case of an error
someone will write sth like ret = -EINVAL in the block driver. this not 
necessary implies *pnum = 0; the old
bdrv_is_allocated framework required a *pnum = 0 on error, but if someone (like 
me for iscsi)
creates bdrv_get_block_status from scratch will not explicity do this.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]