Re: [Qemu-devel] [PATCH 23/55] block: Show whether the guest ejected the medium in info block

[Luiz Capitulino]

On Thu, 21 Jul 2011 13:38:46 -0300
Luiz Capitulino <address@hidden> wrote:

> On Thu, 21 Jul 2011 17:08:56 +0200
> Markus Armbruster <address@hidden> wrote:
> 
> > Luiz Capitulino <address@hidden> writes:
> > 
> > > On Wed, 20 Jul 2011 18:23:57 +0200
> > > Markus Armbruster <address@hidden> wrote:
> > >
> > >> Need to ask the device, so this requires new BlockDevOps member
> > >> is_medium_ejected().
> > >> 
> > >> Signed-off-by: Markus Armbruster <address@hidden>
> > >> ---
> > >>  block.c         |   18 ++++++++++++++++--
> > >>  block.h         |    6 ++++++
> > >>  hw/ide/core.c   |    6 ++++++
> > >>  hw/scsi-disk.c  |   10 ++++++++++
> > >>  qmp-commands.hx |    1 +
> > >>  5 files changed, 39 insertions(+), 2 deletions(-)
> > >> 
> > >> diff --git a/block.c b/block.c
> > >> index ba24bf1..6759066 100644
> > >> --- a/block.c
> > >> +++ b/block.c
> > >> @@ -761,6 +761,14 @@ static void 
> > >> bdrv_dev_change_media_cb(BlockDriverState *bs)
> > >>      }
> > >>  }
> > >>  
> > >> +bool bdrv_dev_is_medium_ejected(BlockDriverState *bs)
> > >> +{
> > >> +    if (bs->dev_ops && bs->dev_ops->is_medium_ejected) {
> > >> +        return bs->dev_ops->is_medium_ejected(bs->dev_opaque);
> > >> +    }
> > >> +    return false;
> > >> +}
> > >> +
> > >>  static void bdrv_dev_resize_cb(BlockDriverState *bs)
> > >>  {
> > >>      if (bs->dev_ops && bs->dev_ops->resize_cb) {
> > >> @@ -1712,7 +1720,9 @@ static void bdrv_print_dict(QObject *obj, void 
> > >> *opaque)
> > >>      if (qdict_get_bool(bs_dict, "removable")) {
> > >>          monitor_printf(mon, " locked=%d", qdict_get_bool(bs_dict, 
> > >> "locked"));
> > >>      }
> > >> -
> > >> +    if (qdict_get_try_bool(bs_dict, "ejected", false)) {
> > >> +        monitor_printf(mon, " ejected");
> > >> +    }
> > >>      if (qdict_haskey(bs_dict, "inserted")) {
> > >>          QDict *qdict = qobject_to_qdict(qdict_get(bs_dict, "inserted"));
> > >>  
> > >> @@ -1747,15 +1757,19 @@ void bdrv_info(Monitor *mon, QObject **ret_data)
> > >>  
> > >>      QTAILQ_FOREACH(bs, &bdrv_states, list) {
> > >>          QObject *bs_obj;
> > >> +        QDict *bs_dict;
> > >>  
> > >>          bs_obj = qobject_from_jsonf("{ 'device': %s, 'type': 'unknown', 
> > >> "
> > >>                                      "'removable': %i, 'locked': %i }",
> > >>                                      bs->device_name, bs->removable,
> > >>                                      bs->locked);
> > >> +        bs_dict = qobject_to_qdict(bs_obj);
> > >>  
> > >> +        if (bdrv_dev_is_medium_ejected(bs)) {
> > >> +            qdict_put(bs_dict, "ejected", qbool_from_int(1));
> > >> +        }
> > >
> > > I'd expect 'ejected' to be always present. It's a bool afterall, so it's
> > > either true or false.
> > >
> > > But I see that "ejected=false" might not be ideal. What about calling it
> > > "inserted" then?
> > 
> > "inserted=true" for disks looks odd to me, but whatever floats your boat
> > :)
> 
> I meant always present for removable media. Maybe "media-inserted" is even
> better.

Given we already have a 'inserted' key, we could call it "media-ejected" which
is always present for devices that support it.