Re: Poking around bdrv_is_inserted()

[Markus Armbruster <armbru@redhat.com>]

Screwed up qemu-devel@nongnu.org, sorry for the inconvenience.

Markus Armbruster <armbru@redhat.com> writes:

> bdrv_is_inserted() returns false when:
>
>     /**
>      * Return TRUE if the media is present
>      */
>     bool bdrv_is_inserted(BlockDriverState *bs)
>     {
>         BlockDriver *drv = bs->drv;
>         BdrvChild *child;
>
>         if (!drv) {
>             return false;
>
> 1. @bs has no driver (this is how we represent "no medium").
>
>         }
>         if (drv->bdrv_is_inserted) {
>             return drv->bdrv_is_inserted(bs);
>
> 2. Its driver's ->bdrv_is_inserted() returns false.  This is how
> passthrough block backends signal "host device has no medium".  Right
> now, the only user is "host_cdrom".
>
>         }
>         QLIST_FOREACH(child, &bs->children, next) {
>             if (!bdrv_is_inserted(child->bs)) {
>                 return false;
>
> 3. Any of its children has no medium.  Common use looking through
> filters, which have a single child.
>
>             }
>         }
>         return true;
>     }
>
> Makes sense.
>
> Now look at the uses of QERR_DEVICE_HAS_NO_MEDIUM.
>
> * external_snapshot_prepare() in blockdev.c:
>
>     if (!bdrv_is_inserted(state->old_bs)) {
>         error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
>         goto out;
>     }
>
>   where @device is the device name, i.e. BlockdevSnapshot member @node
>   or BlockdevSnapshotSync member @device.  Uh-oh: the latter can be
>   null.  If we can reach the error_setg() then, we crash on some
>   systems.
>
> * bdrv_snapshot_delete() and bdrv_snapshot_load_tmp() in
>   block/snaphot.c:
>
>     if (!drv) {
>         error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, bdrv_get_device_name(bs));
>         return -ENOMEDIUM;
>     }
>
>   where @drv is bs->drv.
>
>   Why do we check only for 1. here instead of calling
>   bdrv_is_inserted()?