[Qemu-devel] [PULL 0/7] Block patches

[Qemu-devel] [PULL 0/7] Block patches

[Stefan Hajnoczi]

The following changes since commit 1680d485777ecf436d724631ea8722cc0c66990e:

  Merge remote-tracking branch 'rth/tcg-ldst-6' into staging (2013-10-14 09:59:59 -0700)

are available in the git repository at:


  git://github.com/stefanha/qemu.git block

for you to fetch changes up to dbbcaa8d4358fdf3c42bf01e9e2d687300e84770:

  vmdk: fix VMFS extent parsing (2013-10-18 13:41:36 +0200)

----------------------------------------------------------------
Andreas Färber (1):
      sd: Avoid access to NULL BlockDriverState

Fam Zheng (3):
      blockdev: fix cdrom read_only flag
      vmdk: Only read cid from image file when opening
      vmdk: fix VMFS extent parsing

Max Reitz (1):
      block/raw-win32: Always use -errno in hdev_open

Mike Qiu (1):
      hmp: drop bogus "[not inserted]"

Stefan Weil (1):
      virtio: Remove unneeded memcpy

 block/raw-win32.c     |  5 ++---
 block/vmdk.c          | 10 +++++-----
 blockdev.c            |  7 ++++---
 hmp.c                 |  2 --
 hw/block/virtio-blk.c |  1 -
 hw/sd/sd.c            |  2 +-
 6 files changed, 12 insertions(+), 15 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PULL 1/7] hmp: drop bogus "[not inserted]"

[Stefan Hajnoczi]

From: Mike Qiu <qiudayu@linux.vnet.ibm.com>

Commit 3e9fab690d59ac15956c3733fe0794ce1ae4c4af ("block: Add support for
throttling burst max in QMP and the command line.") introduced bogus
"[not inserted]" output, possibly due to a merge failure.  Remove this
artifact.

Output of 'info block'

scsi0-hd0: /images/f18-ppc64.qcow2 (qcow2)
 [not inserted]
scsi0-cd2: [not inserted]
    Removable device: not locked, tray closed

floppy0: [not inserted]
    Removable device: not locked, tray closed

sd0: [not inserted]
    Removable device: not locked, tray closed

There will be no additional lines between scsi0-hd0 and
scsi0-cd2.

At the same time, scsi0-hd0 already inserted, but still has
'[not inserted]' flag. This line should be removed.

This patch is to solve this.

Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hmp.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hmp.c b/hmp.c
index 5891507..32ee285 100644
--- a/hmp.c
+++ b/hmp.c
@@ -366,8 +366,6 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
                             info->value->inserted->iops_rd_max,
                             info->value->inserted->iops_wr_max,
                             info->value->inserted->iops_size);
