2011-06-17 09:26:02

by Johannes Stezenbach

[permalink] [raw]
Subject: missing compat-ioctl for CDROM_DRIVE_STATUS + FDGETPRM

Hi,

running even a simple "qemu-img create -f qcow2 some.img 1G" causes
the following in dmesg on a Linux host with linux-2.6.39.1 x86_64 kernel
and 32bit userspace:

ioctl32(qemu-img:5296): Unknown cmd fd(3) cmd(00005326){t:'S';sz:0} arg(7fffffff) on some.img
ioctl32(qemu-img:5296): Unknown cmd fd(3) cmd(801c0204){t:02;sz:28} arg(fff77350) on some.img

(The same happens when starting a qemu or kvm vm.)

ioctl 00005326 seems to be CDROM_DRIVE_STATUS,
ioctl 801c0204 is FDGETPRM. Both are used in
qemu/block/raw-posix.c in cdrom_probe_device()
and floppy_probe_device() respectively.

FWIW, I'm using qemu/kvm from Debian unstable
(qemu-0.14.0+dfsg-5.1, qemu-kvm-0.14.1+dfsg-1)


Johannes


2011-06-17 13:02:59

by Arnd Bergmann

[permalink] [raw]
Subject: Re: missing compat-ioctl for CDROM_DRIVE_STATUS + FDGETPRM

On Friday 17 June 2011 11:04:24 Johannes Stezenbach wrote:
> running even a simple "qemu-img create -f qcow2 some.img 1G" causes
> the following in dmesg on a Linux host with linux-2.6.39.1 x86_64 kernel
> and 32bit userspace:
>
> ioctl32(qemu-img:5296): Unknown cmd fd(3) cmd(00005326){t:'S';sz:0} arg(7fffffff) on some.img
> ioctl32(qemu-img:5296): Unknown cmd fd(3) cmd(801c0204){t:02;sz:28} arg(fff77350) on some.img
>
> (The same happens when starting a qemu or kvm vm.)
>
> ioctl 00005326 seems to be CDROM_DRIVE_STATUS,
> ioctl 801c0204 is FDGETPRM. Both are used in
> qemu/block/raw-posix.c in cdrom_probe_device()
> and floppy_probe_device() respectively.
>
> FWIW, I'm using qemu/kvm from Debian unstable
> (qemu-0.14.0+dfsg-5.1, qemu-kvm-0.14.1+dfsg-1)

Both are handled by the kernel for block devices, but not for regular
files. The messages may be annoying but they are harmless. We could
silence them either by checking if the file is actually a block device in
qemu-img, or by adding a nop handler to the kernel for regular files.

Arnd

2011-06-29 11:38:45

by Johannes Stezenbach

[permalink] [raw]
Subject: Re: missing compat-ioctl for CDROM_DRIVE_STATUS + FDGETPRM

On Fri, Jun 17, 2011 at 03:02:39PM +0200, Arnd Bergmann wrote:
> On Friday 17 June 2011 11:04:24 Johannes Stezenbach wrote:
> > running even a simple "qemu-img create -f qcow2 some.img 1G" causes
> > the following in dmesg on a Linux host with linux-2.6.39.1 x86_64 kernel
> > and 32bit userspace:
> >
> > ioctl32(qemu-img:5296): Unknown cmd fd(3) cmd(00005326){t:'S';sz:0} arg(7fffffff) on some.img
> > ioctl32(qemu-img:5296): Unknown cmd fd(3) cmd(801c0204){t:02;sz:28} arg(fff77350) on some.img
> >
> > (The same happens when starting a qemu or kvm vm.)
> >
> > ioctl 00005326 seems to be CDROM_DRIVE_STATUS,
> > ioctl 801c0204 is FDGETPRM. Both are used in
> > qemu/block/raw-posix.c in cdrom_probe_device()
> > and floppy_probe_device() respectively.
> >
> > FWIW, I'm using qemu/kvm from Debian unstable
> > (qemu-0.14.0+dfsg-5.1, qemu-kvm-0.14.1+dfsg-1)
>
> Both are handled by the kernel for block devices, but not for regular
> files. The messages may be annoying but they are harmless. We could
> silence them either by checking if the file is actually a block device in
> qemu-img, or by adding a nop handler to the kernel for regular files.

Sorry for very slow reply. I think qemu's use of these ioctls
to probe if the device is a cdrom or floppy is valid, so instead
of adding a stat() call to check for block device in qemu, I think
it is better to silence the warning in the kernel.

Do I get it right that just adding two IGNORE_IOCTL() to
the ioctl_pointer array in linux/fs/compat_ioctl.c is sufficient,
like in commit 3f001711?
I.e. these ioctls are handled for block devices earlier
in compat_sys_ioctl()?


Thanks,
Johannes

2011-06-29 12:30:48

by Arnd Bergmann

[permalink] [raw]
Subject: Re: missing compat-ioctl for CDROM_DRIVE_STATUS + FDGETPRM

On Wednesday 29 June 2011, Johannes Stezenbach wrote:
> Sorry for very slow reply. I think qemu's use of these ioctls
> to probe if the device is a cdrom or floppy is valid, so instead
> of adding a stat() call to check for block device in qemu, I think
> it is better to silence the warning in the kernel.

