Stefan Hajnoczi <stefa...@linux.vnet.ibm.com> wrote:
> Piggy-back on the guest CD-ROM polling to poll on the host. Open and
> close the host CD-ROM file descriptor to ensure we read the new size and
> not a stale size.
>
> Two things are going on here:
>
> 1. If hald/udisks is not already polling CD-ROMs on the host then
> re-opening the CD-ROM causes the host to read the new medium's size.
>
> 2. There is a bug in Linux which means the CD-ROM file descriptor must
> be re-opened in order for lseek(2) to see the new size. The
> inode size gets out of sync with the underlying device (which you can
> confirm by checking that /sys/block/sr0/size and lseek(2) do not
> match after media change). I have raised this with the
> maintainers but we need a workaround for the foreseeable future.
>
> Note that these changes are all in a #ifdef __linux__ section.
>
> Signed-off-by: Stefan Hajnoczi <stefa...@linux.vnet.ibm.com>
> ---
> block/raw-posix.c | 25 +++++++++++++++++++++----
> 1 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index a95c8d4..ac95467 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -1240,10 +1240,27 @@ static int cdrom_is_inserted(BlockDriverState *bs)
> BDRVRawState *s = bs->opaque;
> int ret;
>
> - ret = ioctl(s->fd, CDROM_DRIVE_STATUS, CDSL_CURRENT);
> - if (ret == CDS_DISC_OK)
> - return 1;
> - return 0;
> + /*
> + * Opening and closing on each drive status check ensures the medium size
> + * is refreshed. If the file descriptor is kept open the size can become
> + * stale. This is essentially replicating CD-ROM polling but is driven
> by
> + * the guest. As the guest polls, we poll the host.
> + */
Comment confused me :p
we are not opening and closing at each status check.
We are opening if the cdrom is _not_ opened. And we are closing if
there is one error. If comment 2 above is right, you need to do
insteand something like:
if (s->fd != -1) {
close(s->fd);
}
s->fd = open( ....);
That is really reopening all the times.
> +
> + if (s->fd == -1) {
> + s->fd = qemu_open(bs->filename, s->open_flags, 0644);
Everything else on that file uses plain "open" not "qemu_open".
diference is basically that qemu_open() adds flag O_CLOEXEC.
I don't know if this one should be vanilla open or the other ones
qemu_open().
What do you think?
> + if (s->fd < 0) {
> + return 0;
> + }
> + }
> +
> + ret = (ioctl(s->fd, CDROM_DRIVE_STATUS, CDSL_CURRENT) == CDS_DISC_OK);
parens are not needed around ==.
> +
> + if (!ret) {
> + close(s->fd);
> + s->fd = -1;
> + }
> + return ret;
> }
>
> static int cdrom_eject(BlockDriverState *bs, int eject_flag)
Later, Juan.
