Skip to content

Commit 03d1cbe

Browse files
committed
Merge remote-tracking branch 'remotes/maxreitz/tags/pull-block-2017-11-14' into staging
Block patches for 2.11.0-rc1 # gpg: Signature made Tue 14 Nov 2017 17:22:17 GMT # gpg: using RSA key 0xF407DB0061D5CF40 # gpg: Good signature from "Max Reitz <mreitz@redhat.com>" # Primary key fingerprint: 91BE B60A 30DB 3E88 57D1 1829 F407 DB00 61D5 CF40 * remotes/maxreitz/tags/pull-block-2017-11-14: qemu-iotests: update unsupported image formats in 194 block/parallels: add migration blocker block/parallels: Do not update header or truncate image when INMIGRATE block/vhdx.c: Don't blindly update the header iotests: 077: Filter out 'resume' lines block/snapshot: dirty all dirty bitmaps on snapshot-switch qcow2: Check that corrupted images can be repaired in iotest 060 iotests: Use new-style NBD connections iotests: Make 136 less flaky iotests: Make 083 less flaky iotests: Make 055 less flaky iotests: Add missing 'blkdebug::' in 040 iotests: Make 030 less flaky qcow2: Assert that the crypto header does not overlap other metadata qcow2: Add iotest for an empty refcount table qcow2: Add iotest for an image with header.refcount_table_offset == 0 qcow2: Don't open images with header.refcount_table_clusters == 0 qcow2: Prevent allocating compressed clusters at offset 0 qcow2: Prevent allocating L2 tables at offset 0 qcow2: Prevent allocating refcount blocks at offset 0 Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2 parents 29af6de + 8b2d7c3 commit 03d1cbe

File tree

17 files changed

+265
-45
lines changed

17 files changed

+265
-45
lines changed

block/parallels.c

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
#include "qemu/module.h"
3636
#include "qemu/bswap.h"
3737
#include "qemu/bitmap.h"
38+
#include "migration/blocker.h"
3839

3940
/**************************************************************/
4041

@@ -100,6 +101,7 @@ typedef struct BDRVParallelsState {
100101
unsigned int tracks;
101102

102103
unsigned int off_multiplier;
104+
Error *migration_blocker;
103105
} BDRVParallelsState;
104106

105107

@@ -708,7 +710,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
708710
s->prealloc_mode = PRL_PREALLOC_MODE_FALLOCATE;
709711
}
710712

