diff mbox

[v5,20/38] block: Prepare remaining BB functions for NULL BDS

Message ID 1442589793-7105-21-git-send-email-mreitz@redhat.com
State New
Headers show

Commit Message

Max Reitz Sept. 18, 2015, 3:22 p.m. UTC 
There are several BlockBackend functions which, in theory, cannot fail.
This patch makes them cope with the BlockDriverState pointer being NULL
by making them fall back to some default action like ignoring the value
in setters and returning the default in getters.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block/block-backend.c | 76 ++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 60 insertions(+), 16 deletions(-)

Comments

Kevin Wolf Sept. 22, 2015, 2:35 p.m. UTC | #1 
Am 18.09.2015 um 17:22 hat Max Reitz geschrieben:
> There are several BlockBackend functions which, in theory, cannot fail.
> This patch makes them cope with the BlockDriverState pointer being NULL
> by making them fall back to some default action like ignoring the value
> in setters and returning the default in getters.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Hm, okay, maybe bdrv_drain() belongs here. I just reviewed the end
result for completeness and didn't check which patch did what.

>  int blk_enable_write_cache(BlockBackend *blk)
>  {
> +    if (!blk->bs) {
> +        return 0;
> +    }
> +
>      return bdrv_enable_write_cache(blk->bs);
>  }
>  
>  void blk_set_enable_write_cache(BlockBackend *blk, bool wce)
>  {
> -    bdrv_set_enable_write_cache(blk->bs, wce);
> +    if (blk->bs) {
> +        bdrv_set_enable_write_cache(blk->bs, wce);
> +    }
>  }

WCE is part of the open_flags in BlockBackendRootState. The root state
doesn't seem to be used yet, and I wrote about my concerns about it in
reply to an earlier patch, but as long as we have it, should this
query/modify the root state if no BDS is attached?

Kevin
Max Reitz Sept. 22, 2015, 3:07 p.m. UTC | #2 
On 22.09.2015 16:35, Kevin Wolf wrote:
> Am 18.09.2015 um 17:22 hat Max Reitz geschrieben:
>> There are several BlockBackend functions which, in theory, cannot fail.
>> This patch makes them cope with the BlockDriverState pointer being NULL
>> by making them fall back to some default action like ignoring the value
>> in setters and returning the default in getters.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> Hm, okay, maybe bdrv_drain() belongs here. I just reviewed the end
> result for completeness and didn't check which patch did what.
> 
>>  int blk_enable_write_cache(BlockBackend *blk)
>>  {
>> +    if (!blk->bs) {
>> +        return 0;
>> +    }
>> +
>>      return bdrv_enable_write_cache(blk->bs);
>>  }
>>  
>>  void blk_set_enable_write_cache(BlockBackend *blk, bool wce)
>>  {
>> -    bdrv_set_enable_write_cache(blk->bs, wce);
>> +    if (blk->bs) {
>> +        bdrv_set_enable_write_cache(blk->bs, wce);
>> +    }
>>  }
> 
> WCE is part of the open_flags in BlockBackendRootState. The root state
> doesn't seem to be used yet, and I wrote about my concerns about it in
> reply to an earlier patch, but as long as we have it, should this
> query/modify the root state if no BDS is attached?

Seems very reasonable, will do.

Max
diff mbox

Patch

diff --git a/block/block-backend.c b/block/block-backend.c
index 0da4be9..33145f8 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -657,7 +657,11 @@  int64_t blk_getlength(BlockBackend *blk)
 
 void blk_get_geometry(BlockBackend *blk, uint64_t *nb_sectors_ptr)
 {
-    bdrv_get_geometry(blk->bs, nb_sectors_ptr);
+    if (!blk->bs) {
+        *nb_sectors_ptr = 0;
+    } else {
+        bdrv_get_geometry(blk->bs, nb_sectors_ptr);
+    }
 }
 
 int64_t blk_nb_sectors(BlockBackend *blk)
@@ -889,17 +893,27 @@  int blk_is_read_only(BlockBackend *blk)
 
 int blk_is_sg(BlockBackend *blk)
 {
+    if (!blk->bs) {
+        return 0;
+    }
+
     return bdrv_is_sg(blk->bs);
 }
 
 int blk_enable_write_cache(BlockBackend *blk)
 {
+    if (!blk->bs) {
+        return 0;
+    }
+
     return bdrv_enable_write_cache(blk->bs);
 }
 
 void blk_set_enable_write_cache(BlockBackend *blk, bool wce)
 {
-    bdrv_set_enable_write_cache(blk->bs, wce);
+    if (blk->bs) {
+        bdrv_set_enable_write_cache(blk->bs, wce);
+    }
 }
 
 void blk_invalidate_cache(BlockBackend *blk, Error **errp)
