Am 09.11.2021 um 07:44 hat Markus Armbruster geschrieben:
> Screwed up qemu-devel@nongnu.org, sorry for the inconvenience.
>
> Markus Armbruster <arm...@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").
Not really any more. "No medium" is blk->root == NULL. These days
bs->drv == NULL basically means "the backend is broken". This happens
after qcow2_signal_corruption(), and I'm not sure if we have more
circumstances like it.
> > }
> > 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.
Sounds like we should write a test case and then fix it.
> > * 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()?
I guess we could philosophise about the theoretically right thing to do,
but last time I checked, host_cdrom didn't support snapshots, so it
probably doesn't matter either way.
Kevin
