Am 29.06.2011 15:36, schrieb Johannes Stezenbach:
> On Linux x86_64 host with 32bit userspace, running
> qemu or even just "qemu-img create -f qcow2 some.img 1G"
> causes a kernel warning:
>
> 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
>
> ioctl 00005326 is CDROM_DRIVE_STATUS,
> ioctl 801c0204 is FDGETPRM.
>
> The warning appears because the Linux compat-ioctl handler for these
> ioctls only applies to block devices, while qemu also uses the ioctls on
> plain files. Work around by calling fstat() the ensure the ioctls are
> only used on block devices.
>
> Signed-off-by: Johannes Stezenbach <js@xxxxxxxxx>
> ---
> discussed in http://lkml.kernel.org/r/20110617090424.GA19345@xxxxxxxxx
>
> I'll also send a Linux kernel patch but this is needed to
> fix the warning on old kernels.
I probably wouldn't have bothered to work around it for older kernels,
but anyway it shouldn't hurt to do so.
Generally the patch looks good to me, just some minor comments:
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 4cd7d7a..10d56f0 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -46,6 +46,8 @@
> #include <sys/dkio.h>
> #endif
> #ifdef __linux__
> +#include <sys/types.h>
> +#include <sys/stat.h>
> #include <sys/ioctl.h>
> #include <sys/param.h>
> #include <linux/cdrom.h>
> @@ -1188,6 +1190,7 @@ static int floppy_probe_device(const char *filename)
> int fd, ret;
> int prio = 0;
> struct floppy_struct fdparam;
> + struct stat st;
>
> if (strstart(filename, "/dev/fd", NULL))
> prio = 50;
> @@ -1196,12 +1199,20 @@ static int floppy_probe_device(const char *filename)
> if (fd < 0) {
> goto out;
> }
> + ret = fstat(fd, &st);
> + if (ret == -1) {
> + goto outc;
> + }
> + if (!S_ISBLK(st.st_mode)) {
> + goto outc;
> + }
Why not a single if (ret == -1 || !S_ISBLK(st.st_mode))?
> /* Attempt to detect via a floppy specific ioctl */
> ret = ioctl(fd, FDGETPRM, &fdparam);
> if (ret >= 0)
> prio = 100;
>
> +outc:
> close(fd);
> out:
> return prio;
> @@ -1290,17 +1301,26 @@ static int cdrom_probe_device(const char *filename)
> {
> int fd, ret;
> int prio = 0;
> + struct stat st;
>
> fd = open(filename, O_RDONLY | O_NONBLOCK);
> if (fd < 0) {
> goto out;
> }
> + ret = fstat(fd, &st);
> + if (ret == -1) {
> + goto outc;
> + }
> + if (!S_ISBLK(st.st_mode)) {
> + goto outc;
> + }
Same here.
> /* Attempt to detect via a CDROM specific ioctl */
> ret = ioctl(fd, CDROM_DRIVE_STATUS, CDSL_CURRENT);
> if (ret >= 0)
> prio = 100;
>
> +outc:
> close(fd);
> out:
> return prio;
> @@ -1309,11 +1329,15 @@ out:
> static int cdrom_is_inserted(BlockDriverState *bs)
> {
> BDRVRawState *s = bs->opaque;
> + struct stat st;
> int ret;
>
> - ret = ioctl(s->fd, CDROM_DRIVE_STATUS, CDSL_CURRENT);
> - if (ret == CDS_DISC_OK)
> - return 1;
> + ret = fstat(s->fd, &st);
> + if (!ret && S_ISBLK(st.st_mode)) {
> + ret = ioctl(s->fd, CDROM_DRIVE_STATUS, CDSL_CURRENT);
> + if (ret == CDS_DISC_OK)
> + return 1;
> + }
> return 0;
> }
I think this last hunk is unnecessary. cdrom_is_inserted should only be
called if cdrom_probe_device returned success before, so we have already
tested this.
Kevin
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