@@ -924,12 +938,16 @@  bool blk_is_available(BlockBackend *blk)
 
 void blk_lock_medium(BlockBackend *blk, bool locked)
 {
-    bdrv_lock_medium(blk->bs, locked);
+    if (blk->bs) {
+        bdrv_lock_medium(blk->bs, locked);
+    }
 }
 
 void blk_eject(BlockBackend *blk, bool eject_flag)
 {
-    bdrv_eject(blk->bs, eject_flag);
+    if (blk->bs) {
+        bdrv_eject(blk->bs, eject_flag);
+    }
 }
 
 int blk_get_flags(BlockBackend *blk)
@@ -943,7 +961,11 @@  int blk_get_flags(BlockBackend *blk)
 
 int blk_get_max_transfer_length(BlockBackend *blk)
 {
-    return blk->bs->bl.max_transfer_length;
+    if (blk->bs) {
+        return blk->bs->bl.max_transfer_length;
+    } else {
+        return 0;
+    }
 }
 
 void blk_set_guest_block_size(BlockBackend *blk, int align)
@@ -958,22 +980,32 @@  void *blk_blockalign(BlockBackend *blk, size_t size)
 
 bool blk_op_is_blocked(BlockBackend *blk, BlockOpType op, Error **errp)
 {
+    if (!blk->bs) {
+        return false;
+    }
+
     return bdrv_op_is_blocked(blk->bs, op, errp);
 }
 
 void blk_op_unblock(BlockBackend *blk, BlockOpType op, Error *reason)
 {
-    bdrv_op_unblock(blk->bs, op, reason);
+    if (blk->bs) {
+        bdrv_op_unblock(blk->bs, op, reason);
+    }
 }
 
 void blk_op_block_all(BlockBackend *blk, Error *reason)
 {
-    bdrv_op_block_all(blk->bs, reason);
+    if (blk->bs) {
+        bdrv_op_block_all(blk->bs, reason);
+    }
 }
 
 void blk_op_unblock_all(BlockBackend *blk, Error *reason)
 {
-    bdrv_op_unblock_all(blk->bs, reason);
+    if (blk->bs) {
+        bdrv_op_unblock_all(blk->bs, reason);
+    }
 }
 
 AioContext *blk_get_aio_context(BlockBackend *blk)
@@ -993,15 +1025,19 @@  static AioContext *blk_aiocb_get_aio_context(BlockAIOCB *acb)
 
 void blk_set_aio_context(BlockBackend *blk, AioContext *new_context)
 {
-    bdrv_set_aio_context(blk->bs, new_context);
+    if (blk->bs) {
+        bdrv_set_aio_context(blk->bs, new_context);
+    }
 }
 
 void blk_add_aio_context_notifier(BlockBackend *blk,
         void (*attached_aio_context)(AioContext *new_context, void *opaque),
         void (*detach_aio_context)(void *opaque), void *opaque)
 {
-    bdrv_add_aio_context_notifier(blk->bs, attached_aio_context,
-                                  detach_aio_context, opaque);
+    if (blk->bs) {
+        bdrv_add_aio_context_notifier(blk->bs, attached_aio_context,
+                                      detach_aio_context, opaque);
+    }
 }
 
 void blk_remove_aio_context_notifier(BlockBackend *blk,
@@ -1010,23 +1046,31 @@  void blk_remove_aio_context_notifier(BlockBackend *blk,
                                      void (*detach_aio_context)(void *),
                                      void *opaque)
 {
-    bdrv_remove_aio_context_notifier(blk->bs, attached_aio_context,
-                                     detach_aio_context, opaque);
+    if (blk->bs) {
+        bdrv_remove_aio_context_notifier(blk->bs, attached_aio_context,
+                                         detach_aio_context, opaque);
+    }
 }
 
 void blk_add_close_notifier(BlockBackend *blk, Notifier *notify)
 {
-    bdrv_add_close_notifier(blk->bs, notify);
+    if (blk->bs) {
+        bdrv_add_close_notifier(blk->bs, notify);
+    }
 }
 
 void blk_io_plug(BlockBackend *blk)
 {
-    bdrv_io_plug(blk->bs);
+    if (blk->bs) {
+        bdrv_io_plug(blk->bs);
+    }
 }
 
 void blk_io_unplug(BlockBackend *blk)
 {
-    bdrv_io_unplug(blk->bs);
+    if (blk->bs) {
+        bdrv_io_unplug(blk->bs);
+    }
 }
 
 BlockAcctStats *blk_get_stats(BlockBackend *blk)