711-
if (flags & BDRV_O_RDWR) {
713+
if ((flags & BDRV_O_RDWR) && !(flags & BDRV_O_INACTIVE)) {
712714
s->header->inuse = cpu_to_le32(HEADER_INUSE_MAGIC);
713715
ret = parallels_update_header(bs);
714716
if (ret < 0) {
@@ -720,6 +722,16 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
720722
s->bat_dirty_bmap =
721723
bitmap_new(DIV_ROUND_UP(s->header_size, s->bat_dirty_block));
722724

725+
/* Disable migration until bdrv_invalidate_cache method is added */
726+
error_setg(&s->migration_blocker, "The Parallels format used by node '%s' "
727+
"does not support live migration",
728+
bdrv_get_device_or_node_name(bs));
729+
ret = migrate_add_blocker(s->migration_blocker, &local_err);
730+
if (local_err) {
731+
error_propagate(errp, local_err);
732+
error_free(s->migration_blocker);
733+
goto fail;
734+
}
723735
qemu_co_mutex_init(&s->lock);
724736
return 0;
725737

@@ -741,18 +753,18 @@ static void parallels_close(BlockDriverState *bs)
741753
{
742754
BDRVParallelsState *s = bs->opaque;
743755

744-
if (bs->open_flags & BDRV_O_RDWR) {
756+
if ((bs->open_flags & BDRV_O_RDWR) && !(bs->open_flags & BDRV_O_INACTIVE)) {
745757
s->header->inuse = 0;
746758
parallels_update_header(bs);
747-
}
748-
749-
if (bs->open_flags & BDRV_O_RDWR) {
750759
bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS,
751760
PREALLOC_MODE_OFF, NULL);
752761
}
753762

754763
g_free(s->bat_dirty_bmap);
755764
qemu_vfree(s->header);
765+
766+
migrate_del_blocker(s->migration_blocker);
767+
error_free(s->migration_blocker);
756768
}
757769

758770
static QemuOptsList parallels_create_opts = {

block/qcow2-cluster.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,14 @@ static int l2_allocate(BlockDriverState *bs, int l1_index, uint64_t **table)
278278
goto fail;
279279
}
280280

281+
/* If we're allocating the table at offset 0 then something is wrong */
282+
if (l2_offset == 0) {
283+
qcow2_signal_corruption(bs, true, -1, -1, "Preventing invalid "
284+
"allocation of L2 table at offset 0");
285+
ret = -EIO;
286+
goto fail;
287+
}
288+
281289
ret = qcow2_cache_flush(bs, s->refcount_block_cache);
282290
if (ret < 0) {
283291
goto fail;

block/qcow2-refcount.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,13 @@ static int alloc_refcount_block(BlockDriverState *bs,
367367
return new_block;
368368
}
369369

370+
/* If we're allocating the block at offset 0 then something is wrong */
371+
if (new_block == 0) {
372+
qcow2_signal_corruption(bs, true, -1, -1, "Preventing invalid "
373+
"allocation of refcount block at offset 0");
374+
return -EIO;
375+
}
376+
370377
#ifdef DEBUG_ALLOC2
371378
fprintf(stderr, "qcow2: Allocate refcount block %d for %" PRIx64
372379
" at %" PRIx64 "\n",
@@ -1075,6 +1082,13 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
10751082
return new_cluster;
10761083
}
10771084

1085+
if (new_cluster == 0) {
1086+
qcow2_signal_corruption(bs, true, -1, -1, "Preventing invalid "
1087+
"allocation of compressed cluster "
1088+
"at offset 0");
1089+
return -EIO;
1090+
}
1091+
10781092
if (!offset || ROUND_UP(offset, s->cluster_size) != new_cluster) {
10791093
offset = new_cluster;
10801094
free_in_cluster = s->cluster_size;

block/qcow2.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ static ssize_t qcow2_crypto_hdr_init_func(QCryptoBlock *block, size_t headerlen,
126126
/* Zero fill remaining space in cluster so it has predictable
127127
* content in case of future spec changes */
128128
clusterlen = size_to_clusters(s, headerlen) * s->cluster_size;
129+
assert(qcow2_pre_write_overlap_check(bs, 0, ret, clusterlen) == 0);
129130
ret = bdrv_pwrite_zeroes(bs->file,
130131
ret + headerlen,
131132
clusterlen - headerlen, 0);
@@ -1280,6 +1281,12 @@ static int qcow2_do_open(BlockDriverState *bs, QDict *options, int flags,
12801281
goto fail;
12811282
}
12821283

1284+
if (header.refcount_table_clusters == 0 && !(flags & BDRV_O_CHECK)) {
1285+
error_setg(errp, "Image does not contain a reference count table");
1286+
ret = -EINVAL;
1287+
goto fail;
1288+
}
1289+
12831290
ret = validate_table_offset(bs, s->refcount_table_offset,
12841291
s->refcount_table_size, sizeof(uint64_t));
12851292
if (ret < 0) {

block/snapshot.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,10 +181,24 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
181181
{
182182
BlockDriver *drv = bs->drv;
183183
int ret, open_ret;
184+
int64_t len;
184185

185186
if (!drv) {
186187
return -ENOMEDIUM;
187188
}
189+
190+
len = bdrv_getlength(bs);
191+
if (len < 0) {
192+
return len;
193+
}
194+
/* We should set all bits in all enabled dirty bitmaps, because dirty
195+
* bitmaps reflect active state of disk and snapshot switch operation
196+
* actually dirties active state.
197+
* TODO: It may make sense not to set all bits but analyze block status of
198+
* current state and destination snapshot and do not set bits corresponding
199+
* to both-zero or both-unallocated areas. */
200+
bdrv_set_dirty(bs, 0, len);
201+
188202
if (drv->bdrv_snapshot_goto) {
189203
return drv->bdrv_snapshot_goto(bs, snapshot_id);
190204
}

block/vhdx.c

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,13 +1008,6 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
10081008
goto fail;
10091009
}
10101010

1011-
if (flags & BDRV_O_RDWR) {
1012-
ret = vhdx_update_headers(bs, s, false, NULL);
1013-
if (ret < 0) {
1014-
goto fail;
1015-
}
1016-
}
1017-
10181011
/* TODO: differencing files */
10191012

10201013
return 0;

tests/qemu-iotests/030

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -666,6 +666,7 @@ class TestENOSPC(TestErrors):
666666
if event['event'] == 'BLOCK_JOB_ERROR':
667667
self.assert_qmp(event, 'data/device', 'drive0')
668668
self.assert_qmp(event, 'data/operation', 'read')
669+
error = True
669670

670671
result = self.vm.qmp('query-block-jobs')
671672
self.assert_qmp(result, 'return[0]/paused', True)
@@ -676,9 +677,11 @@ class TestENOSPC(TestErrors):
676677
self.assert_qmp(result, 'return', {})
677678

678679
result = self.vm.qmp('query-block-jobs')
680+
if result == {'return': []}:
681+
# Race; likely already finished. Check.
682+
continue
679683
self.assert_qmp(result, 'return[0]/paused', False)
680684
self.assert_qmp(result, 'return[0]/io-status', 'ok')
681-
error = True
682685
elif event['event'] == 'BLOCK_JOB_COMPLETED':
683686
self.assertTrue(error, 'job completed unexpectedly')
684687
self.assert_qmp(event, 'data/type', 'stream')
@@ -792,13 +795,14 @@ class TestSetSpeed(iotests.QMPTestCase):
792795

793796
self.assert_no_active_block_jobs()
794797

798+
self.vm.pause_drive('drive0')
795799
result = self.vm.qmp('block-stream', device='drive0')
796800
self.assert_qmp(result, 'return', {})
797801

798802
result = self.vm.qmp('block-job-set-speed', device='drive0', speed=-1)
799803
self.assert_qmp(result, 'error/class', 'GenericError')
800804

801-
self.cancel_and_wait()
805+
self.cancel_and_wait(resume=True)
802806

803807
if __name__ == '__main__':
804808
iotests.main(supported_fmts=['qcow2', 'qed'])

tests/qemu-iotests/040

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ class TestSetSpeed(ImageCommitTestCase):
289289
qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % mid_img, test_img)
290290
qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0x1 0 512', test_img)
291291
qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0xef 524288 524288', mid_img)
292-
self.vm = iotests.VM().add_drive(test_img)
292+
self.vm = iotests.VM().add_drive('blkdebug::' + test_img)
293293
self.vm.launch()
294294

295295
def tearDown(self):

tests/qemu-iotests/055

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ class TestSingleDrive(iotests.QMPTestCase):
4848
def setUp(self):
4949
qemu_img('create', '-f', iotests.imgfmt, blockdev_target_img, str(image_len))
5050

51-
self.vm = iotests.VM().add_drive(test_img)
51+
self.vm = iotests.VM().add_drive('blkdebug::' + test_img)
5252
self.vm.add_drive(blockdev_target_img, interface="none")
5353
if iotests.qemu_default_machine == 'pc':
5454
self.vm.add_drive(None, 'media=cdrom', 'ide')
@@ -65,10 +65,11 @@ class TestSingleDrive(iotests.QMPTestCase):
6565
def do_test_cancel(self, cmd, target):
6666
self.assert_no_active_block_jobs()
6767

68+
self.vm.pause_drive('drive0')
6869
result = self.vm.qmp(cmd, device='drive0', target=target, sync='full')
6970
self.assert_qmp(result, 'return', {})
7071

71-
event = self.cancel_and_wait()
72+
event = self.cancel_and_wait(resume=True)
7273
self.assert_qmp(event, 'data/type', 'backup')
7374

7475
def test_cancel_drive_backup(self):
@@ -166,7 +167,7 @@ class TestSetSpeed(iotests.QMPTestCase):
166167
def setUp(self):
167168
qemu_img('create', '-f', iotests.imgfmt, blockdev_target_img, str(image_len))
168169

169-
self.vm = iotests.VM().add_drive(test_img)
170+
self.vm = iotests.VM().add_drive('blkdebug::' + test_img)
170171
self.vm.add_drive(blockdev_target_img, interface="none")
171172
self.vm.launch()
172173

@@ -246,6 +247,8 @@ class TestSetSpeed(iotests.QMPTestCase):
246247
def test_set_speed_invalid_blockdev_backup(self):
247248
self.do_test_set_speed_invalid('blockdev-backup', 'drive1')
248249

250+
# Note: We cannot use pause_drive() here, or the transaction command
251+
# would stall. Instead, we limit the block job speed here.
249252
class TestSingleTransaction(iotests.QMPTestCase):
250253
def setUp(self):
251254
qemu_img('create', '-f', iotests.imgfmt, blockdev_target_img, str(image_len))
@@ -271,7 +274,8 @@ class TestSingleTransaction(iotests.QMPTestCase):
271274
'type': cmd,
272275
'data': { 'device': 'drive0',
273276
'target': target,
274-
'sync': 'full' },
277+
'sync': 'full',
278+
'speed': 64 * 1024 },
275279
}
276280
])
277281

@@ -289,20 +293,22 @@ class TestSingleTransaction(iotests.QMPTestCase):
289293
def do_test_pause(self, cmd, target, image):
290294
self.assert_no_active_block_jobs()
291295

292-
self.vm.pause_drive('drive0')
293296
result = self.vm.qmp('transaction', actions=[{
294297
'type': cmd,
295298
'data': { 'device': 'drive0',
296299
'target': target,
297-
'sync': 'full' },
300+
'sync': 'full',
301+
'speed': 64 * 1024 },
298302
}
299303
])
300304
self.assert_qmp(result, 'return', {})
301305

302306
result = self.vm.qmp('block-job-pause', device='drive0')
303307
self.assert_qmp(result, 'return', {})
304308

305-
self.vm.resume_drive('drive0')
309+
result = self.vm.qmp('block-job-set-speed', device='drive0', speed=0)
310+
self.assert_qmp(result, 'return', {})
311+
306312
self.pause_job('drive0')
307313

308314
result = self.vm.qmp('query-block-jobs')
@@ -461,7 +467,7 @@ class TestDriveCompression(iotests.QMPTestCase):
461467
pass
462468

463469
def do_prepare_drives(self, fmt, args, attach_target):
464-
self.vm = iotests.VM().add_drive(test_img)
470+
self.vm = iotests.VM().add_drive('blkdebug::' + test_img)
465471

466472
qemu_img('create', '-f', fmt, blockdev_target_img,
467473
str(TestDriveCompression.image_len), *args)
@@ -500,10 +506,11 @@ class TestDriveCompression(iotests.QMPTestCase):
500506

501507
self.assert_no_active_block_jobs()
502508

509+
self.vm.pause_drive('drive0')
503510
result = self.vm.qmp(cmd, device='drive0', sync='full', compress=True, **args)
504511
self.assert_qmp(result, 'return', {})
505512

506-
event = self.cancel_and_wait()
513+
event = self.cancel_and_wait(resume=True)
507514
self.assert_qmp(event, 'data/type', 'backup')
508515

509516
self.vm.shutdown()

tests/qemu-iotests/060

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,65 @@ poke_file "$TEST_IMG" "$(($l2_offset+8))" "\x80\x00\x00\x00\x00\x06\x2a\x00"
242242
# Should emit two error messages
243243
$QEMU_IO -c "discard 0 64k" -c "read 64k 64k" "$TEST_IMG" | _filter_qemu_io
244244

245+
echo
246+
echo "=== Testing empty refcount table ==="
247+
echo
248+
_make_test_img 64M
249+
poke_file "$TEST_IMG" "$rt_offset" "\x00\x00\x00\x00\x00\x00\x00\x00"
250+
$QEMU_IO -c "write 0 64k" "$TEST_IMG" | _filter_qemu_io
251+
# Repair the image
252+
_check_test_img -r all
253+
254+
echo
255+
echo "=== Testing empty refcount table with valid L1 and L2 tables ==="
256+
echo
257+
_make_test_img 64M
258+
$QEMU_IO -c "write 0 64k" "$TEST_IMG" | _filter_qemu_io
259+
poke_file "$TEST_IMG" "$rt_offset" "\x00\x00\x00\x00\x00\x00\x00\x00"
260+
# Since the first data cluster is already allocated this triggers an
261+
# allocation with an explicit offset (using qcow2_alloc_clusters_at())
262+
# causing a refcount block to be allocated at offset 0
263+
$QEMU_IO -c "write 0 128k" "$TEST_IMG" | _filter_qemu_io
264+
# Repair the image
265+
_check_test_img -r all
266+
267+
echo
268+
echo "=== Testing empty refcount block ==="
269+
echo
270+
_make_test_img 64M
271+
poke_file "$TEST_IMG" "$rb_offset" "\x00\x00\x00\x00\x00\x00\x00\x00"
272+
$QEMU_IO -c "write 0 64k" "$TEST_IMG" | _filter_qemu_io
273+
# Repair the image
274+
_check_test_img -r all
275+
276+
echo
277+
echo "=== Testing empty refcount block with compressed write ==="
278+
echo
279+
_make_test_img 64M
280+
$QEMU_IO -c "write 64k 64k" "$TEST_IMG" | _filter_qemu_io
281+
poke_file "$TEST_IMG" "$rb_offset" "\x00\x00\x00\x00\x00\x00\x00\x00"
282+
# The previous write already allocated an L2 table, so now this new
283+
# write will try to allocate a compressed data cluster at offset 0.
284+
$QEMU_IO -c "write -c 0k 64k" "$TEST_IMG" | _filter_qemu_io
285+
# Repair the image
286+
_check_test_img -r all
287+
288+
echo
289+
echo "=== Testing zero refcount table size ==="
290+
echo
291+
_make_test_img 64M
292+
poke_file "$TEST_IMG" "56" "\x00\x00\x00\x00"
293+
$QEMU_IO -c "write 0 64k" "$TEST_IMG" 2>&1 | _filter_testdir | _filter_imgfmt
294+
# Repair the image
295+
_check_test_img -r all
296+
297+
echo
298+
echo "=== Testing incorrect refcount table offset ==="
299+
echo
300+
_make_test_img 64M
301+
poke_file "$TEST_IMG" "48" "\x00\x00\x00\x00\x00\x00\x00\x00"
302+
$QEMU_IO -c "write 0 64k" "$TEST_IMG" | _filter_qemu_io
303+
245304
# success, all done
246305
echo "*** done"
247306
rm -f $seq.full

0 commit comments

Comments
 (0)