From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=36723 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q6N6Y-0006yX-M5 for qemu-devel@nongnu.org; Sun, 03 Apr 2011 09:12:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q6N6T-0001rM-F5 for qemu-devel@nongnu.org; Sun, 03 Apr 2011 09:12:34 -0400 Received: from mail-vw0-f45.google.com ([209.85.212.45]:51299) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q6N6S-0001rA-Nd for qemu-devel@nongnu.org; Sun, 03 Apr 2011 09:12:29 -0400 Received: by vws17 with SMTP id 17so4176749vws.4 for ; Sun, 03 Apr 2011 06:12:28 -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> From: Blue Swirl Date: Sun, 3 Apr 2011 16:12:08 +0300 Message-ID: Subject: Re: [Qemu-devel] [PATCH v2 3/3] raw-posix: Re-open host CD-ROM after media change Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi 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:57 PM, Stefan Hajnoczi wrote: > 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. =C2=A0Open a= nd >> 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 >> =C2=A0 re-opening the CD-ROM causes the host to read the new medium's si= ze. >> >> 2. There is a bug in Linux which means the CD-ROM file descriptor must >> =C2=A0 be re-opened in order for lseek(2) to see the new size. =C2=A0The >> =C2=A0 inode size gets out of sync with the underlying device (which you= can >> =C2=A0 confirm by checking that /sys/block/sr0/size and lseek(2) do not >> =C2=A0 match after media change). =C2=A0I have raised this with the >> =C2=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 >> --- >> =C2=A0block/raw-posix.c | =C2=A0 26 ++++++++++++++++++++++---- >> =C2=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 *b= s) >> =C2=A0 =C2=A0 BDRVRawState *s =3D bs->opaque; >> =C2=A0 =C2=A0 int ret; >> >> - =C2=A0 =C2=A0ret =3D ioctl(s->fd, CDROM_DRIVE_STATUS, CDSL_CURRENT); >> - =C2=A0 =C2=A0if (ret =3D=3D CDS_DISC_OK) >> - =C2=A0 =C2=A0 =C2=A0 =C2=A0return 1; >> - =C2=A0 =C2=A0return 0; >> + =C2=A0 =C2=A0/* >> + =C2=A0 =C2=A0 * Close the file descriptor if no medium is present and = open it to poll >> + =C2=A0 =C2=A0 * again. =C2=A0This ensures the medium size is refreshed= . =C2=A0If the file >> + =C2=A0 =C2=A0 * descriptor is kept open the size can become stale. =C2= =A0This is essentially >> + =C2=A0 =C2=A0 * replicating CD-ROM polling but is driven by the guest.= =C2=A0As the guest >> + =C2=A0 =C2=A0 * polls, we poll the host. >> + =C2=A0 =C2=A0 */ >> + >> + =C2=A0 =C2=A0if (s->fd =3D=3D -1) { >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0s->fd =3D qemu_open(bs->filename, s->open_f= lags, 0644); >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0if (s->fd < 0) { >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return 0; >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0} >> + =C2=A0 =C2=A0} >> + >> + =C2=A0 =C2=A0ret =3D (ioctl(s->fd, CDROM_DRIVE_STATUS, CDSL_CURRENT) = =3D=3D CDS_DISC_OK); >> + >> + =C2=A0 =C2=A0if (!ret) { >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0close(s->fd); >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0s->fd =3D -1; >> + =C2=A0 =C2=A0} >> + =C2=A0 =C2=A0return ret; >> =C2=A0} >> >> =C2=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. =C2=A0It appears that libvirt chowns image files (includin= g > 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. =C2=A0This causes open(2) to fail with EACC= ES > 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? =C2=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.