[Qemu-devel] [PATCH] block: Remove bdrv_make_anon()

[Qemu-devel] [PATCH] block: Remove bdrv_make_anon()

[Kevin Wolf]

The call in hmp_drive_del() is dead code because blk_remove_bs() is
called a few lines above. The only other remaining user is
bdrv_delete(), which only abuses bdrv_make_anon() to remove it from the
named nodes list. This path inlines the list entry removal into
bdrv_delete() and removes bdrv_make_anon().

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c               | 15 +++------------
 blockdev.c            |  3 ---
 include/block/block.h |  1 -
 3 files changed, 3 insertions(+), 16 deletions(-)

diff --git a/block.c b/block.c
index b4107fc..e0b280b 100644
--- a/block.c
+++ b/block.c
@@ -2241,16 +2241,6 @@ void bdrv_close_all(void)
     }
 }
 
-/* make a BlockDriverState anonymous by removing from graph_bdrv_state list.
- * Also, NULL terminate the device_name to prevent double remove */
-void bdrv_make_anon(BlockDriverState *bs)
-{
-    if (bs->node_name[0] != '\0') {
-        QTAILQ_REMOVE(&graph_bdrv_states, bs, node_list);
-    }
-    bs->node_name[0] = '\0';
-}
-
 /* Fields that need to stay with the top-level BDS */
 static void bdrv_move_feature_fields(BlockDriverState *bs_dest,
                                      BlockDriverState *bs_src)
@@ -2380,8 +2370,9 @@ static void bdrv_delete(BlockDriverState *bs)
     bdrv_close(bs);
 
     /* remove from list, if necessary */
-    bdrv_make_anon(bs);
-
+    if (bs->node_name[0] != '\0') {
+        QTAILQ_REMOVE(&graph_bdrv_states, bs, node_list);
+    }
     QTAILQ_REMOVE(&all_bdrv_states, bs, bs_list);
 
     g_free(bs);
diff --git a/blockdev.c b/blockdev.c
index 85dee57..a9118c8 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2867,9 +2867,6 @@ void hmp_drive_del(Monitor *mon, const QDict *qdict)
 
     /* Make the BlockBackend and the attached BlockDriverState anonymous */
     monitor_remove_blk(blk);
-    if (blk_bs(blk)) {
-        bdrv_make_anon(blk_bs(blk));
-    }
 
     /* If this BlockBackend has a device attached to it, its refcount will be
      * decremented when the device is removed; otherwise we have to do so here.
diff --git a/include/block/block.h b/include/block/block.h
index 01349ef..4adb1e9 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -201,7 +201,6 @@ int bdrv_create(BlockDriver *drv, const char* filename,
 int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp);
 BlockDriverState *bdrv_new_root(void);
 BlockDriverState *bdrv_new(void);
-void bdrv_make_anon(BlockDriverState *bs);
 void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top);
 void bdrv_replace_in_backing_chain(BlockDriverState *old,
                                    BlockDriverState *new);
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] block: Remove bdrv_make_anon()

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 778 bytes --]

On 03/18/2016 04:31 AM, Kevin Wolf wrote:
> The call in hmp_drive_del() is dead code because blk_remove_bs() is
> called a few lines above. The only other remaining user is
> bdrv_delete(), which only abuses bdrv_make_anon() to remove it from the
> named nodes list. This path inlines the list entry removal into
> bdrv_delete() and removes bdrv_make_anon().
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block.c               | 15 +++------------
>  blockdev.c            |  3 ---
>  include/block/block.h |  1 -
>  3 files changed, 3 insertions(+), 16 deletions(-)

Nice diffstat, as well.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] block: Remove bdrv_make_anon()

[Max Reitz]

[-- Attachment #1.1: Type: text/plain, Size: 759 bytes --]

On 18.03.2016 11:31, Kevin Wolf wrote:
> The call in hmp_drive_del() is dead code because blk_remove_bs() is
> called a few lines above.

Ah, so that's why I didn't have it in v3. Thanks for solving that
mystery for me. :-)

>                           The only other remaining user is
> bdrv_delete(), which only abuses bdrv_make_anon() to remove it from the
> named nodes list. This path inlines the list entry removal into
> bdrv_delete() and removes bdrv_make_anon().
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block.c               | 15 +++------------
>  blockdev.c            |  3 ---
>  include/block/block.h |  1 -
>  3 files changed, 3 insertions(+), 16 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH] block: Remove bdrv_make_anon()

[Kevin Wolf]

[-- Attachment #1: Type: text/plain, Size: 972 bytes --]

Am 23.03.2016 um 21:11 hat Max Reitz geschrieben:
> On 18.03.2016 11:31, Kevin Wolf wrote:
> > The call in hmp_drive_del() is dead code because blk_remove_bs() is
> > called a few lines above.
> 
> Ah, so that's why I didn't have it in v3. Thanks for solving that
> mystery for me. :-)

I could have mentioned it in the review, but I figured it was quicker to
just send a follow-up patch.

> >                           The only other remaining user is
> > bdrv_delete(), which only abuses bdrv_make_anon() to remove it from the
> > named nodes list. This path inlines the list entry removal into
> > bdrv_delete() and removes bdrv_make_anon().
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >  block.c               | 15 +++------------
> >  blockdev.c            |  3 ---
> >  include/block/block.h |  1 -
> >  3 files changed, 3 insertions(+), 16 deletions(-)
> 
> Reviewed-by: Max Reitz <mreitz@redhat.com>

Thanks.

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]