From 982aa95532a3a7b549695d5b3e18442975eecfb5 Mon Sep 17 00:00:00 2001 From: Jes Sorensen Date: Fri, 4 Feb 2011 09:22:14 +0100 Subject: [PATCH 01/11] Change snapshot_blkdev hmp to use correct argument type for device Pointed out by Markus Signed-off-by: Jes Sorensen Signed-off-by: Kevin Wolf --- hmp-commands.hx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index 38e1eb7ebb..372bef4290 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -822,7 +822,7 @@ ETEXI { .name = "snapshot_blkdev", - .args_type = "device:s,snapshot_file:s?,format:s?", + .args_type = "device:B,snapshot_file:s?,format:s?", .params = "device [new-image-file] [format]", .help = "initiates a live snapshot\n\t\t\t" "of device. If a new image file is specified, the\n\t\t\t" From e0d9c6f93729c9bfc98fcafcd73098bb8e131aeb Mon Sep 17 00:00:00 2001 From: Chunqiang Tang Date: Thu, 3 Feb 2011 10:12:49 -0500 Subject: [PATCH 02/11] QCOW2: bug fix - read base image beyond its size This patch fixes the following bug in QCOW2. For a QCOW2 image that is larger than its base image, when handling a read request straddling over the end of the base image, the QCOW2 driver attempts to read beyond the end of the base image and the request would fail. This bug was found by Fast Virtual Disk (FVD)'s fully automated testing tool. The following test triggered the bug. dd if=/dev/zero of=/var/ramdisk/truth.raw count=0 bs=1 seek=1098561536 dd if=/dev/zero of=/var/ramdisk/zero-500M.raw count=0 bs=1 seek=593099264 ./qemu-img create -f qcow2 -ocluster_size=65536,backing_fmt=blksim -b /var/ramdisk/zero-500M.raw /var/ramdisk/test.qcow2 1098561536 ./qemu-io --auto --seed=30477694 --truth=/var/ramdisk/truth.raw --format=qcow2 --test=blksim:/var/ramdisk/test.qcow2 --verify_write=true --compare_before=false --compare_after=true --round=100000 --parallel=100 --io_size=10485760 --fail_prob=0 --cancel_prob=0 --instant_qemubh=true Signed-off-by: Chunqiang Tang Signed-off-by: Kevin Wolf --- block/qcow2.c | 5 ++--- cutils.c | 31 +++++++++++++++++++++++++++++++ qemu-common.h | 2 ++ 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index a1773e4790..28338bf9d0 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -355,7 +355,7 @@ int qcow2_backing_read1(BlockDriverState *bs, QEMUIOVector *qiov, else n1 = bs->total_sectors - sector_num; - qemu_iovec_memset(qiov, 0, 512 * (nb_sectors - n1)); + qemu_iovec_memset_skip(qiov, 0, 512 * (nb_sectors - n1), 512 * n1); return n1; } @@ -478,8 +478,7 @@ static void qcow2_aio_read_cb(void *opaque, int ret) if (n1 > 0) { BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO); acb->hd_aiocb = bdrv_aio_readv(bs->backing_hd, acb->sector_num, - &acb->hd_qiov, acb->cur_nr_sectors, - qcow2_aio_read_cb, acb); + &acb->hd_qiov, n1, qcow2_aio_read_cb, acb); if (acb->hd_aiocb == NULL) goto done; } else { diff --git a/cutils.c b/cutils.c index 8d562b28ab..f9a7e3689e 100644 --- a/cutils.c +++ b/cutils.c @@ -267,6 +267,37 @@ void qemu_iovec_memset(QEMUIOVector *qiov, int c, size_t count) } } +void qemu_iovec_memset_skip(QEMUIOVector *qiov, int c, size_t count, + size_t skip) +{ + int i; + size_t done; + void *iov_base; + uint64_t iov_len; + + done = 0; + for (i = 0; (i < qiov->niov) && (done != count); i++) { + if (skip >= qiov->iov[i].iov_len) { + /* Skip the whole iov */ + skip -= qiov->iov[i].iov_len; + continue; + } else { + /* Skip only part (or nothing) of the iov */ + iov_base = (uint8_t*) qiov->iov[i].iov_base + skip; + iov_len = qiov->iov[i].iov_len - skip; + skip = 0; + } + + if (done + iov_len > count) { + memset(iov_base, c, count - done); + break; + } else { + memset(iov_base, c, iov_len); + } + done += iov_len; + } +} + #ifndef _WIN32 /* Sets a specific flag */ int fcntl_setfl(int fd, int flag) diff --git a/qemu-common.h b/qemu-common.h index c7ff280b95..cb4b7e0d38 100644 --- a/qemu-common.h +++ b/qemu-common.h @@ -322,6 +322,8 @@ void qemu_iovec_reset(QEMUIOVector *qiov); void qemu_iovec_to_buffer(QEMUIOVector *qiov, void *buf); void qemu_iovec_from_buffer(QEMUIOVector *qiov, const void *buf, size_t count); void qemu_iovec_memset(QEMUIOVector *qiov, int c, size_t count); +void qemu_iovec_memset_skip(QEMUIOVector *qiov, int c, size_t count, + size_t skip); struct Monitor; typedef struct Monitor Monitor; From 3ab4c7e92d39d40e6dc0bdb1c2320889543691cb Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 8 Feb 2011 18:12:35 +0100 Subject: [PATCH 03/11] qcow2: Fix error handling for immediate backing file read failure Requests could return success even though they failed when bdrv_aio_readv returned NULL for a backing file read. Reported-by: Chunqiang Tang Signed-off-by: Kevin Wolf --- block/qcow2.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/block/qcow2.c b/block/qcow2.c index 28338bf9d0..647c2a4bc4 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -479,8 +479,10 @@ static void qcow2_aio_read_cb(void *opaque, int ret) BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO); acb->hd_aiocb = bdrv_aio_readv(bs->backing_hd, acb->sector_num, &acb->hd_qiov, n1, qcow2_aio_read_cb, acb); - if (acb->hd_aiocb == NULL) + if (acb->hd_aiocb == NULL) { + ret = -EIO; goto done; + } } else { ret = qcow2_schedule_bh(qcow2_aio_read_bh, acb); if (ret < 0) From 8af364884355b3f0c5d60a2d2f427927739658ea Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 9 Feb 2011 10:26:06 +0100 Subject: [PATCH 04/11] qcow2: Fix error handling for reading compressed clusters When reading a compressed cluster failed, qcow2 falsely returned success. Signed-off-by: Kevin Wolf Reviewed-by: Markus Armbruster --- block/qcow2-cluster.c | 4 ++-- block/qcow2.c | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 5fb8c6695a..437aaa8174 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -878,11 +878,11 @@ int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset) BLKDBG_EVENT(bs->file, BLKDBG_READ_COMPRESSED); ret = bdrv_read(bs->file, coffset >> 9, s->cluster_data, nb_csectors); if (ret < 0) { - return -1; + return ret; } if (decompress_buffer(s->cluster_cache, s->cluster_size, s->cluster_data + sector_offset, csize) < 0) { - return -1; + return -EIO; } s->cluster_cache_offset = coffset; } diff --git a/block/qcow2.c b/block/qcow2.c index 647c2a4bc4..551b3c2f82 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -497,8 +497,10 @@ static void qcow2_aio_read_cb(void *opaque, int ret) } } else if (acb->cluster_offset & QCOW_OFLAG_COMPRESSED) { /* add AIO support for compressed blocks ? */ - if (qcow2_decompress_cluster(bs, acb->cluster_offset) < 0) + ret = qcow2_decompress_cluster(bs, acb->cluster_offset); + if (ret < 0) { goto done; + } qemu_iovec_from_buffer(&acb->hd_qiov, s->cluster_cache + index_in_cluster * 512, From f54e3641122e51c6343d587805422642f307462e Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 9 Feb 2011 11:09:38 +0100 Subject: [PATCH 05/11] qerror: Add QERR_UNKNOWN_BLOCK_FORMAT_FEATURE Signed-off-by: Kevin Wolf Reviewed-by: Anthony Liguori --- qerror.c | 5 +++++ qerror.h | 3 +++ 2 files changed, 8 insertions(+) diff --git a/qerror.c b/qerror.c index 9d0cdeb45c..485560418b 100644 --- a/qerror.c +++ b/qerror.c @@ -200,6 +200,11 @@ static const QErrorStringTable qerror_table[] = { .error_fmt = QERR_UNDEFINED_ERROR, .desc = "An undefined error has ocurred", }, + { + .error_fmt = QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, + .desc = "'%(device)' uses a %(format) feature which is not " + "supported by this qemu version: %(feature)", + }, { .error_fmt = QERR_VNC_SERVER_FAILED, .desc = "Could not start VNC server on %(target)", diff --git a/qerror.h b/qerror.h index b0f69dabe5..f732d45e80 100644 --- a/qerror.h +++ b/qerror.h @@ -165,6 +165,9 @@ QError *qobject_to_qerror(const QObject *obj); #define QERR_UNDEFINED_ERROR \ "{ 'class': 'UndefinedError', 'data': {} }" +#define QERR_UNKNOWN_BLOCK_FORMAT_FEATURE \ + "{ 'class': 'UnknownBlockFormatFeature', 'data': { 'device': %s, 'format': %s, 'feature': %s } }" + #define QERR_VNC_SERVER_FAILED \ "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }" From e8cdcec123facf0ed273d941caeeeb9b08f14955 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 9 Feb 2011 11:11:07 +0100 Subject: [PATCH 06/11] qcow2: Report error for version > 2 The qcow2 driver is now declared responsible for any QCOW image that has version 2 or greater (before this, version 3 would be detected as raw). For everything newer than version 2, an error is reported. Signed-off-by: Kevin Wolf Reviewed-by: Anthony Liguori --- block/qcow2.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index 551b3c2f82..75b8becc0a 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -28,6 +28,7 @@ #include "aes.h" #include "block/qcow2.h" #include "qemu-error.h" +#include "qerror.h" /* Differences with QCOW: @@ -59,7 +60,7 @@ static int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename) if (buf_size >= sizeof(QCowHeader) && be32_to_cpu(cow_header->magic) == QCOW_MAGIC && - be32_to_cpu(cow_header->version) == QCOW_VERSION) + be32_to_cpu(cow_header->version) >= QCOW_VERSION) return 100; else return 0; @@ -163,10 +164,18 @@ static int qcow2_open(BlockDriverState *bs, int flags) be64_to_cpus(&header.snapshots_offset); be32_to_cpus(&header.nb_snapshots); - if (header.magic != QCOW_MAGIC || header.version != QCOW_VERSION) { + if (header.magic != QCOW_MAGIC) { ret = -EINVAL; goto fail; } + if (header.version != QCOW_VERSION) { + char version[64]; + snprintf(version, sizeof(version), "QCOW version %d", header.version); + qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, + bs->device_name, "qcow2", version); + ret = -ENOTSUP; + goto fail; + } if (header.cluster_bits < MIN_CLUSTER_BITS || header.cluster_bits > MAX_CLUSTER_BITS) { ret = -EINVAL; From 10b758e85c9b38b4b370cff81435f6ed26024a26 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 9 Feb 2011 11:13:26 +0100 Subject: [PATCH 07/11] qed: Report error for unsupported features Instead of just returning -ENOTSUP, generate a more detailed error. Unfortunately we don't have a helpful text for features that we don't know yet, so just print the feature mask. It might be useful at least if someone asks for help. Signed-off-by: Kevin Wolf Reviewed-by: Anthony Liguori Acked-by: Stefan Hajnoczi --- block/qed.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/block/qed.c b/block/qed.c index 32734486c5..75ae2440ee 100644 --- a/block/qed.c +++ b/block/qed.c @@ -14,6 +14,7 @@ #include "trace.h" #include "qed.h" +#include "qerror.h" static void qed_aio_cancel(BlockDriverAIOCB *blockacb) { @@ -311,7 +312,13 @@ static int bdrv_qed_open(BlockDriverState *bs, int flags) return -EINVAL; } if (s->header.features & ~QED_FEATURE_MASK) { - return -ENOTSUP; /* image uses unsupported feature bits */ + /* image uses unsupported feature bits */ + char buf[64]; + snprintf(buf, sizeof(buf), "%" PRIx64, + s->header.features & ~QED_FEATURE_MASK); + qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, + bs->device_name, "QED", buf); + return -ENOTSUP; } if (!qed_is_cluster_size_valid(s->header.cluster_size)) { return -EINVAL; From b9eaf9ecb15a9c69a592f386159163d5efc3b919 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 9 Feb 2011 11:25:53 +0100 Subject: [PATCH 08/11] qemu-img: Improve error messages for failed bdrv_open Output the error message string of the bdrv_open return code. Also set a non-empty device name for the images because the unknown feature error message includes it. Signed-off-by: Kevin Wolf Reviewed-by: Anthony Liguori --- qemu-img.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index 4a3735811c..7e3cc4cbd5 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -213,8 +213,9 @@ static BlockDriverState *bdrv_new_open(const char *filename, BlockDriverState *bs; BlockDriver *drv; char password[256]; + int ret; - bs = bdrv_new(""); + bs = bdrv_new("image"); if (fmt) { drv = bdrv_find_format(fmt); @@ -225,10 +226,13 @@ static BlockDriverState *bdrv_new_open(const char *filename, } else { drv = NULL; } - if (bdrv_open(bs, filename, flags, drv) < 0) { - error_report("Could not open '%s'", filename); + + ret = bdrv_open(bs, filename, flags, drv); + if (ret < 0) { + error_report("Could not open '%s': %s", filename, strerror(-ret)); goto fail; } + if (bdrv_is_encrypted(bs)) { printf("Disk image '%s' is encrypted.\n", filename); if (read_password(password, sizeof(password)) < 0) { From 2753d4a5fa44d980cc6a279f323a12ca8d172972 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 8 Feb 2011 15:12:38 +0100 Subject: [PATCH 09/11] blockdev: Plug memory leak in drive_uninit() Started leaking in commit 1dae12e6. Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- blockdev.c | 1 + 1 file changed, 1 insertion(+) diff --git a/blockdev.c b/blockdev.c index ecfadc138f..24d765806b 100644 --- a/blockdev.c +++ b/blockdev.c @@ -182,6 +182,7 @@ static void drive_uninit(DriveInfo *dinfo) { qemu_opts_del(dinfo->opts); bdrv_delete(dinfo->bdrv); + qemu_free(dinfo->id); QTAILQ_REMOVE(&drives, dinfo, next); qemu_free(dinfo); } From a9ae2bffea62ce5158be7475fe41e5fba6d026c1 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 8 Feb 2011 15:12:39 +0100 Subject: [PATCH 10/11] blockdev: Plug memory leak in drive_init() error paths Should have spotted this when doing commit 319ae529. Signed-off-by: Markus Armbruster Signed-off-by: Kevin Wolf --- blockdev.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/blockdev.c b/blockdev.c index 24d765806b..0690cc8bea 100644 --- a/blockdev.c +++ b/blockdev.c @@ -526,7 +526,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) } else if (ro == 1) { if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY && type != IF_NONE) { error_report("readonly not supported by this bus type"); - return NULL; + goto err; } } @@ -536,12 +536,19 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) if (ret < 0) { error_report("could not open disk image %s: %s", file, strerror(-ret)); - return NULL; + goto err; } if (bdrv_key_required(dinfo->bdrv)) autostart = 0; return dinfo; + +err: + bdrv_delete(dinfo->bdrv); + qemu_free(dinfo->id); + QTAILQ_REMOVE(&drives, dinfo, next); + qemu_free(dinfo); + return NULL; } void do_commit(Monitor *mon, const QDict *qdict) From 16fde5f2c2788232b16c06d34d0459a5c1ec1f6c Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 9 Feb 2011 17:36:19 +0100 Subject: [PATCH 11/11] qcow2: Fix order in L2 table COW When copying L2 tables (this happens only with internal snapshots), the order wasn't completely safe, so that after a crash you could end up with a L2 table that has too low refcount, possibly leading to corruption in the long run. This patch puts the operations in the right order: First allocate the new L2 table and replace the reference, and only then decrease the refcount of the old table. Signed-off-by: Kevin Wolf --- block/qcow2-cluster.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 437aaa8174..750abe37d4 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -515,13 +515,16 @@ static int get_cluster_table(BlockDriverState *bs, uint64_t offset, return ret; } } else { - /* FIXME Order */ - if (l2_offset) - qcow2_free_clusters(bs, l2_offset, s->l2_size * sizeof(uint64_t)); + /* First allocate a new L2 table (and do COW if needed) */ ret = l2_allocate(bs, l1_index, &l2_table); if (ret < 0) { return ret; } + + /* Then decrease the refcount of the old table */ + if (l2_offset) { + qcow2_free_clusters(bs, l2_offset, s->l2_size * sizeof(uint64_t)); + } l2_offset = s->l1_table[l1_index] & ~QCOW_OFLAG_COPIED; }