[Qemu-devel] [PATCH v4 2/4] block: remove bdrv_media_changed

Manos Pitsidianakis posted 4 patches 8 years, 7 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v4 2/4] block: remove bdrv_media_changed
Posted by Manos Pitsidianakis 8 years, 7 months ago
This function is not used anywhere, so remove it.

Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr>
---
 block.c                   | 14 --------------
 block/raw-format.c        |  6 ------
 include/block/block.h     |  1 -
 include/block/block_int.h |  1 -
 4 files changed, 22 deletions(-)

diff --git a/block.c b/block.c
index 7f7530b6..5ca60f97 100644
--- a/block.c
+++ b/block.c
@@ -4213,20 +4213,6 @@ bool bdrv_is_inserted(BlockDriverState *bs)
 }
 
 /**
- * Return whether the media changed since the last call to this
- * function, or -ENOTSUP if we don't know.  Most drivers don't know.
- */
-int bdrv_media_changed(BlockDriverState *bs)
-{
-    BlockDriver *drv = bs->drv;
-
-    if (drv && drv->bdrv_media_changed) {
-        return drv->bdrv_media_changed(bs);
-    }
-    return -ENOTSUP;
-}
-
-/**
  * If eject_flag is TRUE, eject the media. Otherwise, close the tray
  */
 void bdrv_eject(BlockDriverState *bs, bool eject_flag)
diff --git a/block/raw-format.c b/block/raw-format.c
index 1ea8c2d7..df3bc5d5 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -346,11 +346,6 @@ static int raw_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
     return bdrv_truncate(bs->file, offset, errp);
 }
 