-        } else {
-            monitor_printf(mon, " [not inserted]");
         }
 
         if (verbose) {
-- 
1.8.3.1

[Qemu-devel] [PULL 2/7] sd: Avoid access to NULL BlockDriverState

[Stefan Hajnoczi]

From: Andreas Färber <afaerber@suse.de>

Commit 4f8a066b5fc254eeaabbbde56ba4f5b29cc68fdf (blockdev: Remove IF_*
check for read-only blockdev_init) added a usage of bdrv_is_read_only()
to sd_init(), which is called for versatilepb, versatileab and
xilinx-zynq-a9 machines among others with NULL argument by default,
causing the new qom-test to fail.

Add a check to prevent this.

Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/sd/sd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 7380f06..4502ad1 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -494,7 +494,7 @@ SDState *sd_init(BlockDriverState *bs, bool is_spi)
 {
     SDState *sd;
 
-    if (bdrv_is_read_only(bs)) {
+    if (bs && bdrv_is_read_only(bs)) {
         fprintf(stderr, "sd_init: Cannot use read-only drive\n");
         return NULL;
     }
-- 
1.8.3.1

[Qemu-devel] [PULL 3/7] blockdev: fix cdrom read_only flag

[Stefan Hajnoczi]

From: Fam Zheng <famz@redhat.com>

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 <edivaldoapereira@yahoo.com.br>
Signed-off-by: Fam Zheng <famz@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 blockdev.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 4f76e28..b260477 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", false);
     copy_on_read = qemu_opt_get_bool(legacy_opts, "copy-on-read", false);
 
     if (read_only && copy_on_read) {
-- 
1.8.3.1

[Qemu-devel] [PULL 4/7] block/raw-win32: Always use -errno in hdev_open

[Stefan Hajnoczi]

From: Max Reitz <mreitz@redhat.com>

On one occasion, hdev_open() returned -1 in case of an unknown error
instead of a proper -errno value. Adjust this to match the behavior of
raw_open() (in raw-win32), which is to return -EINVAL in this case.
Also, change the call to error_setg*() to match the one in raw_open() as
well.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/raw-win32.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/block/raw-win32.c b/block/raw-win32.c
index c3e4c62..676b570 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -590,12 +590,11 @@ static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
         int err = GetLastError();
 
         if (err == ERROR_ACCESS_DENIED) {
-            error_setg_errno(errp, EACCES, "Could not open device");
             ret = -EACCES;
         } else {
-            error_setg(errp, "Could not open device");
-            ret = -1;
+            ret = -EINVAL;
         }
+        error_setg_errno(errp, -ret, "Could not open device");
         goto done;
     }
 
-- 
1.8.3.1

[Qemu-devel] [PULL 5/7] virtio: Remove unneeded memcpy

[Stefan Hajnoczi]

From: Stefan Weil <sw@weilnetz.de>

Report from valgrind:

==19521== Source and destination overlap in memcpy(0x31d38938, 0x31d38938, 64)
==19521==    at 0x4A0A343: memcpy@@GLIBC_2.14 (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==19521==    by 0x42774E: virtio_blk_device_init (virtio-blk.c:686)
==19521==    by 0x46EE9E: virtio_device_init (virtio.c:1158)
==19521==    by 0x25405E: device_realize (qdev.c:178)
==19521==    by 0x2559B5: device_set_realized (qdev.c:699)
==19521==    by 0x3A819B: property_set_bool (object.c:1315)
==19521==    by 0x3A6CE0: object_property_set (object.c:803)

Valgrind is right: blk == &s->blks, so it is a memcpy of 64 byte with
source == destination which can be removed.

Reported-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/block/virtio-blk.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 49a23c3..13f6d82 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -703,7 +703,6 @@ static int virtio_blk_device_init(VirtIODevice *vdev)
 
     s->bs = blk->conf.bs;
     s->conf = &blk->conf;
-    memcpy(&(s->blk), blk, sizeof(struct VirtIOBlkConf));
     s->rq = NULL;
     s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1;
 
-- 
1.8.3.1

[Qemu-devel] [PULL 6/7] vmdk: Only read cid from image file when opening

[Stefan Hajnoczi]

From: Fam Zheng <famz@redhat.com>

Previously cid of parent is parsed from image file for every IO request.
We already have L1/L2 cache and don't have assumption that parent image
can be updated behind us, so remove this to get more efficiency.

The parent CID is checked only for once after opening.

Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/vmdk.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 5a9f278..b8901e2 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -112,6 +112,7 @@ typedef struct BDRVVmdkState {
     CoMutex lock;
     uint64_t desc_offset;
     bool cid_updated;
+    bool cid_checked;
     uint32_t parent_cid;
     int num_extents;
     /* Extent array with num_extents entries, ascend ordered by address */
@@ -197,8 +198,6 @@ static int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename)
     }
 }
 
-#define CHECK_CID 1
-
 #define SECTOR_SIZE 512
 #define DESC_SIZE (20 * SECTOR_SIZE)    /* 20 sectors of 512 bytes each */
 #define BUF_SIZE 4096
@@ -301,19 +300,18 @@ static int vmdk_write_cid(BlockDriverState *bs, uint32_t cid)
 
 static int vmdk_is_cid_valid(BlockDriverState *bs)
 {
-#ifdef CHECK_CID
     BDRVVmdkState *s = bs->opaque;
     BlockDriverState *p_bs = bs->backing_hd;
     uint32_t cur_pcid;
 
-    if (p_bs) {
+    if (!s->cid_checked && p_bs) {
         cur_pcid = vmdk_read_cid(p_bs, 0);
         if (s->parent_cid != cur_pcid) {
             /* CID not valid */
             return 0;
         }
     }
-#endif
+    s->cid_checked = true;
     /* CID valid */
     return 1;
 }
-- 
1.8.3.1

[Qemu-devel] [PULL 7/7] vmdk: fix VMFS extent parsing

[Stefan Hajnoczi]

From: Fam Zheng <famz@redhat.com>

The VMFS extent line in description file doesn't have start offset as
FLAT lines does, and it should be defaulted to 0. The flat_offset
variable is initialized to -1, so we need to set it in this case.

Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/vmdk.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/vmdk.c b/block/vmdk.c
index b8901e2..32ec8b7 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -726,6 +726,8 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
                 error_setg(errp, "Invalid extent lines: \n%s", p);
                 return -EINVAL;
             }
+        } else if (!strcmp(type, "VMFS")) {
+            flat_offset = 0;
         } else if (ret != 4) {
             error_setg(errp, "Invalid extent lines: \n%s", p);
             return -EINVAL;
-- 
1.8.3.1