[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [libvirt] [Qemu-devel] [PATCH v2 3/3] raw-posix: Re-open host CD-ROM
From: |
Daniel P. Berrange |
Subject: |
Re: [libvirt] [Qemu-devel] [PATCH v2 3/3] raw-posix: Re-open host CD-ROM after media change |
Date: |
Mon, 4 Apr 2011 11:47:53 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Sun, Apr 03, 2011 at 07:06:17PM +0100, Stefan Hajnoczi wrote:
> On Sun, Apr 3, 2011 at 2:12 PM, Blue Swirl <address@hidden> wrote:
> > On Sun, Apr 3, 2011 at 2:57 PM, Stefan Hajnoczi <address@hidden> wrote:
> >> On Tue, Mar 29, 2011 at 8:04 PM, Stefan Hajnoczi
> >> <address@hidden> 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 <address@hidden>
> >>> ---
> >>> 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;
> >>> }
> >>>
> >>> static int cdrom_eject(BlockDriverState *bs, int eject_flag)
> >>> --
> >>> 1.7.4.1
> >>>
> >>>
> >>>
> >>
> >> There is an issue with reopening host devices in QEMU when running
> >> under libvirt. It appears that libvirt chowns image files (including
> >> device nodes) so that the launched QEMU process can access them.
> >>
> >> Unfortunately after media change on host devices udev will reset the
> >> ownership of the device node. This causes open(2) to fail with EACCES
> >> since the QEMU process does not have the right uid/gid/groups and
> >> libvirt is unaware that the file's ownership has changed.
> >>
> >> In order for media change to work with Linux host CD-ROM it is
> >> necessary to reopen the file (otherwise the inode size will not
> >> refresh, this is an issue with existing kernels).
> >>
> >> How can libvirt's security model be made to support this case? In
> >> theory udev could be temporarily configured with libvirt permissions
> >> for the CD-ROM device while passed through to the guest, but is that
> >> feasible?
> >
> > How about something like this: Add an explicit reopen method to
> > BlockDriver. Make a special block device for passed file descriptors.
> > Pass descriptors in libvirt for CD-ROMs instead of the device paths.
> > The reopen method for file descriptors should notify libvirt about
> > need to pass a reopened descriptor and then block all accesses until a
> > new descriptor is available. This should also solve your earlier
> > problem.
>
> I'm hoping libvirt's behavior can be made to just work rather than
> adding new features to QEMU. But perhaps passing file descriptors is
> useful for more than just reopening host devices. This would
> basically be a privilege separation model where the QEMU process isn't
> able to open files itself but can request libvirt to open them on its
> behalf.
It is rather frickin' annoying the way udev resets the ownership
when the media merely changes. If it isn't possible to stop udev
doing this, then i think the only practical thing is to use ACLs
instead of user/group ownership. We wanted to switch to ACLs in
libvirt for other reasons already, but it isn't quite as simple
as it sounds[1] so we've not done it just yet.
Daniel
[1] Mostly due to handling upgrades from existing libvirtd while
VMs are running, and coping with filesystems which don't
support ACLs (or have them turned of by mount options)
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
- Re: [Qemu-devel] [PATCH v2 3/3] raw-posix: Re-open host CD-ROM after media change, Stefan Hajnoczi, 2011/04/03
- Re: [Qemu-devel] [PATCH v2 3/3] raw-posix: Re-open host CD-ROM after media change, Blue Swirl, 2011/04/03
- Re: [Qemu-devel] [PATCH v2 3/3] raw-posix: Re-open host CD-ROM after media change, Stefan Hajnoczi, 2011/04/03
- Re: [libvirt] [Qemu-devel] [PATCH v2 3/3] raw-posix: Re-open host CD-ROM after media change,
Daniel P. Berrange <=
- Re: [libvirt] [Qemu-devel] [PATCH v2 3/3] raw-posix: Re-open host CD-ROM after media change, Stefan Hajnoczi, 2011/04/04
- Re: [libvirt] [Qemu-devel] [PATCH v2 3/3] raw-posix: Re-open host CD-ROM after media change, Anthony Liguori, 2011/04/04
- Re: [libvirt] [Qemu-devel] [PATCH v2 3/3] raw-posix: Re-open host CD-ROM after media change, Daniel P. Berrange, 2011/04/04
- Re: [libvirt] [Qemu-devel] [PATCH v2 3/3] raw-posix: Re-open host CD-ROM after media change, Anthony Liguori, 2011/04/04
- Re: [libvirt] [Qemu-devel] [PATCH v2 3/3] raw-posix: Re-open host CD-ROM after media change, Daniel P. Berrange, 2011/04/04
- Re: [libvirt] [Qemu-devel] [PATCH v2 3/3] raw-posix: Re-open host CD-ROM after media change, Anthony Liguori, 2011/04/04
- Re: [libvirt] [Qemu-devel] [PATCH v2 3/3] raw-posix: Re-open host CD-ROM after media change, Blue Swirl, 2011/04/04
Re: [Qemu-devel] [PATCH v2 3/3] raw-posix: Re-open host CD-ROM after media change, Avi Kivity, 2011/04/04