From 684b25447c10b9171e5aa9305075b830885fe6e3 Mon Sep 17 00:00:00 2001 From: Mike Qiu Date: Wed, 16 Oct 2013 23:16:01 -0400 Subject: [PATCH 1/7] hmp: drop bogus "[not inserted]" 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 Signed-off-by: Stefan Hajnoczi --- hmp.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/hmp.c b/hmp.c index 589150773e..32ee285a1e 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) { From 794cbc26eb94ce13c75d105eea9ff0afff56e2c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20F=C3=A4rber?= Date: Wed, 16 Oct 2013 15:24:01 +0200 Subject: [PATCH 2/7] sd: Avoid access to NULL BlockDriverState MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Andreas Färber Signed-off-by: Stefan Hajnoczi --- 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 7380f063f7..4502ad143d 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; } From a7fdbcf0e6e52d935ebff6d849fe4b5473e5860d Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 15 Oct 2013 17:45:50 +0800 Subject: [PATCH 3/7] blockdev: fix cdrom read_only flag 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 Signed-off-by: Fam Zheng Reviewed-by: Kevin Wolf Signed-off-by: Stefan Hajnoczi --- blockdev.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/blockdev.c b/blockdev.c index 4f76e28164..b260477f1b 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) { From 45d57f6e718e44e55780bcf1d09fa140dce7ec08 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 11 Oct 2013 14:30:16 +0200 Subject: [PATCH 4/7] block/raw-win32: Always use -errno in hdev_open 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 Reviewed-by: Eric Blake Signed-off-by: Stefan Hajnoczi --- 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 c3e4c62d53..676b5701db 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; } From b432779a9fe9c2a1bb8cbd98feb341af6e32f892 Mon Sep 17 00:00:00 2001 From: Stefan Weil Date: Thu, 17 Oct 2013 21:23:26 +0200 Subject: [PATCH 5/7] virtio: Remove unneeded memcpy 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 Signed-off-by: Stefan Weil Reviewed-by: Peter Maydell Signed-off-by: Stefan Hajnoczi --- 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 49a23c33f7..13f6d8276e 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; From c338b6ad609699cf352c8dd6338360b7e3895ad0 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Fri, 18 Oct 2013 13:17:19 +0800 Subject: [PATCH 6/7] vmdk: Only read cid from image file when opening 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 Signed-off-by: Stefan Hajnoczi --- block/vmdk.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 5a9f2787f8..b8901e272a 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; } From dbbcaa8d4358fdf3c42bf01e9e2d687300e84770 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Fri, 18 Oct 2013 15:07:33 +0800 Subject: [PATCH 7/7] vmdk: fix VMFS extent parsing 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 Signed-off-by: Stefan Hajnoczi --- block/vmdk.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/block/vmdk.c b/block/vmdk.c index b8901e272a..32ec8b7766 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;