Am 29.03.2011 21:04, schrieb Stefan Hajnoczi: > 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>
Applied the first two patches of the series. I have some comments on this one. > --- > block/raw-posix.c | 26 ++++++++++++++++++++++---- > 1 files changed, 22 insertions(+), 4 deletions(-) > > diff --git a/block/raw-posix.c b/block/raw-posix.c > index 6b72470..8b5205c 100644 > --- a/block/raw-posix.c > +++ b/block/raw-posix.c > @@ -1238,10 +1238,28 @@ 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; > + /* > + * Close the file descriptor if no medium is present and open it to poll > + * again. This 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. > + */ > + > + if (s->fd == -1) { > + s->fd = qemu_open(bs->filename, s->open_flags, 0644); > + if (s->fd < 0) { > + return 0; > + } > + } > + > + ret = (ioctl(s->fd, CDROM_DRIVE_STATUS, CDSL_CURRENT) == CDS_DISC_OK); > + > + if (!ret) { > + close(s->fd); > + s->fd = -1; > + } > + return ret; > } We have this code in raw_close: if (s->fd >= 0) { close(s->fd); s->fd = -1; if (s->aligned_buf != NULL) qemu_vfree(s->aligned_buf); } So now that we set s->fd = -1, this part won't be run and we leak s->aligned_buf. This problem exists in other places in raw-posix, too. The other thing is that I'm not sure if everything in raw-posix is prepared to deal with a -1 fd. At the very least, I think we'll get -EBADF errors instead of the expected -ENOMEDIUM. The general approach looks good to me. Kevin