From 555a608c5d5dcc44e45a483ca09b449d8db519d1 Mon Sep 17 00:00:00 2001 From: Klim Kireev Date: Wed, 27 Jul 2016 20:08:20 +0300 Subject: [PATCH 1/5] block/parallels: check new image size Before this patch incorrect image could be created via qemu-img (Example: qemu-img create -f parallels -o size=4096T hack.img), incorrect images cannot be used due to overflow in main image structure. This patch add check of size in image creation. After reading size it compare it with UINT32_MAX * cluster_size. Signed-off-by: Klim Kireev Signed-off-by: Denis V. Lunev Message-id: 1469639300-12155-1-git-send-email-den@openvz.org CC: Stefan Hajnoczi Signed-off-by: Stefan Hajnoczi --- block/parallels.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/block/parallels.c b/block/parallels.c index 807a80169f..2ccefa7d85 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -43,6 +43,7 @@ #define HEADER_MAGIC2 "WithouFreSpacExt" #define HEADER_VERSION 2 #define HEADER_INUSE_MAGIC (0x746F6E59) +#define MAX_PARALLELS_IMAGE_FACTOR (1ull << 32) #define DEFAULT_CLUSTER_SIZE 1048576 /* 1 MiB */ @@ -475,6 +476,10 @@ static int parallels_create(const char *filename, QemuOpts *opts, Error **errp) BDRV_SECTOR_SIZE); cl_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_CLUSTER_SIZE, DEFAULT_CLUSTER_SIZE), BDRV_SECTOR_SIZE); + if (total_size >= MAX_PARALLELS_IMAGE_FACTOR * cl_size) { + error_propagate(errp, local_err); + return -E2BIG; + } ret = bdrv_create_file(filename, opts, &local_err); if (ret < 0) { From aaa1e77ffae5255fb2a35284e7fc721742bb41c8 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Thu, 28 Jul 2016 11:08:12 +0300 Subject: [PATCH 2/5] throttle: Don't allow burst limits to be lower than the normal limits Setting FOO_max to a value that is lower than FOO does not make sense, and it produces odd results depending on the value of FOO_max_length. Although the user should not set that configuration in the first place it's better to reject it explicitly. https://bugzilla.redhat.com/show_bug.cgi?id=1355665 Signed-off-by: Alberto Garcia Reported-by: Gu Nini Reviewed-by: Eric Blake Message-id: 663d5aca406060e31f80d8113f77b6feee63b919.1469693110.git.berto@igalia.com Signed-off-by: Stefan Hajnoczi --- util/throttle.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/util/throttle.c b/util/throttle.c index 654f95caf2..3817d9b904 100644 --- a/util/throttle.c +++ b/util/throttle.c @@ -348,6 +348,11 @@ bool throttle_is_valid(ThrottleConfig *cfg, Error **errp) " bps/iops values"); return false; } + + if (cfg->buckets[i].max && cfg->buckets[i].max < cfg->buckets[i].avg) { + error_setg(errp, "bps_max/iops_max cannot be lower than bps/iops"); + return false; + } } return true; From 5fc8c052ce0ab8cc476a4a3888cc31d52783c315 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Thu, 28 Jul 2016 11:08:13 +0300 Subject: [PATCH 3/5] throttle: Test burst limits lower than the normal limits This checks that making FOO_max lower than FOO is not allowed. We could also forbid having FOO_max == FOO, but that doesn't have any odd side effects and it would require us to update several other tests, so let's keep it simple. Signed-off-by: Alberto Garcia Reviewed-by: Eric Blake Message-id: 2f90f9ee58aa14b7bd985f67c5996b06e0ab6c19.1469693110.git.berto@igalia.com Signed-off-by: Stefan Hajnoczi --- tests/test-throttle.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/test-throttle.c b/tests/test-throttle.c index afe094bcc4..363b59a38f 100644 --- a/tests/test-throttle.c +++ b/tests/test-throttle.c @@ -394,6 +394,14 @@ static void test_max_is_missing_limit(void) cfg.buckets[i].max = 0; cfg.buckets[i].avg = 100; g_assert(throttle_is_valid(&cfg, NULL)); + + cfg.buckets[i].max = 30; + cfg.buckets[i].avg = 100; + g_assert(!throttle_is_valid(&cfg, NULL)); + + cfg.buckets[i].max = 100; + cfg.buckets[i].avg = 100; + g_assert(throttle_is_valid(&cfg, NULL)); } } From 26307f6aa451708b8c60a49d605a78bbd29f70ef Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Thu, 4 Aug 2016 10:44:13 +0800 Subject: [PATCH 4/5] virtio-blk: Release s->rq queue at system_reset At system_reset, there is no point in retrying the queued request, because the driver that issued the request won't be around any more. Analyzed-by: Laszlo Ersek Reported-by: Laszlo Ersek Signed-off-by: Fam Zheng Reviewed-by: Laszlo Ersek Message-id: 1470278654-13525-2-git-send-email-famz@redhat.com Signed-off-by: Stefan Hajnoczi --- hw/block/virtio-blk.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 475a822f5a..1e348b1abb 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -654,6 +654,7 @@ static void virtio_blk_reset(VirtIODevice *vdev) { VirtIOBlock *s = VIRTIO_BLK(vdev); AioContext *ctx; + VirtIOBlockReq *req; /* * This should cancel pending requests, but can't do nicely until there @@ -663,6 +664,14 @@ static void virtio_blk_reset(VirtIODevice *vdev) aio_context_acquire(ctx); blk_drain(s->blk); + /* We drop queued requests after blk_drain() because blk_drain() itself can + * produce them. */ + while (s->rq) { + req = s->rq; + s->rq = req->next; + virtio_blk_free_request(req); + } + if (s->dataplane) { virtio_blk_data_plane_stop(s->dataplane); } From 27d1b87688dcea8702f06b5240abf6b8d8f53346 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Thu, 4 Aug 2016 10:44:14 +0800 Subject: [PATCH 5/5] virtio-blk: Remove stale comment about draining This is stale after commit 6e40b3bf (virtio-blk: Use blk_drain() to drain IO requests), remove it. Suggested-by: Laszlo Ersek Signed-off-by: Fam Zheng Reviewed-by: Laszlo Ersek Message-id: 1470278654-13525-3-git-send-email-famz@redhat.com Signed-off-by: Stefan Hajnoczi --- hw/block/virtio-blk.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 1e348b1abb..331d7667ec 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -656,10 +656,6 @@ static void virtio_blk_reset(VirtIODevice *vdev) AioContext *ctx; VirtIOBlockReq *req; - /* - * This should cancel pending requests, but can't do nicely until there - * are per-device request lists. - */ ctx = blk_get_aio_context(s->blk); aio_context_acquire(ctx); blk_drain(s->blk);