On 1 August 2011 13:33, Markus Armbruster <arm...@redhat.com> wrote: > 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)".
This seems like an argument for providing a bdrv_supports_eject() or whatever we're actually trying to test for here. I kind of felt the same way as Andrzej when I saw this patch going past but it got lost in my mail folder... -- PMM
