From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=55445 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q6Rgp-0001Hg-W3 for qemu-devel@nongnu.org; Sun, 03 Apr 2011 14:06:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q6Rgo-0000Kg-S0 for qemu-devel@nongnu.org; Sun, 03 Apr 2011 14:06:20 -0400 Received: from mail-yx0-f173.google.com ([209.85.213.173]:46737) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q6Rgo-0000KI-OM for qemu-devel@nongnu.org; Sun, 03 Apr 2011 14:06:18 -0400 Received: by yxk8 with SMTP id 8so2380072yxk.4 for ; Sun, 03 Apr 2011 11:06:17 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <1301425482-8722-1-git-send-email-stefanha@linux.vnet.ibm.com> <1301425482-8722-4-git-send-email-stefanha@linux.vnet.ibm.com> Date: Sun, 3 Apr 2011 19:06:17 +0100 Message-ID: Subject: Re: [Qemu-devel] [PATCH v2 3/3] raw-posix: Re-open host CD-ROM after media change From: Stefan Hajnoczi Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: Kevin Wolf , Anthony Liguori , Stefan Hajnoczi , Juan Quintela , libvir-list@redhat.com, qemu-devel@nongnu.org, Ryan Harper , Amit Shah On Sun, Apr 3, 2011 at 2:12 PM, Blue Swirl wrote: > On Sun, Apr 3, 2011 at 2:57 PM, Stefan Hajnoczi wrot= e: >> On Tue, Mar 29, 2011 at 8:04 PM, Stefan Hajnoczi >> wrote: >>> Piggy-back on the guest CD-ROM polling to poll on the host. =A0Open and >>> close the host CD-ROM file descriptor to ensure we read the new size an= d >>> not a stale size. >>> >>> Two things are going on here: >>> >>> 1. If hald/udisks is not already polling CD-ROMs on the host then >>> =A0 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 >>> =A0 be re-opened in order for lseek(2) to see the new size. =A0The >>> =A0 inode size gets out of sync with the underlying device (which you c= an >>> =A0 confirm by checking that /sys/block/sr0/size and lseek(2) do not >>> =A0 match after media change). =A0I have raised this with the >>> =A0 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 >>> --- >>> =A0block/raw-posix.c | =A0 26 ++++++++++++++++++++++---- >>> =A01 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) >>> =A0 =A0 BDRVRawState *s =3D bs->opaque; >>> =A0 =A0 int ret; >>> >>> - =A0 =A0ret =3D ioctl(s->fd, CDROM_DRIVE_STATUS, CDSL_CURRENT); >>> - =A0 =A0if (ret =3D=3D CDS_DISC_OK) >>> - =A0 =A0 =A0 =A0return 1; >>> - =A0 =A0return 0; >>> + =A0 =A0/* >>> + =A0 =A0 * Close the file descriptor if no medium is present and open = it to poll >>> + =A0 =A0 * again. =A0This ensures the medium size is refreshed. =A0If = the file >>> + =A0 =A0 * descriptor is kept open the size can become stale. =A0This = is essentially >>> + =A0 =A0 * replicating CD-ROM polling but is driven by the guest. =A0A= s the guest >>> + =A0 =A0 * polls, we poll the host. >>> + =A0 =A0 */ >>> + >>> + =A0 =A0if (s->fd =3D=3D -1) { >>> + =A0 =A0 =A0 =A0s->fd =3D qemu_open(bs->filename, s->open_flags, 0644)= ; >>> + =A0 =A0 =A0 =A0if (s->fd < 0) { >>> + =A0 =A0 =A0 =A0 =A0 =A0return 0; >>> + =A0 =A0 =A0 =A0} >>> + =A0 =A0} >>> + >>> + =A0 =A0ret =3D (ioctl(s->fd, CDROM_DRIVE_STATUS, CDSL_CURRENT) =3D=3D= CDS_DISC_OK); >>> + >>> + =A0 =A0if (!ret) { >>> + =A0 =A0 =A0 =A0close(s->fd); >>> + =A0 =A0 =A0 =A0s->fd =3D -1; >>> + =A0 =A0} >>> + =A0 =A0return ret; >>> =A0} >>> >>> =A0static 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. =A0It 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. =A0This 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? =A0In >> 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. Stefan