Re: [Qemu-devel] [PATCH] Error on O_DIRECT for physical CDROM/DVD drives
On 07/20/10 19:19, Markus Armbruster wrote: > Jes Sorensen writes: > >> On 07/20/10 18:35, David S. Ahern wrote: >>> On 07/20/10 10:09, jes.soren...@redhat.com wrote: >>> If a cdrom is added via the monitor this would abruptly terminate the VM >>> - which is not good. >> >> True, but this would only happen if the user explicitly marks it >> cache=none which is not the default value for a new drive, so it would >> be a corner case. > > Why not return -ENOTSUP? Good point, just posted v3 of the patch that does this instead of exit() Jes
Re: [Qemu-devel] [PATCH] Error on O_DIRECT for physical CDROM/DVD drives
Jes Sorensen writes: > On 07/20/10 18:35, David S. Ahern wrote: >> On 07/20/10 10:09, jes.soren...@redhat.com wrote: >>> diff --git a/block/raw-posix.c b/block/raw-posix.c >>> index 291699f..1b840c4 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); >>> +exit(1); >>> +} >> >> If a cdrom is added via the monitor this would abruptly terminate the VM >> - which is not good. > > True, but this would only happen if the user explicitly marks it > cache=none which is not the default value for a new drive, so it would > be a corner case. Why not return -ENOTSUP?
Re: [Qemu-devel] [PATCH] Error on O_DIRECT for physical CDROM/DVD drives
On 07/20/10 18:35, David S. Ahern wrote: > On 07/20/10 10:09, jes.soren...@redhat.com wrote: >> diff --git a/block/raw-posix.c b/block/raw-posix.c >> index 291699f..1b840c4 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); >> +exit(1); >> +} > > If a cdrom is added via the monitor this would abruptly terminate the VM > - which is not good. True, but this would only happen if the user explicitly marks it cache=none which is not the default value for a new drive, so it would be a corner case. Cheers, Jes
Re: [Qemu-devel] [PATCH] Error on O_DIRECT for physical CDROM/DVD drives
On 07/20/10 10:09, jes.soren...@redhat.com wrote: > From: Jes Sorensen > > 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. Exit with an > error! > > Signed-off-by: Jes Sorensen > --- > 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..1b840c4 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); > +exit(1); > +} If a cdrom is added via the monitor this would abruptly terminate the VM - which is not good. David > > /* open will not fail even if no CD is inserted, so add O_NONBLOCK */ > return raw_open_common(bs, filename, flags, O_NONBLOCK);
[Qemu-devel] [PATCH] Error on O_DIRECT for physical CDROM/DVD drives
From: Jes Sorensen 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. Exit with an error! Signed-off-by: Jes Sorensen --- 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..1b840c4 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); +exit(1); +} /* 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