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

[Eric Blake]

On 07/11/2017 11:37 AM, Manos Pitsidianakis wrote:
> This function is not used anywhere, so remove it.
> 
> Signed-off-by: Manos Pitsidianakis 
> ---
>  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



signature.asc
Description: OpenPGP digital signature

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

[Manos Pitsidianakis]

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  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!


signature.asc
Description: PGP signature

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

[Kevin Wolf]

Am 12.07.2017 um 09:41 hat Markus Armbruster geschrieben:
> Eric Blake  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

[Stefan Hajnoczi]

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 
> ---
>  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 


signature.asc
Description: PGP signature

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

[Markus Armbruster]

Eric Blake  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

[Eric Blake]

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 
> ---
>  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 

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



signature.asc
Description: OpenPGP digital signature

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

[Manos Pitsidianakis]

This function is not used anywhere, so remove it.

Signed-off-by: Manos Pitsidianakis 
---
 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  = _refresh_limits,
 .bdrv_probe_blocksizes = _probe_blocksizes,
 .bdrv_probe_geometry  = _probe_geometry,
-.bdrv_media_changed   = _media_changed,
 .bdrv_eject   = _eject,
 .bdrv_lock_medium = _lock_medium,
 .bdrv_co_ioctl= _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