Re: [Qemu-devel] [PATCH] blockdev: fix cdrom read_only flag

2013-10-15 Thread Kevin Wolf
Am 15.10.2013 um 03:27 hat Fam Zheng geschrieben:
 Since 0ebd24e0, cdrom doesn't have read-only on by default, which will
 error out when using an read only image. Fix it by setting the default
 value when parsing opts.
 
 Reported-by: Edivaldo de Araujo Pereira edivaldoapere...@yahoo.com.br
 Signed-off-by: Fam Zheng f...@redhat.com
 ---
  blockdev.c | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)
 
 diff --git a/blockdev.c b/blockdev.c
 index 4f76e28..7f5ef4a 100644
 --- a/blockdev.c
 +++ b/blockdev.c
 @@ -625,7 +625,8 @@ DriveInfo *drive_init(QemuOpts *all_opts, 
 BlockInterfaceType block_default_type)
  int cyls, heads, secs, translation;
  int max_devs, bus_id, unit_id, index;
  const char *devaddr;
 -bool read_only, copy_on_read;
 +bool read_only = false;
 +bool copy_on_read;
  Error *local_err = NULL;
  
  /* Change legacy command line options into QMP ones */
 @@ -701,7 +702,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, 
 BlockInterfaceType block_default_type)
  media = MEDIA_DISK;
  } else if (!strcmp(value, cdrom)) {
  media = MEDIA_CDROM;
 -qdict_put(bs_opts, read-only, qstring_from_str(on));
 +read_only = true;
  } else {
  error_report('%s' invalid media, value);
  goto fail;
 @@ -709,7 +710,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, 
 BlockInterfaceType block_default_type)
  }
  
  /* copy-on-read is disabled with a warning for read-only devices */
 -read_only = qemu_opt_get_bool(legacy_opts, read-only, false);
 +read_only = qemu_opt_get_bool(legacy_opts, read-only, read_only);

I believe we must move this line to above the media=... handling for
compatibility with old versions. Or actually, using |= instead of = may
be enough.

The reason is this command line:

-drive file=test.iso,media=cdrom,readonly=off

Which, obviously, means that a read-only CD-ROM device should be
created.

Kevin



Re: [Qemu-devel] [PATCH] blockdev: fix cdrom read_only flag

2013-10-15 Thread Fam Zheng
On Tue, 10/15 11:13, Kevin Wolf wrote:
 Am 15.10.2013 um 03:27 hat Fam Zheng geschrieben:
  Since 0ebd24e0, cdrom doesn't have read-only on by default, which will
  error out when using an read only image. Fix it by setting the default
  value when parsing opts.
  
  Reported-by: Edivaldo de Araujo Pereira edivaldoapere...@yahoo.com.br
  Signed-off-by: Fam Zheng f...@redhat.com
  ---
   blockdev.c | 7 ---
   1 file changed, 4 insertions(+), 3 deletions(-)
  
  diff --git a/blockdev.c b/blockdev.c
  index 4f76e28..7f5ef4a 100644
  --- a/blockdev.c
  +++ b/blockdev.c
  @@ -625,7 +625,8 @@ DriveInfo *drive_init(QemuOpts *all_opts, 
  BlockInterfaceType block_default_type)
   int cyls, heads, secs, translation;
   int max_devs, bus_id, unit_id, index;
   const char *devaddr;
  -bool read_only, copy_on_read;
  +bool read_only = false;
  +bool copy_on_read;
   Error *local_err = NULL;
   
   /* Change legacy command line options into QMP ones */
  @@ -701,7 +702,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, 
  BlockInterfaceType block_default_type)
   media = MEDIA_DISK;
   } else if (!strcmp(value, cdrom)) {
   media = MEDIA_CDROM;
  -qdict_put(bs_opts, read-only, qstring_from_str(on));
  +read_only = true;
   } else {
   error_report('%s' invalid media, value);
   goto fail;
  @@ -709,7 +710,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, 
  BlockInterfaceType block_default_type)
   }
   
   /* copy-on-read is disabled with a warning for read-only devices */
  -read_only = qemu_opt_get_bool(legacy_opts, read-only, false);
  +read_only = qemu_opt_get_bool(legacy_opts, read-only, read_only);
 
 I believe we must move this line to above the media=... handling for
 compatibility with old versions. Or actually, using |= instead of = may
 be enough.
 
 The reason is this command line:
 
 -drive file=test.iso,media=cdrom,readonly=off
 
 Which, obviously, means that a read-only CD-ROM device should be
 created.
 

Oh, I didn't notice readony=off means a read-only CD-ROM before...

Fam



[Qemu-devel] [PATCH] blockdev: fix cdrom read_only flag

2013-10-14 Thread Fam Zheng
Since 0ebd24e0, cdrom doesn't have read-only on by default, which will
error out when using an read only image. Fix it by setting the default
value when parsing opts.

Reported-by: Edivaldo de Araujo Pereira edivaldoapere...@yahoo.com.br
Signed-off-by: Fam Zheng f...@redhat.com
---
 blockdev.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 4f76e28..7f5ef4a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -625,7 +625,8 @@ DriveInfo *drive_init(QemuOpts *all_opts, 
BlockInterfaceType block_default_type)
 int cyls, heads, secs, translation;
 int max_devs, bus_id, unit_id, index;
 const char *devaddr;
-bool read_only, copy_on_read;
+bool read_only = false;
+bool copy_on_read;
 Error *local_err = NULL;
 
 /* Change legacy command line options into QMP ones */
@@ -701,7 +702,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, 
BlockInterfaceType block_default_type)
 media = MEDIA_DISK;
 } else if (!strcmp(value, cdrom)) {
 media = MEDIA_CDROM;
-qdict_put(bs_opts, read-only, qstring_from_str(on));
+read_only = true;
 } else {
 error_report('%s' invalid media, value);
 goto fail;
@@ -709,7 +710,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, 
BlockInterfaceType block_default_type)
 }
 
 /* copy-on-read is disabled with a warning for read-only devices */
-read_only = qemu_opt_get_bool(legacy_opts, read-only, false);
+read_only = qemu_opt_get_bool(legacy_opts, read-only, read_only);
 copy_on_read = qemu_opt_get_bool(legacy_opts, copy-on-read, false);
 
 if (read_only  copy_on_read) {
-- 
1.8.3.1