-static int raw_media_changed(BlockDriverState *bs)
-{
-    return bdrv_media_changed(bs->file->bs);
-}
-
 static void raw_eject(BlockDriverState *bs, bool eject_flag)
 {
     bdrv_eject(bs->file->bs, eject_flag);
@@ -483,7 +478,6 @@ BlockDriver bdrv_raw = {
     .bdrv_refresh_limits  = &raw_refresh_limits,
     .bdrv_probe_blocksizes = &raw_probe_blocksizes,
     .bdrv_probe_geometry  = &raw_probe_geometry,
-    .bdrv_media_changed   = &raw_media_changed,
     .bdrv_eject           = &raw_eject,
     .bdrv_lock_medium     = &raw_lock_medium,
     .bdrv_co_ioctl        = &raw_co_ioctl,
diff --git a/include/block/block.h b/include/block/block.h
index afe1b615..3cb4cae7 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -438,7 +438,6 @@ int bdrv_can_set_read_only(BlockDriverState *bs, bool read_only, Error **errp);
 int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp);
 bool bdrv_is_sg(BlockDriverState *bs);
 bool bdrv_is_inserted(BlockDriverState *bs);
-int bdrv_media_changed(BlockDriverState *bs);
 void bdrv_lock_medium(BlockDriverState *bs, bool locked);
 void bdrv_eject(BlockDriverState *bs, bool eject_flag);
 const char *bdrv_get_format_name(BlockDriverState *bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 75e93f72..3d9a8164 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -246,7 +246,6 @@ struct BlockDriver {
 
     /* removable device specific */
     bool (*bdrv_is_inserted)(BlockDriverState *bs);
-    int (*bdrv_media_changed)(BlockDriverState *bs);
     void (*bdrv_eject)(BlockDriverState *bs, bool eject_flag);
     void (*bdrv_lock_medium)(BlockDriverState *bs, bool locked);
 
-- 
2.11.0


Re: [Qemu-devel] [PATCH v4 2/4] block: remove bdrv_media_changed
Posted by Eric Blake 8 years, 7 months ago
On 07/11/2017 11:37 AM, Manos Pitsidianakis wrote:
> This function is not used anywhere, so remove it.
> 

Might be interesting to figure out when it WAS last used.  If I grepped
correctly, it was commit 21fcf360 back in May 2012?

> Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr>
> ---
>  block.c                   | 14 --------------
>  block/raw-format.c        |  6 ------
>  include/block/block.h     |  1 -
>  include/block/block_int.h |  1 -
>  4 files changed, 22 deletions(-)

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

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v4 2/4] block: remove bdrv_media_changed
Posted by Markus Armbruster 8 years, 7 months ago
Eric Blake <eblake@redhat.com> writes:

> On 07/11/2017 11:37 AM, Manos Pitsidianakis wrote:
>> This function is not used anywhere, so remove it.
>> 
>
> Might be interesting to figure out when it WAS last used.

Yes.  When I see "remove X because it's unused" during patch review, I
immediately ask "why is it unused now, and what was it used for
previously?"  Ideally, the commit message answers these questions
preemptively.

>                                                            If I grepped
> correctly, it was commit 21fcf360 back in May 2012?

Yes.  "fdc: simplify media change handling".  I suspect that commit
broke media change for passed-through host floppy.

Its only implementation went away in commit f709623 "block: Remove host
floppy support".

Suggest

    block: bdrv_media_changed() is unused, remove

    The i82078 floppy device model used to call bdrv_media_changed() to
    implement its media change bit when backed by a host floppy.  This
    went away in 21fcf36 "fdc: simplify media change handling".
    Probably broke host floppy media change.  Host floppy pass-through
    was dropped in commit f709623.  bdrv_media_changed() has never been
    used for anything else.  Remove it.

Re: [Qemu-devel] [PATCH v4 2/4] block: remove bdrv_media_changed
Posted by Kevin Wolf 8 years, 7 months ago
Am 12.07.2017 um 09:41 hat Markus Armbruster geschrieben:
> Eric Blake <eblake@redhat.com> writes:
> 
> > On 07/11/2017 11:37 AM, Manos Pitsidianakis wrote:
> >> This function is not used anywhere, so remove it.
> >> 
> >
> > Might be interesting to figure out when it WAS last used.
> 
> Yes.  When I see "remove X because it's unused" during patch review, I
> immediately ask "why is it unused now, and what was it used for
> previously?"  Ideally, the commit message answers these questions
> preemptively.
> 
> >                                                            If I grepped
> > correctly, it was commit 21fcf360 back in May 2012?
> 
> Yes.  "fdc: simplify media change handling".  I suspect that commit
> broke media change for passed-through host floppy.
> 
> Its only implementation went away in commit f709623 "block: Remove host
> floppy support".
> 
> Suggest
> 
>     block: bdrv_media_changed() is unused, remove
> 
>     The i82078 floppy device model used to call bdrv_media_changed() to
>     implement its media change bit when backed by a host floppy.  This
>     went away in 21fcf36 "fdc: simplify media change handling".
>     Probably broke host floppy media change.  Host floppy pass-through
>     was dropped in commit f709623.  bdrv_media_changed() has never been
>     used for anything else.  Remove it.

Manos, if you're happy with this, I can update the commit message while
applying the series.

Kevin

Re: [Qemu-devel] [PATCH v4 2/4] block: remove bdrv_media_changed
Posted by Manos Pitsidianakis 8 years, 7 months ago
On Wed, Jul 12, 2017 at 05:15:01PM +0200, Kevin Wolf wrote:
>Am 12.07.2017 um 09:41 hat Markus Armbruster geschrieben:
>> Eric Blake <eblake@redhat.com> writes:
>>
>> > On 07/11/2017 11:37 AM, Manos Pitsidianakis wrote:
>> >> This function is not used anywhere, so remove it.
>> >>
>> >
>> > Might be interesting to figure out when it WAS last used.
>>
>> Yes.  When I see "remove X because it's unused" during patch review, I
>> immediately ask "why is it unused now, and what was it used for
>> previously?"  Ideally, the commit message answers these questions
>> preemptively.
>>
>> >                                                            If I grepped
>> > correctly, it was commit 21fcf360 back in May 2012?
>>
>> Yes.  "fdc: simplify media change handling".  I suspect that commit
>> broke media change for passed-through host floppy.
>>
>> Its only implementation went away in commit f709623 "block: Remove host
>> floppy support".
>>
>> Suggest
>>
>>     block: bdrv_media_changed() is unused, remove
>>
>>     The i82078 floppy device model used to call bdrv_media_changed() to
>>     implement its media change bit when backed by a host floppy.  This
>>     went away in 21fcf36 "fdc: simplify media change handling".
>>     Probably broke host floppy media change.  Host floppy pass-through
>>     was dropped in commit f709623.  bdrv_media_changed() has never been
>>     used for anything else.  Remove it.
>
>Manos, if you're happy with this, I can update the commit message while
>applying the series.

Of course, no problem.
Thanks!
Re: [Qemu-devel] [PATCH v4 2/4] block: remove bdrv_media_changed
Posted by Stefan Hajnoczi 8 years, 7 months ago
On Tue, Jul 11, 2017 at 07:37:46PM +0300, Manos Pitsidianakis wrote:
> This function is not used anywhere, so remove it.
> 
> Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr>
> ---
>  block.c                   | 14 --------------
>  block/raw-format.c        |  6 ------
>  include/block/block.h     |  1 -
>  include/block/block_int.h |  1 -
>  4 files changed, 22 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Re: [Qemu-devel] [PATCH v4 2/4] block: remove bdrv_media_changed
Posted by Eric Blake 8 years, 7 months ago
On 07/11/2017 11:37 AM, Manos Pitsidianakis wrote:
> This function is not used anywhere, so remove it.
> 
> Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr>
> ---
>  block.c                   | 14 --------------
>  block/raw-format.c        |  6 ------
>  include/block/block.h     |  1 -
>  include/block/block_int.h |  1 -
>  4 files changed, 22 deletions(-)
> 

> +++ b/block/raw-format.c
> @@ -346,11 +346,6 @@ static int raw_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
>      return bdrv_truncate(bs->file, offset, errp);
>  }
>  
> -static int raw_media_changed(BlockDriverState *bs)
> -{
> -    return bdrv_media_changed(bs->file->bs);
> -}
> -

There's a merge conflict here if this lands after Max's pending block
pull request, but should be trivial to resolve.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org