Well, correct or not, it will keep causing annoying messages on
old kernels, so why not do both?

> Do I get it right that just adding two IGNORE_IOCTL() to
> the ioctl_pointer array in linux/fs/compat_ioctl.c is sufficient,
> like in commit 3f001711?

Yes.

> I.e. these ioctls are handled for block devices earlier
> in compat_sys_ioctl()?

Right. We always first ask the driver, and then search the lookup table
when the driver does not know how to handle it in compat mode.

Arnd

2011-06-29 12:51:27

by Johannes Stezenbach

[permalink] [raw]
Subject: Re: missing compat-ioctl for CDROM_DRIVE_STATUS + FDGETPRM

On Wed, Jun 29, 2011 at 02:30:42PM +0200, Arnd Bergmann wrote:
> On Wednesday 29 June 2011, Johannes Stezenbach wrote:
> > Sorry for very slow reply. I think qemu's use of these ioctls
> > to probe if the device is a cdrom or floppy is valid, so instead
> > of adding a stat() call to check for block device in qemu, I think
> > it is better to silence the warning in the kernel.
>
> Well, correct or not, it will keep causing annoying messages on
> old kernels, so why not do both?

OK, I'll send a patch for qemu, too.

> > Do I get it right that just adding two IGNORE_IOCTL() to
> > the ioctl_pointer array in linux/fs/compat_ioctl.c is sufficient,
> > like in commit 3f001711?
>
> Yes.
>
> > I.e. these ioctls are handled for block devices earlier
> > in compat_sys_ioctl()?
>
> Right. We always first ask the driver, and then search the lookup table
> when the driver does not know how to handle it in compat mode.

Good, I'll send a patch soon.


Thanks,
Johannes

2011-06-29 15:09:19

by Johannes Stezenbach

[permalink] [raw]
Subject: Re: missing compat-ioctl for CDROM_DRIVE_STATUS + FDGETPRM

On Wed, Jun 29, 2011 at 02:51:17PM +0200, Johannes Stezenbach wrote:
> On Wed, Jun 29, 2011 at 02:30:42PM +0200, Arnd Bergmann wrote:
> > On Wednesday 29 June 2011, Johannes Stezenbach wrote:
> > > Do I get it right that just adding two IGNORE_IOCTL() to
> > > the ioctl_pointer array in linux/fs/compat_ioctl.c is sufficient,
> > > like in commit 3f001711?
> >
> > Yes.

Hm, it only works for CDROM_DRIVE_STATUS, but I still get
the warning for FDGETPRM (801c0204). Could you give
me a hint how to address this?

(tested with both 2.6.39.2 and 3.0-rc5-63-g0d72c6f)

diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index 61abb63..ae24aba 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -68,6 +68,8 @@

#ifdef CONFIG_BLOCK
#include <linux/loop.h>
+#include <linux/cdrom.h>
+#include <linux/fd.h>
#include <scsi/scsi.h>
#include <scsi/scsi_ioctl.h>
#include <scsi/sg.h>
@@ -944,6 +946,9 @@ COMPATIBLE_IOCTL(FIOQSIZE)
IGNORE_IOCTL(LOOP_CLR_FD)
/* md calls this on random blockdevs */
IGNORE_IOCTL(RAID_VERSION)
+/* qemu/qemu-img might calls these two on plain files for probing */
+IGNORE_IOCTL(CDROM_DRIVE_STATUS)
+IGNORE_IOCTL(FDGETPRM)
/* SG stuff */
COMPATIBLE_IOCTL(SG_SET_TIMEOUT)
COMPATIBLE_IOCTL(SG_GET_TIMEOUT)


Thanks,
Johannes

2011-06-29 15:37:00

by Arnd Bergmann

[permalink] [raw]
Subject: Re: missing compat-ioctl for CDROM_DRIVE_STATUS + FDGETPRM

On Wednesday 29 June 2011, Johannes Stezenbach wrote:
> On Wed, Jun 29, 2011 at 02:51:17PM +0200, Johannes Stezenbach wrote:
> > On Wed, Jun 29, 2011 at 02:30:42PM +0200, Arnd Bergmann wrote:
> > > On Wednesday 29 June 2011, Johannes Stezenbach wrote:
> > > > Do I get it right that just adding two IGNORE_IOCTL() to
> > > > the ioctl_pointer array in linux/fs/compat_ioctl.c is sufficient,
> > > > like in commit 3f001711?
> > >
> > > Yes.
>
> Hm, it only works for CDROM_DRIVE_STATUS, but I still get
> the warning for FDGETPRM (801c0204). Could you give
> me a hint how to address this?
>
> (tested with both 2.6.39.2 and 3.0-rc5-63-g0d72c6f)

FDGETPRM is incompatible in multiple ways: it uses a different
structure and a different command number on 32 bits, because
the structure has a different size.

You will actually need to add FDGETPRM32 to fs/compat_ioctl.h.
This is locally defined in block/compat_ioctl.c, so you may have
to move the definition to include/linux/fd.h.

Arnd