[Qemu-devel] [PATCH 0/3] Even more read-write/read-only fixes and cleanup

[Qemu-devel] [PATCH 0/3] Even more read-write/read-only fixes and cleanup

[Naphtali Sprei]

A comment from Christoph Hellwig made me check (and fix) some stuff.
I'm afraid there are some more callers that needs to explicitly ask for read-write permissions.
I'd like to get comments about that.

P.S. The qcow2 bug (I introduced) in pre-allocate showed me there's a need to check return value.

Naphtali Sprei (3):
  No need anymore for bdrv_set_read_only
  Ask for read-write permissions when opening files where needed
  Read-only device changed to opens it's file for read-only.

 block.c           |    7 -------
 block.h           |    1 -
 block/bochs.c     |    6 ++----
 block/parallels.c |    6 ++----
 block/qcow2.c     |    2 +-
 block/vvfat.c     |    2 +-
 qemu-img.c        |    4 ++--
 7 files changed, 8 insertions(+), 20 deletions(-)

[Qemu-devel] [PATCH 0/3] *** SUBJECT HERE ***

[Naphtali Sprei]

*** BLURB HERE ***

Naphtali Sprei (3):
  No need anymoe for bdrv_set_read_only
  Ask for read-write permissions when opening files
  Read-only device changed to opens it's file for read-only.

 block.c           |    7 -------
 block.h           |    1 -
 block/bochs.c     |    6 ++----
 block/parallels.c |    6 ++----
 block/qcow2.c     |    2 +-
 block/vvfat.c     |    2 +-
 qemu-img.c        |    4 ++--
 7 files changed, 8 insertions(+), 20 deletions(-)

[Qemu-devel] [PATCH 0/3] *** SUBJECT HERE ***

[Naphtali Sprei]

*** BLURB HERE ***

Naphtali Sprei (3):
  No need anymore for bdrv_set_read_only
  Ask for read-write permissions when opening files where needed
  Read-only device changed to opens it's file for read-only.

 block.c           |    7 -------
 block.h           |    1 -
 block/bochs.c     |    6 ++----
 block/parallels.c |    6 ++----
 block/qcow2.c     |    2 +-
 block/vvfat.c     |    2 +-
 qemu-img.c        |    4 ++--
 7 files changed, 8 insertions(+), 20 deletions(-)

[Qemu-devel] [PATCH 1/3] No need anymoe for bdrv_set_read_only

[Naphtali Sprei]

Signed-off-by: Naphtali Sprei <nsprei@redhat.com>
---
 block.c |    7 -------
 block.h |    1 -
 2 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/block.c b/block.c
index 73c26ec..8378c18 100644
--- a/block.c
+++ b/block.c
@@ -1009,13 +1009,6 @@ int bdrv_is_read_only(BlockDriverState *bs)
     return bs->read_only;
 }
 
-int bdrv_set_read_only(BlockDriverState *bs, int read_only)
-{
-    int ret = bs->read_only;
-    bs->read_only = read_only;
-    return ret;
-}
-
 int bdrv_is_sg(BlockDriverState *bs)
 {
     return bs->sg;
diff --git a/block.h b/block.h
index fd4e8dd..1aec453 100644
--- a/block.h
+++ b/block.h
@@ -147,7 +147,6 @@ int bdrv_get_type_hint(BlockDriverState *bs);
 int bdrv_get_translation_hint(BlockDriverState *bs);
 int bdrv_is_removable(BlockDriverState *bs);
 int bdrv_is_read_only(BlockDriverState *bs);
-int bdrv_set_read_only(BlockDriverState *bs, int read_only);
 int bdrv_is_sg(BlockDriverState *bs);
 int bdrv_enable_write_cache(BlockDriverState *bs);
 int bdrv_is_inserted(BlockDriverState *bs);
-- 
1.6.3.3

[Qemu-devel] [PATCH 1/3] No need anymore for bdrv_set_read_only

[Naphtali Sprei]

Signed-off-by: Naphtali Sprei <nsprei@redhat.com>
---
 block.c |    7 -------
 block.h |    1 -
 2 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/block.c b/block.c
index 73c26ec..8378c18 100644
--- a/block.c
+++ b/block.c
@@ -1009,13 +1009,6 @@ int bdrv_is_read_only(BlockDriverState *bs)
     return bs->read_only;
 }
 
