andrzej zaborowski <balr...@gmail.com> writes: > On 20 July 2011 18:24, Markus Armbruster <arm...@redhat.com> wrote: >> We try the drive defined with -drive if=ide,index=0 (or equivalent >> sugar). We use it only if (dinfo && bdrv_is_inserted(dinfo->bdrv) && >> !bdrv_is_removable(dinfo->bdrv)). This is a convoluted way to test >> for "drive media can't be removed". >> >> The only way to create such a drive with -drive if=ide is media=cdrom. >> And that sets dinfo->media_cd, so just test that. > > This is a less generic test and more prone to be broken inadvertently, > so it seems like a step back. What's the argument against the > convoluted and explicit test?
My motivation for changing it was to reduce the uses of BlockDriverState member removable prior to nuking it from orbit [PATCH 45/55]. I consider my change an improvement, because I find "dinfo->media_cd" clearer than "bdrv_is_inserted(dinfo->bdrv) && !bdrv_is_removable(dinfo->bdrv)". Moreover, it's the only place that uses bdrv_is_removable() to test whether the block driver supports media eject. All the other places use dinfo->media_cd: hw/scsi-disk.c hw/xen_devconfig.c hw/ide/core.c hw/ide/qdev.c. Can't see why spitz & tosa need to check the same thing differently, and why it's worth keeping bdrv_is_removable() around just for that.
