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

[Luiz Capitulino]

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.

> 
> > Also misses QMP documentation.
> 
> Last patch hunk.

Ah, thanks, missed it.

> 
> >>          if (bs->drv) {
> >>              QObject *obj;
> >> -            QDict *bs_dict = qobject_to_qdict(bs_obj);
> >>  
> >>              obj = qobject_from_jsonf("{ 'file': %s, 'ro': %i, 'drv': %s, "
> >>                                       "'encrypted': %i }",
> [...]
> >> diff --git a/qmp-commands.hx b/qmp-commands.hx
> >> index 92c5c3a..cc5d30f 100644
> >> --- a/qmp-commands.hx
> >> +++ b/qmp-commands.hx
> >> @@ -1070,6 +1070,7 @@ Each json-object contain the following:
> >>           - Possible values: "unknown"
> >>  - "removable": true if the device is removable, false otherwise 
> >> (json-bool)
> >>  - "locked": true if the device is locked, false otherwise (json-bool)
> >> +- "ejected": present and true if the device ejected its media (json-bool)
> >>  - "inserted": only present if the device is inserted, it is a json-object
> >>     containing the following:
> >>           - "file": device file name (json-string)
>