-int bdrv_set_read_only(BlockDriverState *bs, int read_only)
-{
-    int ret = bs->read_only;
-    bs->read_only = read_only;
-    return ret;
-}
-
 int bdrv_is_sg(BlockDriverState *bs)
 {
     return bs->sg;
diff --git a/block.h b/block.h
index fd4e8dd..1aec453 100644
--- a/block.h
+++ b/block.h
@@ -147,7 +147,6 @@ int bdrv_get_type_hint(BlockDriverState *bs);
 int bdrv_get_translation_hint(BlockDriverState *bs);
 int bdrv_is_removable(BlockDriverState *bs);
 int bdrv_is_read_only(BlockDriverState *bs);
-int bdrv_set_read_only(BlockDriverState *bs, int read_only);
 int bdrv_is_sg(BlockDriverState *bs);
 int bdrv_enable_write_cache(BlockDriverState *bs);
 int bdrv_is_inserted(BlockDriverState *bs);
-- 
1.6.3.3

[Qemu-devel] [PATCH 2/3] Ask for read-write permissions when opening files

[Naphtali Sprei]

Found some places that seems needs this explicitly, now that
read-write is not the default.

Signed-off-by: Naphtali Sprei <nsprei@redhat.com>
---
 block/qcow2.c |    2 +-
 block/vvfat.c |    2 +-
 qemu-img.c    |    4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 6622eba..91835f1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -961,7 +961,7 @@ static int qcow_create2(const char *filename, int64_t total_size,
     if (prealloc) {
         BlockDriverState *bs;
         bs = bdrv_new("");
-        bdrv_open(bs, filename, BDRV_O_CACHE_WB);
+        bdrv_open(bs, filename, BDRV_O_CACHE_WB | BDRV_O_RDWR);
         preallocate(bs);
         bdrv_close(bs);
     }
diff --git a/block/vvfat.c b/block/vvfat.c
index 063f731..8e12f92 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2787,7 +2787,7 @@ static int enable_write_target(BDRVVVFATState *s)
     if (bdrv_create(bdrv_qcow, s->qcow_filename, options) < 0)
 	return -1;
     s->qcow = bdrv_new("");
-    if (s->qcow == NULL || bdrv_open(s->qcow, s->qcow_filename, 0) < 0)
+    if (s->qcow == NULL || bdrv_open(s->qcow, s->qcow_filename, BDRV_O_RDWR) < 0)
 	return -1;
 
 #ifndef _WIN32
diff --git a/qemu-img.c b/qemu-img.c
index 3cea8ce..cbba4fc 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1116,7 +1116,7 @@ static int img_rebase(int argc, char **argv)
     if (!bs)
         error("Not enough memory");
 
-    flags = BRDV_O_FLAGS | (unsafe ? BDRV_O_NO_BACKING : 0);
+    flags = BRDV_O_FLAGS | BDRV_O_RDWR | (unsafe ? BDRV_O_NO_BACKING : 0);
     if (bdrv_open2(bs, filename, flags, NULL) < 0) {
         error("Could not open '%s'", filename);
     }
@@ -1157,7 +1157,7 @@ static int img_rebase(int argc, char **argv)
         }
 
         bs_new_backing = bdrv_new("new_backing");
-        if (bdrv_open2(bs_new_backing, out_baseimg, BRDV_O_FLAGS,
+        if (bdrv_open2(bs_new_backing, out_baseimg, BRDV_O_FLAGS | BDRV_O_RDWR,
             new_backing_drv))
         {
             error("Could not open new backing file '%s'", backing_name);
-- 
1.6.3.3

[Qemu-devel] [PATCH 2/3] Ask for read-write permissions when opening files where needed

[Naphtali Sprei]

Found some places that seems needs this explicitly, now that
read-write is not the default.

Signed-off-by: Naphtali Sprei <nsprei@redhat.com>
---
 block/qcow2.c |    2 +-
 block/vvfat.c |    2 +-
 qemu-img.c    |    4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 6622eba..91835f1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -961,7 +961,7 @@ static int qcow_create2(const char *filename, int64_t total_size,
     if (prealloc) {
         BlockDriverState *bs;
         bs = bdrv_new("");
-        bdrv_open(bs, filename, BDRV_O_CACHE_WB);
+        bdrv_open(bs, filename, BDRV_O_CACHE_WB | BDRV_O_RDWR);
         preallocate(bs);
         bdrv_close(bs);
     }
diff --git a/block/vvfat.c b/block/vvfat.c
index 063f731..8e12f92 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2787,7 +2787,7 @@ static int enable_write_target(BDRVVVFATState *s)
     if (bdrv_create(bdrv_qcow, s->qcow_filename, options) < 0)
 	return -1;
     s->qcow = bdrv_new("");
-    if (s->qcow == NULL || bdrv_open(s->qcow, s->qcow_filename, 0) < 0)
+    if (s->qcow == NULL || bdrv_open(s->qcow, s->qcow_filename, BDRV_O_RDWR) < 0)
 	return -1;
 
 #ifndef _WIN32
diff --git a/qemu-img.c b/qemu-img.c
index 3cea8ce..cbba4fc 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1116,7 +1116,7 @@ static int img_rebase(int argc, char **argv)
     if (!bs)
         error("Not enough memory");
 
-    flags = BRDV_O_FLAGS | (unsafe ? BDRV_O_NO_BACKING : 0);
+    flags = BRDV_O_FLAGS | BDRV_O_RDWR | (unsafe ? BDRV_O_NO_BACKING : 0);
     if (bdrv_open2(bs, filename, flags, NULL) < 0) {
         error("Could not open '%s'", filename);
     }
@@ -1157,7 +1157,7 @@ static int img_rebase(int argc, char **argv)
         }
 
         bs_new_backing = bdrv_new("new_backing");
-        if (bdrv_open2(bs_new_backing, out_baseimg, BRDV_O_FLAGS,
+        if (bdrv_open2(bs_new_backing, out_baseimg, BRDV_O_FLAGS | BDRV_O_RDWR,
             new_backing_drv))
         {
             error("Could not open new backing file '%s'", backing_name);
-- 
1.6.3.3

[Qemu-devel] [PATCH 3/3] Read-only device changed to opens it's file for read-only.

[Naphtali Sprei]

Signed-off-by: Naphtali Sprei <nsprei@redhat.com>
---
 block/bochs.c     |    6 ++----
 block/parallels.c |    6 ++----
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/block/bochs.c b/block/bochs.c
index 3489258..fb83594 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -116,11 +116,9 @@ static int bochs_open(BlockDriverState *bs, const char *filename, int flags)
     struct bochs_header bochs;
     struct bochs_header_v1 header_v1;
 
-    fd = open(filename, O_RDWR | O_BINARY);
+    fd = open(filename, O_RDONLY | O_BINARY);
     if (fd < 0) {
-        fd = open(filename, O_RDONLY | O_BINARY);
-        if (fd < 0)
-            return -1;
+        return -1;
     }
 
     bs->read_only = 1; // no write support yet
diff --git a/block/parallels.c b/block/parallels.c
index 63b6738..41b3a7c 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -74,11 +74,9 @@ static int parallels_open(BlockDriverState *bs, const char *filename, int flags)
     int fd, i;
     struct parallels_header ph;
 
-    fd = open(filename, O_RDWR | O_BINARY | O_LARGEFILE);
+    fd = open(filename, O_RDONLY | O_BINARY | O_LARGEFILE);
     if (fd < 0) {
-        fd = open(filename, O_RDONLY | O_BINARY | O_LARGEFILE);
-        if (fd < 0)
-            return -1;
+        return -1;
     }
 
     bs->read_only = 1; // no write support yet
-- 
1.6.3.3

Re: [Qemu-devel] [PATCH 1/3] No need anymoe for bdrv_set_read_only

[Anthony Liguori]

On 01/21/2010 06:40 AM, Naphtali Sprei wrote:
> Signed-off-by: Naphtali Sprei<nsprei@redhat.com>
>    

Applied all.  Thanks.

Regards,

Anthony Liguori

> ---
>   block.c |    7 -------
>   block.h |    1 -
>   2 files changed, 0 insertions(+), 8 deletions(-)
>
> diff --git a/block.c b/block.c
> index 73c26ec..8378c18 100644
> --- a/block.c
> +++ b/block.c
> @@ -1009,13 +1009,6 @@ int bdrv_is_read_only(BlockDriverState *bs)
>       return bs->read_only;
>   }
>
> -int bdrv_set_read_only(BlockDriverState *bs, int read_only)
> -{
> -    int ret = bs->read_only;
> -    bs->read_only = read_only;
> -    return ret;
> -}
> -
>   int bdrv_is_sg(BlockDriverState *bs)
>   {
>       return bs->sg;
> diff --git a/block.h b/block.h
> index fd4e8dd..1aec453 100644
> --- a/block.h
> +++ b/block.h
> @@ -147,7 +147,6 @@ int bdrv_get_type_hint(BlockDriverState *bs);
>   int bdrv_get_translation_hint(BlockDriverState *bs);
>   int bdrv_is_removable(BlockDriverState *bs);
>   int bdrv_is_read_only(BlockDriverState *bs);
> -int bdrv_set_read_only(BlockDriverState *bs, int read_only);
>   int bdrv_is_sg(BlockDriverState *bs);
>   int bdrv_enable_write_cache(BlockDriverState *bs);
>   int bdrv_is_inserted(BlockDriverState *bs);
>