* [Qemu-devel] [PATCH 2/4] raw-posix: add a raw_open_common helper
@ 2009-05-25 7:59 Christoph Hellwig
2009-06-09 19:54 ` Glauber Costa
0 siblings, 1 reply; 3+ messages in thread
From: Christoph Hellwig @ 2009-05-25 7:59 UTC (permalink / raw)
To: qemu-devel
raw_open and hdev_open contain the same basic logic. Add a new
raw_open_common helper containing the guts of the open routine
and call it from raw_open and hdev_open.
We use the new open_flags field in BDRVRawState to allow passing
additional open flags to raw_open_common from both.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: qemu/block/raw-posix.c
===================================================================
--- qemu.orig/block/raw-posix.c 2009-05-25 09:20:13.949939473 +0200
+++ qemu/block/raw-posix.c 2009-05-25 09:21:56.307963685 +0200
@@ -124,7 +124,8 @@ static int cd_open(BlockDriverState *bs)
static int raw_is_inserted(BlockDriverState *bs);
-static int raw_open(BlockDriverState *bs, const char *filename, int flags)
+static int raw_open_common(BlockDriverState *bs, const char *filename,
+ int flags)
{
BDRVRawState *s = bs->opaque;
int fd, ret;
@@ -140,8 +141,6 @@ static int raw_open(BlockDriverState *bs
s->open_flags |= O_RDONLY;
bs->read_only = 1;
}
- if (flags & BDRV_O_CREAT)
- s->open_flags |= O_CREAT | O_TRUNC;
/* Use O_DSYNC for write-through caching, no flags for write-back caching,
* and O_DIRECT for no caching. */
@@ -150,8 +149,7 @@ static int raw_open(BlockDriverState *bs
else if (!(flags & BDRV_O_CACHE_WB))
s->open_flags |= O_DSYNC;
- s->type = FTYPE_FILE;
-
+ s->fd = -1;
fd = open(filename, s->open_flags, 0644);
if (fd < 0) {
ret = -errno;
@@ -172,6 +170,17 @@ static int raw_open(BlockDriverState *bs
return 0;
}
+static int raw_open(BlockDriverState *bs, const char *filename, int flags)
+{
+ BDRVRawState *s = bs->opaque;
+
+ s->type = FTYPE_FILE;
+ if (flags & BDRV_O_CREAT)
+ s->open_flags |= O_CREAT | O_TRUNC;
+
+ return raw_open_common(bs, filename, flags);
+}
+
/* XXX: use host sector size if necessary with:
#ifdef DIOCGSECTORSIZE
{
@@ -942,9 +951,7 @@ kern_return_t GetBSDPath( io_iterator_t
static int hdev_open(BlockDriverState *bs, const char *filename, int flags)
{
BDRVRawState *s = bs->opaque;
- int fd, ret;
-
- posix_aio_init();
+ int ret;
#ifdef CONFIG_COCOA
if (strstart(filename, "/dev/cdrom", NULL)) {
@@ -972,19 +979,6 @@ static int hdev_open(BlockDriverState *b
IOObjectRelease( mediaIterator );
}
#endif
- s->open_flags |= O_BINARY;
- if ((flags & BDRV_O_ACCESS) == O_RDWR) {
- s->open_flags |= O_RDWR;
- } else {
- s->open_flags |= O_RDONLY;
- bs->read_only = 1;
- }
- /* Use O_DSYNC for write-through caching, no flags for write-back caching,
- * and O_DIRECT for no caching. */
- if ((flags & BDRV_O_NOCACHE))
- s->open_flags |= O_DIRECT;
- else if (!(flags & BDRV_O_CACHE_WB))
- s->open_flags |= O_DSYNC;
s->type = FTYPE_FILE;
#if defined(__linux__)
@@ -1008,15 +1002,11 @@ static int hdev_open(BlockDriverState *b
s->type = FTYPE_CD;
}
#endif
- s->fd = -1;
- fd = open(filename, s->open_flags, 0644);
- if (fd < 0) {
- ret = -errno;
- if (ret == -EROFS)
- ret = -EACCES;
+
+ ret = raw_open_common(bs, filename, flags);
+ if (ret)
return ret;
- }
- s->fd = fd;
+
#if defined(__FreeBSD__)
/* make sure the door isnt locked at this time */
if (s->type == FTYPE_CD)
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH 2/4] raw-posix: add a raw_open_common helper
2009-05-25 7:59 [Qemu-devel] [PATCH 2/4] raw-posix: add a raw_open_common helper Christoph Hellwig
@ 2009-06-09 19:54 ` Glauber Costa
2009-06-11 16:00 ` Christoph Hellwig
0 siblings, 1 reply; 3+ messages in thread
From: Glauber Costa @ 2009-06-09 19:54 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: qemu-devel
On Mon, May 25, 2009 at 09:59:22AM +0200, Christoph Hellwig wrote:
>
> raw_open and hdev_open contain the same basic logic. Add a new
> raw_open_common helper containing the guts of the open routine
> and call it from raw_open and hdev_open.
>
> We use the new open_flags field in BDRVRawState to allow passing
> additional open flags to raw_open_common from both.
>
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
>
> Index: qemu/block/raw-posix.c
> ===================================================================
> --- qemu.orig/block/raw-posix.c 2009-05-25 09:20:13.949939473 +0200
> +++ qemu/block/raw-posix.c 2009-05-25 09:21:56.307963685 +0200
> @@ -124,7 +124,8 @@ static int cd_open(BlockDriverState *bs)
>
> static int raw_is_inserted(BlockDriverState *bs);
>
> -static int raw_open(BlockDriverState *bs, const char *filename, int flags)
> +static int raw_open_common(BlockDriverState *bs, const char *filename,
> + int flags)
> {
> BDRVRawState *s = bs->opaque;
> int fd, ret;
> @@ -140,8 +141,6 @@ static int raw_open(BlockDriverState *bs
> s->open_flags |= O_RDONLY;
> bs->read_only = 1;
> }
> - if (flags & BDRV_O_CREAT)
> - s->open_flags |= O_CREAT | O_TRUNC;
>
> /* Use O_DSYNC for write-through caching, no flags for write-back caching,
> * and O_DIRECT for no caching. */
> @@ -150,8 +149,7 @@ static int raw_open(BlockDriverState *bs
> else if (!(flags & BDRV_O_CACHE_WB))
> s->open_flags |= O_DSYNC;
>
> - s->type = FTYPE_FILE;
> -
> + s->fd = -1;
> fd = open(filename, s->open_flags, 0644);
> if (fd < 0) {
> ret = -errno;
it is not equivalent to the current code, while you claim it is.
this one will return EROFS to the external world, while old code
would return EACCES. If there is any user relying on that return value,
we're screwed. And as a matter of fact, it appears to be (bdrv_open2)
> #ifdef CONFIG_COCOA
> if (strstart(filename, "/dev/cdrom", NULL)) {
> @@ -972,19 +979,6 @@ static int hdev_open(BlockDriverState *b
> IOObjectRelease( mediaIterator );
> }
> #endif
> - s->open_flags |= O_BINARY;
> - if ((flags & BDRV_O_ACCESS) == O_RDWR) {
> - s->open_flags |= O_RDWR;
> - } else {
> - s->open_flags |= O_RDONLY;
> - bs->read_only = 1;
> - }
> - /* Use O_DSYNC for write-through caching, no flags for write-back caching,
> - * and O_DIRECT for no caching. */
> - if ((flags & BDRV_O_NOCACHE))
> - s->open_flags |= O_DIRECT;
> - else if (!(flags & BDRV_O_CACHE_WB))
> - s->open_flags |= O_DSYNC;
>
What happened to those flags? Are you just throwing them away?
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH 2/4] raw-posix: add a raw_open_common helper
2009-06-09 19:54 ` Glauber Costa
@ 2009-06-11 16:00 ` Christoph Hellwig
0 siblings, 0 replies; 3+ messages in thread
From: Christoph Hellwig @ 2009-06-11 16:00 UTC (permalink / raw)
To: Glauber Costa; +Cc: Christoph Hellwig, qemu-devel
On Tue, Jun 09, 2009 at 04:54:49PM -0300, Glauber Costa wrote:
> > -
> > + s->fd = -1;
> > fd = open(filename, s->open_flags, 0644);
> > if (fd < 0) {
> > ret = -errno;
> it is not equivalent to the current code, while you claim it is.
> this one will return EROFS to the external world, while old code
> would return EACCES. If there is any user relying on that return value,
> we're screwed. And as a matter of fact, it appears to be (bdrv_open2)
The code to turn -EROFS into -EACCES is also present in raw_open (and
thus the new raw_open_common), it just didn't make it into the visible
patch hunk.
> > - s->open_flags |= O_BINARY;
> > - if ((flags & BDRV_O_ACCESS) == O_RDWR) {
> > - s->open_flags |= O_RDWR;
> > - } else {
> > - s->open_flags |= O_RDONLY;
> > - bs->read_only = 1;
> > - }
> > - /* Use O_DSYNC for write-through caching, no flags for write-back caching,
> > - * and O_DIRECT for no caching. */
> > - if ((flags & BDRV_O_NOCACHE))
> > - s->open_flags |= O_DIRECT;
> > - else if (!(flags & BDRV_O_CACHE_WB))
> > - s->open_flags |= O_DSYNC;
> >
> What happened to those flags? Are you just throwing them away?
raw_open_common deals with those, we just consolidated those to one
single site.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2009-06-11 16:00 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-05-25 7:59 [Qemu-devel] [PATCH 2/4] raw-posix: add a raw_open_common helper Christoph Hellwig
2009-06-09 19:54 ` Glauber Costa
2009-06-11 16:00 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).