Re: [Qemu-devel] [PATCH] Fix qemu-img can't create qcow image based on read-only image

2010-01-28 Thread Kevin Wolf 
Am 28.01.2010 06:22, schrieb Sheng Yang:
 Commit 03cbdac7 Disable fall-back to read-only when cannot open drive's
 file for read-write result in read-only image can't be used as backed
 image in qemu-img.
 
 CC: Naphtali Sprei nsp...@redhat.com
 Signed-off-by: Sheng Yang sh...@linux.intel.com
 ---
 
 This issue blocked our QA's KVM nightly test. But in fact, I don't like this
 patch, feeling uncomfortable to change long existed interface... Any
 alternative? Add a readonly command line would change the default behavior(I
 don't think fall back to readonly looks like a bug); or even revert the
 commit? What's the story behind it?
 
  qemu-img.c |   15 ++-
  1 files changed, 10 insertions(+), 5 deletions(-)
 
 diff --git a/qemu-img.c b/qemu-img.c
 index 3cea8ce..f8be5cb 100644
 --- a/qemu-img.c
 +++ b/qemu-img.c
 @@ -188,11 +188,13 @@ static int read_password(char *buf, int buf_size)
  #endif
  
  static BlockDriverState *bdrv_new_open(const char *filename,
 -   const char *fmt)
 +   const char *fmt,
 +   int readonly)
  {
  BlockDriverState *bs;
  BlockDriver *drv;
  char password[256];
 +int flags = BRDV_O_FLAGS;
  
  bs = bdrv_new();
  if (!bs)
 @@ -204,7 +206,10 @@ static BlockDriverState *bdrv_new_open(const char 
 *filename,
  } else {
  drv = NULL;
  }
 -if (bdrv_open2(bs, filename, BRDV_O_FLAGS | BDRV_O_RDWR, drv)  0) {
 +if (!readonly) {
 +flags |= BDRV_O_RDWR;
 +}
 +if (bdrv_open2(bs, filename, flags, drv)  0) {
  error(Could not open '%s', filename);
  }
  if (bdrv_is_encrypted(bs)) {
 @@ -343,7 +348,7 @@ static int img_create(int argc, char **argv)
  }
  }
  
 -bs = bdrv_new_open(backing_file-value.s, fmt);
 +bs = bdrv_new_open(backing_file-value.s, fmt, 1);
  bdrv_get_geometry(bs, size);
  size *= 512;
  bdrv_delete(bs);
 @@ -627,7 +632,7 @@ static int img_convert(int argc, char **argv)
  
  total_sectors = 0;
  for (bs_i = 0; bs_i  bs_n; bs_i++) {
 -bs[bs_i] = bdrv_new_open(argv[optind + bs_i], fmt);
 +bs[bs_i] = bdrv_new_open(argv[optind + bs_i], fmt, 0);

Shouldn't it be read-only here, too?

Kevin
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Qemu-devel] [PATCH] Fix qemu-img can't create qcow image based on read-only image

2010-01-28 Thread Sheng Yang 
On Thursday 28 January 2010 17:17:22 Kevin Wolf wrote:
 Am 28.01.2010 06:22, schrieb Sheng Yang:
  Commit 03cbdac7 Disable fall-back to read-only when cannot open drive's
  file for read-write result in read-only image can't be used as backed
  image in qemu-img.
 
  CC: Naphtali Sprei nsp...@redhat.com
  Signed-off-by: Sheng Yang sh...@linux.intel.com
  ---
 
  This issue blocked our QA's KVM nightly test. But in fact, I don't like
  this patch, feeling uncomfortable to change long existed interface... Any
  alternative? Add a readonly command line would change the default
  behavior(I don't think fall back to readonly looks like a bug); or even
  revert the commit? What's the story behind it?
 
   qemu-img.c |   15 ++-
   1 files changed, 10 insertions(+), 5 deletions(-)
 
  diff --git a/qemu-img.c b/qemu-img.c
  index 3cea8ce..f8be5cb 100644
  --- a/qemu-img.c
  +++ b/qemu-img.c
  @@ -188,11 +188,13 @@ static int read_password(char *buf, int buf_size)
   #endif
 
   static BlockDriverState *bdrv_new_open(const char *filename,
  -   const char *fmt)
  +   const char *fmt,
  +   int readonly)
   {
   BlockDriverState *bs;
   BlockDriver *drv;
   char password[256];
  +int flags = BRDV_O_FLAGS;
 
   bs = bdrv_new();
   if (!bs)
  @@ -204,7 +206,10 @@ static BlockDriverState *bdrv_new_open(const char
  *filename, } else {
   drv = NULL;
   }
  -if (bdrv_open2(bs, filename, BRDV_O_FLAGS | BDRV_O_RDWR, drv)  0) {
  +if (!readonly) {
  +flags |= BDRV_O_RDWR;
  +}
  +if (bdrv_open2(bs, filename, flags, drv)  0) {
   error(Could not open '%s', filename);
   }
   if (bdrv_is_encrypted(bs)) {
  @@ -343,7 +348,7 @@ static int img_create(int argc, char **argv)
   }
   }
 
  -bs = bdrv_new_open(backing_file-value.s, fmt);
  +bs = bdrv_new_open(backing_file-value.s, fmt, 1);
   bdrv_get_geometry(bs, size);
   size *= 512;
   bdrv_delete(bs);
  @@ -627,7 +632,7 @@ static int img_convert(int argc, char **argv)
 
   total_sectors = 0;
   for (bs_i = 0; bs_i  bs_n; bs_i++) {
  -bs[bs_i] = bdrv_new_open(argv[optind + bs_i], fmt);
  +bs[bs_i] = bdrv_new_open(argv[optind + bs_i], fmt, 0);
 
 Shouldn't it be read-only here, too?
 
Yes. 

Seems you guys are OK with this solution(and I think it's reasonable to remove 
the fall back). I would update the patch, hopefully it can be applied soon and 
picked up by Marcelo to resolve our block issue...

-- 
regards
Yang, Sheng
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Fix qemu-img can't create qcow image based on read-only image

2010-01-27 Thread Sheng Yang 
Commit 03cbdac7 Disable fall-back to read-only when cannot open drive's
file for read-write result in read-only image can't be used as backed
image in qemu-img.

CC: Naphtali Sprei nsp...@redhat.com
Signed-off-by: Sheng Yang sh...@linux.intel.com
---

This issue blocked our QA's KVM nightly test. But in fact, I don't like this
patch, feeling uncomfortable to change long existed interface... Any
alternative? Add a readonly command line would change the default behavior(I
don't think fall back to readonly looks like a bug); or even revert the
commit? What's the story behind it?

 qemu-img.c |   15 ++-
 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 3cea8ce..f8be5cb 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -188,11 +188,13 @@ static int read_password(char *buf, int buf_size)
 #endif
 
 static BlockDriverState *bdrv_new_open(const char *filename,
-   const char *fmt)
+   const char *fmt,
+   int readonly)
 {
 BlockDriverState *bs;
 BlockDriver *drv;
 char password[256];
+int flags = BRDV_O_FLAGS;
 
 bs = bdrv_new();
 if (!bs)
@@ -204,7 +206,10 @@ static BlockDriverState *bdrv_new_open(const char 
*filename,
 } else {
 drv = NULL;
 }
-if (bdrv_open2(bs, filename, BRDV_O_FLAGS | BDRV_O_RDWR, drv)  0) {
+if (!readonly) {
+flags |= BDRV_O_RDWR;
+}
+if (bdrv_open2(bs, filename, flags, drv)  0) {
 error(Could not open '%s', filename);
 }
 if (bdrv_is_encrypted(bs)) {
@@ -343,7 +348,7 @@ static int img_create(int argc, char **argv)
 }
 }
 
-bs = bdrv_new_open(backing_file-value.s, fmt);
+bs = bdrv_new_open(backing_file-value.s, fmt, 1);
 bdrv_get_geometry(bs, size);
 size *= 512;
 bdrv_delete(bs);
@@ -627,7 +632,7 @@ static int img_convert(int argc, char **argv)
 
 total_sectors = 0;
 for (bs_i = 0; bs_i  bs_n; bs_i++) {
-bs[bs_i] = bdrv_new_open(argv[optind + bs_i], fmt);
+bs[bs_i] = bdrv_new_open(argv[optind + bs_i], fmt, 0);
 if (!bs[bs_i])
 error(Could not open '%s', argv[optind + bs_i]);
 bdrv_get_geometry(bs[bs_i], bs_sectors);
@@ -685,7 +690,7 @@ static int img_convert(int argc, char **argv)
 }
 }
 
-out_bs = bdrv_new_open(out_filename, out_fmt);
+out_bs = bdrv_new_open(out_filename, out_fmt, 0);
 
 bs_i = 0;
 bs_offset = 0;
-- 
1.5.4.5

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html