From fca23f0ad211e4debf80796a65165d0eea146424 Mon Sep 17 00:00:00 2001 From: Liu Yuan Date: Mon, 18 Mar 2013 14:27:55 +0800 Subject: [PATCH 1/5] sheepdog: show error message for halt status Sheepdog (neither quorum nor unsafe mode) will refuse to serve IO requests when number of alive nodes is less than that of copies specified by users. This will return 0x19 to QEMU client which currently doesn't recognize it. This patch adds an error description when QEMU client receives it, other than plainly printing 'Invalid error code' Cc: MORITA Kazutaka Cc: Kevin Wolf Cc: Stefan Hajnoczi Signed-off-by: Liu Yuan Reviewed-by: Stefan Hajnoczi Reviewed-by: MORITA Kazutaka Signed-off-by: Kevin Wolf --- block/sheepdog.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/block/sheepdog.c b/block/sheepdog.c index 4245328569..54d3e53162 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -65,6 +65,7 @@ #define SD_RES_WAIT_FOR_FORMAT 0x16 /* Waiting for a format operation */ #define SD_RES_WAIT_FOR_JOIN 0x17 /* Waiting for other nodes joining */ #define SD_RES_JOIN_FAILED 0x18 /* Target node had failed to join sheepdog */ +#define SD_RES_HALT 0x19 /* Sheepdog is stopped serving IO request */ /* * Object ID rules @@ -344,6 +345,7 @@ static const char * sd_strerror(int err) {SD_RES_WAIT_FOR_FORMAT, "Sheepdog is waiting for a format operation"}, {SD_RES_WAIT_FOR_JOIN, "Sheepdog is waiting for other nodes joining"}, {SD_RES_JOIN_FAILED, "Target node had failed to join sheepdog"}, + {SD_RES_HALT, "Sheepdog is stopped serving IO request"}, }; for (i = 0; i < ARRAY_SIZE(errors); ++i) { From acdfb480ba7e2779bdbffb5280cf12ff6e43669c Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 18 Mar 2013 13:08:10 +0100 Subject: [PATCH 2/5] qcow2: Fix segfault in qcow2_invalidate_cache Need to pass an options QDict to qcow2_open() now. This fixes a segfault on the migration target with qcow2. Signed-off-by: Kevin Wolf --- block/qcow2.c | 12 ++++++++++-- block/qcow2.h | 3 +++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index 1f99866cf4..98bb7f31bf 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -29,6 +29,7 @@ #include "block/qcow2.h" #include "qemu/error-report.h" #include "qapi/qmp/qerror.h" +#include "qapi/qmp/qbool.h" #include "trace.h" /* @@ -520,7 +521,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags) goto fail; } - s->use_lazy_refcounts = qemu_opt_get_bool(opts, "lazy_refcounts", + s->use_lazy_refcounts = qemu_opt_get_bool(opts, QCOW2_OPT_LAZY_REFCOUNTS, (s->compatible_features & QCOW2_COMPAT_LAZY_REFCOUNTS)); qemu_opts_del(opts); @@ -930,6 +931,7 @@ static void qcow2_invalidate_cache(BlockDriverState *bs) AES_KEY aes_encrypt_key; AES_KEY aes_decrypt_key; uint32_t crypt_method = 0; + QDict *options; /* * Backing files are read-only which makes all of their metadata immutable, @@ -944,8 +946,14 @@ static void qcow2_invalidate_cache(BlockDriverState *bs) qcow2_close(bs); + options = qdict_new(); + qdict_put(options, QCOW2_OPT_LAZY_REFCOUNTS, + qbool_from_int(s->use_lazy_refcounts)); + memset(s, 0, sizeof(BDRVQcowState)); - qcow2_open(bs, NULL, flags); + qcow2_open(bs, options, flags); + + QDECREF(options); if (crypt_method) { s->crypt_method = crypt_method; diff --git a/block/qcow2.h b/block/qcow2.h index 103abdb2c0..e4b5e11a91 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -58,6 +58,9 @@ #define DEFAULT_CLUSTER_SIZE 65536 + +#define QCOW2_OPT_LAZY_REFCOUNTS "lazy_refcounts" + typedef struct QCowHeader { uint32_t magic; uint32_t version; From 4d70655bcb852ea0a006d3923f0b0a9c69ff462e Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 18 Mar 2013 17:58:53 +0100 Subject: [PATCH 3/5] block: fix BDRV_O_SNAPSHOT protocol detection realpath(3) is used to get an absolute path to the image file when creating a -drive snapshot=on temporary qcow2. This does not work for protocols since their filenames ("proto:foo:...") do not correspond to file system paths. Commit 7c96d46ec245d73fd76726588409f9abe4bd5dc1 ("Let snapshot work with protocols") skipped realpath(3) for protocols. Later on the "raw" format was introduced and broke the check. Use path_has_protocol(filename) to decide if this image uses a protocol or a filename. Reported-by: Richard Jones Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- block.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/block.c b/block.c index 037e15e065..0a062c9a7c 100644 --- a/block.c +++ b/block.c @@ -830,7 +830,6 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, if (flags & BDRV_O_SNAPSHOT) { BlockDriverState *bs1; int64_t total_size; - int is_protocol = 0; BlockDriver *bdrv_qcow2; QEMUOptionParameter *options; char backing_filename[PATH_MAX]; @@ -847,9 +846,6 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, } total_size = bdrv_getlength(bs1) & BDRV_SECTOR_MASK; - if (bs1->drv && bs1->drv->protocol_name) - is_protocol = 1; - bdrv_delete(bs1); ret = get_tmp_filename(tmp_filename, sizeof(tmp_filename)); @@ -858,7 +854,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options, } /* Real path is meaningless for protocols */ - if (is_protocol) { + if (path_has_protocol(filename)) { snprintf(backing_filename, sizeof(backing_filename), "%s", filename); } else if (!realpath(filename, backing_filename)) { From f95e26ddf526a025aa1334bbe527739397970443 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 18 Mar 2013 17:58:54 +0100 Subject: [PATCH 4/5] qemu-iotests: add 052 BDRV_O_SNAPSHOT test Check that writes to an image opened with BDRV_O_SNAPSHOT do not modify the underlying image file. Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- tests/qemu-iotests/052 | 61 ++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/052.out | 13 ++++++++ tests/qemu-iotests/group | 1 + 3 files changed, 75 insertions(+) create mode 100755 tests/qemu-iotests/052 create mode 100644 tests/qemu-iotests/052.out diff --git a/tests/qemu-iotests/052 b/tests/qemu-iotests/052 new file mode 100755 index 0000000000..14a5126635 --- /dev/null +++ b/tests/qemu-iotests/052 @@ -0,0 +1,61 @@ +#!/bin/bash +# +# Test bdrv_read/bdrv_write using BDRV_O_SNAPSHOT +# +# Copyright (C) 2013 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +# creator +owner=stefanha@redhat.com + +seq=`basename $0` +echo "QA output created by $seq" + +here=`pwd` +tmp=/tmp/$$ +status=1 # failure is the default! + +_cleanup() +{ + _cleanup_test_img +} +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter + +_supported_fmt generic +_supported_proto generic +_supported_os Linux + + +size=128M +_make_test_img $size + +echo +echo "== reading whole image ==" +$QEMU_IO -s -c "read 0 $size" $TEST_IMG | _filter_qemu_io + +echo +echo "== writing whole image does not modify image ==" +$QEMU_IO -s -c "write -P 0xa 0 $size" $TEST_IMG | _filter_qemu_io +$QEMU_IO -c "read -P 0 0 $size" $TEST_IMG | _filter_qemu_io + +# success, all done +echo "*** done" +rm -f $seq.full +status=0 diff --git a/tests/qemu-iotests/052.out b/tests/qemu-iotests/052.out new file mode 100644 index 0000000000..8617aa2a9e --- /dev/null +++ b/tests/qemu-iotests/052.out @@ -0,0 +1,13 @@ +QA output created by 052 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 + +== reading whole image == +read 134217728/134217728 bytes at offset 0 +128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +== writing whole image does not modify image == +wrote 134217728/134217728 bytes at offset 0 +128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 134217728/134217728 bytes at offset 0 +128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +*** done diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index 1d7e4f31a0..73c5966e10 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -57,3 +57,4 @@ 048 img auto quick 049 rw auto 050 rw auto backing quick +052 rw auto backing From a8e5cc0c076a6e3a62f0e9aad88b007dccf3dd17 Mon Sep 17 00:00:00 2001 From: Dunrong Huang Date: Tue, 19 Mar 2013 16:27:29 +0800 Subject: [PATCH 5/5] virtio-blk: Do not segfault fault if failed to initialize dataplane $ ~/usr/bin/qemu-system-x86_64 -enable-kvm -m 1024 -drive if=none,id=drive0,cache=none,aio=native,format=raw,file=/root/Image/centos-6.4.raw -device virtio-blk-pci,drive=drive0,scsi=off,x-data-plane=on,config-wce=on # make dataplane fail to initialize qemu-system-x86_64: -device virtio-blk-pci,drive=drive0,scsi=off,x-data-plane=on,config-wce=on: device is incompatible with x-data-plane, use config-wce=off *** glibc detected *** /root/usr/bin/qemu-system-x86_64: free(): invalid pointer: 0x00007f001fef12f8 *** ======= Backtrace: ========= /lib64/libc.so.6(+0x7d776)[0x7f00153a5776] /root/usr/bin/qemu-system-x86_64(+0x2c34ec)[0x7f001cf5b4ec] /root/usr/bin/qemu-system-x86_64(+0x342f9a)[0x7f001cfdaf9a] /root/usr/bin/qemu-system-x86_64(+0x33694e)[0x7f001cfce94e] .................... (gdb) bt #0 0x00007f3bf3a12015 in raise () from /lib64/libc.so.6 #1 0x00007f3bf3a1348b in abort () from /lib64/libc.so.6 #2 0x00007f3bf3a51a4e in __libc_message () from /lib64/libc.so.6 #3 0x00007f3bf3a57776 in malloc_printerr () from /lib64/libc.so.6 #4 0x00007f3bfb60d4ec in free_and_trace (mem=0x7f3bfe0129f8) at vl.c:2786 #5 0x00007f3bfb68cf9a in virtio_cleanup (vdev=0x7f3bfe0129f8) at /root/Develop/QEMU/qemu/hw/virtio.c:900 #6 0x00007f3bfb68094e in virtio_blk_device_init (vdev=0x7f3bfe0129f8) at /root/Develop/QEMU/qemu/hw/virtio-blk.c:666 #7 0x00007f3bfb68dadf in virtio_device_init (qdev=0x7f3bfe0129f8) at /root/Develop/QEMU/qemu/hw/virtio.c:1092 #8 0x00007f3bfb50da46 in device_realize (dev=0x7f3bfe0129f8, err=0x7fff479c9258) at hw/qdev.c:176 ............................. In virtio_blk_device_init(), the memory which vdev point to is a static member of "struct VirtIOBlkPCI", not heap memory, and it does not get freed. So we shoule use virtio_common_cleanup() to clean this VirtIODevice rather than virtio_cleanup(), which attempts to free the vdev. This error was introduced by commit 05ff686536f408ba6e8426b1b54d25bd3379fda2 recently. Signed-off-by: Dunrong Huang Signed-off-by: Kevin Wolf --- hw/virtio-blk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c index e6f8875d00..f2143fded3 100644 --- a/hw/virtio-blk.c +++ b/hw/virtio-blk.c @@ -663,7 +663,7 @@ static int virtio_blk_device_init(VirtIODevice *vdev) s->vq = virtio_add_queue(vdev, 128, virtio_blk_handle_output); #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE if (!virtio_blk_data_plane_create(vdev, blk, &s->dataplane)) { - virtio_cleanup(vdev); + virtio_common_cleanup(vdev); return -1; } #endif