All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3] Error on O_DIRECT for physical CDROM/DVD drives
@ 2010-07-21  7:45 Jes.Sorensen
  2010-07-21  9:20 ` Markus Armbruster
  2010-07-21 13:15 ` Christoph Hellwig
  0 siblings, 2 replies; 5+ messages in thread
From: Jes.Sorensen @ 2010-07-21  7:45 UTC (permalink / raw)
  To: qemu-devel

From: Jes Sorensen <Jes.Sorensen@redhat.com>

O_DIRECT (cache=none) requires sector alignment, however the physical
sector size of CDROM/DVD drives is 2048, as opposed to most disk
devices which use 512. QEMU is hard coding 512 all over the place, so
allowing O_DIRECT for CDROM/DVD devices does not work.

Return -ENOTSUP from cdrom_open() in this case.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 block/raw-posix.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 291699f..4b84770 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1139,6 +1139,11 @@ static int cdrom_open(BlockDriverState *bs, const char *filename, int flags)
     BDRVRawState *s = bs->opaque;
 
     s->type = FTYPE_CD;
+    if (flags & BDRV_O_NOCACHE) {
+        fprintf(stderr, "O_DIRECT (cache=none) for CDROM/DVD device (%s) "
+                "is unsupported\n", filename);
+        return -ENOTSUP;
+    }
 
     /* open will not fail even if no CD is inserted, so add O_NONBLOCK */
     return raw_open_common(bs, filename, flags, O_NONBLOCK);
-- 
1.7.1.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH v3] Error on O_DIRECT for physical CDROM/DVD drives
  2010-07-21  7:45 [Qemu-devel] [PATCH v3] Error on O_DIRECT for physical CDROM/DVD drives Jes.Sorensen
@ 2010-07-21  9:20 ` Markus Armbruster
  2010-07-21 13:15 ` Christoph Hellwig
  1 sibling, 0 replies; 5+ messages in thread
From: Markus Armbruster @ 2010-07-21  9:20 UTC (permalink / raw)
  To: Jes.Sorensen; +Cc: qemu-devel

Jes.Sorensen@redhat.com writes:

> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> O_DIRECT (cache=none) requires sector alignment, however the physical
> sector size of CDROM/DVD drives is 2048, as opposed to most disk
> devices which use 512. QEMU is hard coding 512 all over the place, so
> allowing O_DIRECT for CDROM/DVD devices does not work.
>
> Return -ENOTSUP from cdrom_open() in this case.

Good short-term fix.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH v3] Error on O_DIRECT for physical CDROM/DVD drives
  2010-07-21  7:45 [Qemu-devel] [PATCH v3] Error on O_DIRECT for physical CDROM/DVD drives Jes.Sorensen
  2010-07-21  9:20 ` Markus Armbruster
@ 2010-07-21 13:15 ` Christoph Hellwig
  2010-07-21 14:13   ` Markus Armbruster
  1 sibling, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2010-07-21 13:15 UTC (permalink / raw)
  To: Jes.Sorensen; +Cc: qemu-devel

On Wed, Jul 21, 2010 at 09:45:19AM +0200, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> O_DIRECT (cache=none) requires sector alignment, however the physical
> sector size of CDROM/DVD drives is 2048, as opposed to most disk
> devices which use 512. QEMU is hard coding 512 all over the place, so
> allowing O_DIRECT for CDROM/DVD devices does not work.
> 
> Return -ENOTSUP from cdrom_open() in this case.

The patch is not quite correct.  There are CDROMs with 512 byte sectors,
just as there are disks with larger sector sizes.  And of course these
limitations also apply when running ontop of filesystems.

So we really need to handle these things better and need to query
the sector size of the device and handle it properly.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH v3] Error on O_DIRECT for physical CDROM/DVD drives
  2010-07-21 13:15 ` Christoph Hellwig
@ 2010-07-21 14:13   ` Markus Armbruster
  2010-07-21 14:25     ` Christoph Hellwig
  0 siblings, 1 reply; 5+ messages in thread
From: Markus Armbruster @ 2010-07-21 14:13 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Jes.Sorensen, qemu-devel

Christoph Hellwig <hch@lst.de> writes:

> On Wed, Jul 21, 2010 at 09:45:19AM +0200, Jes.Sorensen@redhat.com wrote:
>> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>> 
>> O_DIRECT (cache=none) requires sector alignment, however the physical
>> sector size of CDROM/DVD drives is 2048, as opposed to most disk
>> devices which use 512. QEMU is hard coding 512 all over the place, so
>> allowing O_DIRECT for CDROM/DVD devices does not work.
>> 
>> Return -ENOTSUP from cdrom_open() in this case.
>
> The patch is not quite correct.  There are CDROMs with 512 byte sectors,
> just as there are disks with larger sector sizes.  And of course these
> limitations also apply when running ontop of filesystems.
>
> So we really need to handle these things better and need to query
> the sector size of the device and handle it properly.

Agreed, but I figure Jes's fix is the best we can do for .13, and worth
doing for .13.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH v3] Error on O_DIRECT for physical CDROM/DVD drives
  2010-07-21 14:13   ` Markus Armbruster
@ 2010-07-21 14:25     ` Christoph Hellwig
  0 siblings, 0 replies; 5+ messages in thread
From: Christoph Hellwig @ 2010-07-21 14:25 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Jes.Sorensen, Christoph Hellwig, qemu-devel

On Wed, Jul 21, 2010 at 04:13:10PM +0200, Markus Armbruster wrote:
> Agreed, but I figure Jes's fix is the best we can do for .13, and worth
> doing for .13.

As said in the previous mail it's incorrect in many ways.  If at all
you could reject devices and files on filesysystems on device with
sector size > 512 bytes.  But once that infrastructure is there fixing
it for real should be rather trivial.  I'm looking into it now.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2010-07-21 14:25 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-07-21  7:45 [Qemu-devel] [PATCH v3] Error on O_DIRECT for physical CDROM/DVD drives Jes.Sorensen
2010-07-21  9:20 ` Markus Armbruster
2010-07-21 13:15 ` Christoph Hellwig
2010-07-21 14:13   ` Markus Armbruster
2010-07-21 14:25     ` Christoph Hellwig

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.