On Fri, 27 May 2011 18:10:08 +0530
Amit Shah <amit.s...@redhat.com> wrote:
> On (Fri) 27 May 2011 [17:04:30], Amit Shah wrote:
> > On (Fri) 27 May 2011 [17:01:35], Amit Shah wrote:
> > > On (Thu) 26 May 2011 [15:29:29], Luiz Capitulino wrote:
> > > >
> > > > I'm testing with qemu.git (HEAD aa29141d84d), procedure:
> > > >
> > > > 1. Start a VM with:
> > > >
> > > > # qemu -hda disks/test.img -enable-kvm -m 1G -cdrom
> > > > Fedora-14-x86_64-DVD.iso
> > > >
> > > > 2. Then inside the guest run:
> > > >
> > > > # eject /dev/sr0 && mount /dev/sr0 /mnt
> > > >
> > > > Results:
> > > >
> > > > Actual: The cdrom is successfully mounted
> > > > Expected: The cdrom is not mounted (mount fails, medium not found)
> > >
> > > Really? That's what you expect? :-)
That was the VM behavior before 996faf1ad, therefore it's what I was
expecting.
> > > Where will the medium go?
We were leaking it then?
> > > What happens is mount auto-closes the tray and mounts whatever is
> > > there, which is the image you provided. Works as expected, IMO.
>
> Confirmed, that's what happens.
Ok. I got this by testing my series that adds the BLOCK_MEDIA_EJECT event.
At first I thought your commit wasn't handling the tray status correctly
(which would cause problems to the new event), but it seems to work fine,
specially now that I know what's doing. Sorry for the noise.
> What's weird though is 'eject' in the monitor makes the cdrom go away
> -- a subsequent mount in the guest results in a no medium error. I
> thought we had solved that, Markus?
>
> By not doing a bdrv_close() in the do_eject()->eject_device() call
> path this starts working as expected.
Yes, also note that with the -f option eject is capable of purging
any block device. I wonder if libvirt (or any client) relies on this.
IMHO, we should do the following:
1. Drop bdrv_close() from eject and change it to just eject a device
(which also means working only on removable media)
2. If we really want to be able to remove block devices from the VM,
we should add a new command for that
3. The change command does an eject followed by a bdrv_close(), this is
fine, considering we can't break compatibility here
