[Qemu-devel] [PULL 0/4] Block patches
[PULL 0/2] Block patches
1
The following changes since commit 20d6c7312f1b812bb9c750f4087f69ac8485cc90:
1
The following changes since commit 67c1115edd98f388ca89dd38322ea3fadf034523:
2
2
3
Merge remote-tracking branch 'remotes/palmer/tags/riscv-for-master-3.2-part1' into staging (2019-01-03 13:26:30 +0000)
3
Merge remote-tracking branch 'remotes/kraxel/tags/ui-20210323-pull-request' into staging (2021-03-23 23:47:30 +0000)
4
4
5
are available in the Git repository at:
5
are available in the Git repository at:
6
6
7
git://github.com/stefanha/qemu.git tags/block-pull-request
7
https://gitlab.com/stefanha/qemu.git tags/block-pull-request
8
8
9
for you to fetch changes up to 39a0408e768cd00142f5b57d27ab234282bf4df5:
9
for you to fetch changes up to 3460fd7f3959d1fa7bcc255796844aa261c805a4:
10
10
11
dmg: don't skip zero chunk (2019-01-04 11:15:09 +0000)
11
migrate-bitmaps-postcopy-test: check that we can't remove in-flight bitmaps (2021-03-24 13:41:19 +0000)
12
12
13
----------------------------------------------------------------
13
----------------------------------------------------------------
14
Pull request
14
Pull request
15
15
16
Bug fixes for the .dmg image file format.
16
This dirty bitmap fix solves a crash that can be triggered in the destination
17
QEMU process during live migration.
17
18
18
----------------------------------------------------------------
19
----------------------------------------------------------------
19
20
20
Julio Faracco (1):
21
Vladimir Sementsov-Ogievskiy (2):
21
dmg: Fixing wrong dmg block type value for block terminator.
22
migration/block-dirty-bitmap: make incoming disabled bitmaps busy
23
migrate-bitmaps-postcopy-test: check that we can't remove in-flight
24
bitmaps
22
25
23
yuchenlin (3):
26
migration/block-dirty-bitmap.c | 6 ++++++
24
dmg: fix binary search
27
tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test | 10 ++++++++++
25
dmg: use enumeration type instead of hard coding number
28
2 files changed, 16 insertions(+)
26
dmg: don't skip zero chunk
27
28
block/dmg.c | 31 ++++++++++++++++++++-----------
29
1 file changed, 20 insertions(+), 11 deletions(-)
30
29
31
--
30
--
32
2.20.1
31
2.30.2
33
32
34
diff view generated by jsdifflib
[Qemu-devel] [PULL 1/4] dmg: Fixing wrong dmg block type value for block terminator.
Deleted patch
1
From: Julio Faracco <jcfaracco@gmail.com>
2
1
3
This is a trivial patch to fix a wrong value for block terminator.
4
The old value was 0x7fffffff which is wrong. It was not affecting the
5
code because QEMU dmg block is not handling block terminator right now.
6
Neverthless, it should be fixed.
7
8
Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
9
Reviewed-by: yuchenlin <yuchenlin@synology.com>
10
Message-id: 20181228145055.18039-1-jcfaracco@gmail.com
11
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
12
---
13
block/dmg.c | 2 +-
14
1 file changed, 1 insertion(+), 1 deletion(-)
15
16
diff --git a/block/dmg.c b/block/dmg.c
17
index XXXXXXX..XXXXXXX 100644
18
--- a/block/dmg.c
19
+++ b/block/dmg.c
20
@@ -XXX,XX +XXX,XX @@ enum {
21
UDBZ,
22
ULFO,
23
UDCM = 0x7ffffffe, /* Comments */
24
- UDLE /* Last Entry */
25
+ UDLE = 0xffffffff /* Last Entry */
26
};
27
28
static int dmg_probe(const uint8_t *buf, int buf_size, const char *filename)
29
--
30
2.20.1
31
32
diff view generated by jsdifflib
[Qemu-devel] [PULL 2/4] dmg: fix binary search
Deleted patch
1
From: yuchenlin <npes87184@gmail.com>
2
1
3
There is a possible hang in original binary search implementation. That is
4
if chunk1 = 4, chunk2 = 5, chunk3 = 4, and we go else case.
5
6
The chunk1 will be still 4, and so on.
7
8
Signed-off-by: yuchenlin <npes87184@gmail.com>
9
Message-id: 20190103114700.9686-2-npes87184@gmail.com
10
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
11
---
12
block/dmg.c | 10 +++++++---
13
1 file changed, 7 insertions(+), 3 deletions(-)
14
15
diff --git a/block/dmg.c b/block/dmg.c
16
index XXXXXXX..XXXXXXX 100644
17
--- a/block/dmg.c
18
+++ b/block/dmg.c
19
@@ -XXX,XX +XXX,XX @@ static inline uint32_t search_chunk(BDRVDMGState *s, uint64_t sector_num)
20
{
21
/* binary search */
22
uint32_t chunk1 = 0, chunk2 = s->n_chunks, chunk3;
23
- while (chunk1 != chunk2) {
24
+ while (chunk1 <= chunk2) {
25
chunk3 = (chunk1 + chunk2) / 2;
26
if (s->sectors[chunk3] > sector_num) {
27
- chunk2 = chunk3;
28
+ if (chunk3 == 0) {
29
+ goto err;
30
+ }
31
+ chunk2 = chunk3 - 1;
32
} else if (s->sectors[chunk3] + s->sectorcounts[chunk3] > sector_num) {
33
return chunk3;
34
} else {
35
- chunk1 = chunk3;
36
+ chunk1 = chunk3 + 1;
37
}
38
}
39
+err:
40
return s->n_chunks; /* error */
41
}
42
43
--
44
2.20.1
45
46
diff view generated by jsdifflib
[Qemu-devel] [PULL 4/4] dmg: don't skip zero chunk
[PULL 1/2] migration/block-dirty-bitmap: make incoming disabled bitmaps busy
1
From: yuchenlin <npes87184@gmail.com>
1
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
2
2
3
The dmg file has many tables which describe: "start from sector XXX to
3
Incoming enabled bitmaps are busy, because we do
4
sector XXX, the compression method is XXX and where the compressed data
4
bdrv_dirty_bitmap_create_successor() for them. But disabled bitmaps
5
resides on".
5
being migrated are not marked busy, and user can remove them during the
6
incoming migration. Then we may crash in cancel_incoming_locked() when
7
try to remove the bitmap that was already removed by user, like this:
6
8
7
Each sector in the expanded file should be covered by a table. The table
9
#0 qemu_mutex_lock_impl (mutex=0x5593d88c50d1, file=0x559680554b20
8
will describe the offset of compressed data (or raw depends on the type)
10
"../block/dirty-bitmap.c", line=64) at ../util/qemu-thread-posix.c:77
9
in the dmg.
11
#1 bdrv_dirty_bitmaps_lock (bs=0x5593d88c0ee9)
12
at ../block/dirty-bitmap.c:64
13
#2 bdrv_release_dirty_bitmap (bitmap=0x5596810e9570)
14
at ../block/dirty-bitmap.c:362
15
#3 cancel_incoming_locked (s=0x559680be8208 <dbm_state+40>)
16
at ../migration/block-dirty-bitmap.c:918
17
#4 dirty_bitmap_load (f=0x559681d02b10, opaque=0x559680be81e0
18
<dbm_state>, version_id=1) at ../migration/block-dirty-bitmap.c:1194
19
#5 vmstate_load (f=0x559681d02b10, se=0x559680fb5810)
20
at ../migration/savevm.c:908
21
#6 qemu_loadvm_section_part_end (f=0x559681d02b10,
22
mis=0x559680fb4a30) at ../migration/savevm.c:2473
23
#7 qemu_loadvm_state_main (f=0x559681d02b10, mis=0x559680fb4a30)
24
at ../migration/savevm.c:2626
25
#8 postcopy_ram_listen_thread (opaque=0x0)
26
at ../migration/savevm.c:1871
27
#9 qemu_thread_start (args=0x5596817ccd10)
28
at ../util/qemu-thread-posix.c:521
29
#10 start_thread () at /lib64/libpthread.so.0
30
#11 clone () at /lib64/libc.so.6
10
31
11
For example:
32
Note bs pointer taken from bitmap: it's definitely bad aligned. That's
33
because we are in use after free, bitmap is already freed.
12
34
13
[-----------The expanded file------------]
35
So, let's make disabled bitmaps (being migrated) busy during incoming
14
[---bzip table ---]/* zeros */[---zlib---]
36
migration.
15
^
16
| if we want to read this sector.
17
37
18
we will find bzip table which contains this sector, and get the
38
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
19
compressed data offset, read it from dmg, uncompress it, finally write to
39
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
20
expanded file.
40
Message-Id: <20210322094906.5079-2-vsementsov@virtuozzo.com>
41
---
42
migration/block-dirty-bitmap.c | 6 ++++++
43
1 file changed, 6 insertions(+)
21
44
22
If we skip zero chunk (table), some sector cannot find the table which
45
diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
23
will cause search_chunk() return s->n_chunks, dmg_read_chunk() return -1
46
index XXXXXXX..XXXXXXX 100644
24
and finally causing dmg_co_preadv() return EIO.
47
--- a/migration/block-dirty-bitmap.c
48
+++ b/migration/block-dirty-bitmap.c
49
@@ -XXX,XX +XXX,XX @@ static int dirty_bitmap_load_start(QEMUFile *f, DBMLoadState *s)
50
error_report_err(local_err);
51
return -EINVAL;
52
}
53
+ } else {
54
+ bdrv_dirty_bitmap_set_busy(s->bitmap, true);
55
}
56
57
b = g_new(LoadBitmapState, 1);
58
@@ -XXX,XX +XXX,XX @@ static void cancel_incoming_locked(DBMLoadState *s)
59
assert(!s->before_vm_start_handled || !b->migrated);
60
if (bdrv_dirty_bitmap_has_successor(b->bitmap)) {
61
bdrv_reclaim_dirty_bitmap(b->bitmap, &error_abort);
62
+ } else {
63
+ bdrv_dirty_bitmap_set_busy(b->bitmap, false);
64
}
65
bdrv_release_dirty_bitmap(b->bitmap);
66
}
67
@@ -XXX,XX +XXX,XX @@ static void dirty_bitmap_load_complete(QEMUFile *f, DBMLoadState *s)
68
69
if (bdrv_dirty_bitmap_has_successor(s->bitmap)) {
70
bdrv_reclaim_dirty_bitmap(s->bitmap, &error_abort);
71
+ } else {
72
+ bdrv_dirty_bitmap_set_busy(s->bitmap, false);
73
}
74
75
for (item = s->bitmaps; item; item = g_slist_next(item)) {
76
--
77
2.30.2
25
78
26
See:
27
28
[-----------The expanded file------------]
29
[---bzip table ---]/* zeros */[---zlib---]
30
^
31
| if we want to read this sector.
32
33
Oops, we cannot find the table contains it...
34
35
In the original implementation, we don't have zero table. When we try to
36
read sector inside the zero chunk. We will get EIO, and skip reading.
37
38
After this patch, we treat zero chunk the same as ignore chunk, it will
39
directly write zero and avoid some sector may not find the table.
40
41
After this patch:
42
43
[-----------The expanded file------------]
44
[---bzip table ---][--zeros--][---zlib---]
45
46
Signed-off-by: yuchenlin <npes87184@gmail.com>
47
Reviewed-by: Julio Faracco <jcfaracco@gmail.com>
48
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
49
Message-id: 20190103114700.9686-4-npes87184@gmail.com
50
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
51
---
52
block/dmg.c | 19 ++++++++++++-------
53
1 file changed, 12 insertions(+), 7 deletions(-)
54
55
diff --git a/block/dmg.c b/block/dmg.c
56
index XXXXXXX..XXXXXXX 100644
57
--- a/block/dmg.c
58
+++ b/block/dmg.c
59
@@ -XXX,XX +XXX,XX @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t chunk,
60
case UDRW: /* copy */
61
uncompressed_sectors = DIV_ROUND_UP(s->lengths[chunk], 512);
62
break;
63
- case UDIG: /* zero */
64
+ case UDZE: /* zero */
65
+ case UDIG: /* ignore */
66
/* as the all-zeroes block may be large, it is treated specially: the
67
* sector is not copied from a large buffer, a simple memset is used
68
* instead. Therefore uncompressed_sectors does not need to be set. */
69
@@ -XXX,XX +XXX,XX @@ typedef struct DmgHeaderState {
70
static bool dmg_is_known_block_type(uint32_t entry_type)
71
{
72
switch (entry_type) {
73
+ case UDZE: /* zeros */
74
case UDRW: /* uncompressed */
75
- case UDIG: /* zeroes */
76
+ case UDIG: /* ignore */
77
case UDZO: /* zlib */
78
return true;
79
case UDBZ: /* bzip2 */
80
@@ -XXX,XX +XXX,XX @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
81
/* sector count */
82
s->sectorcounts[i] = buff_read_uint64(buffer, offset + 0x10);
83
84
- /* all-zeroes sector (type 2) does not need to be "uncompressed" and can
85
- * therefore be unbounded. */
86
- if (s->types[i] != UDIG && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
87
+ /* all-zeroes sector (type UDZE and UDIG) does not need to be
88
+ * "uncompressed" and can therefore be unbounded. */
89
+ if (s->types[i] != UDZE && s->types[i] != UDIG
90
+ && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
91
error_report("sector count %" PRIu64 " for chunk %" PRIu32
92
" is larger than max (%u)",
93
s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX);
94
@@ -XXX,XX +XXX,XX @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num)
95
return -1;
96
}
97
break;
98
- case UDIG: /* zero */
99
+ case UDZE: /* zeros */
100
+ case UDIG: /* ignore */
101
/* see dmg_read, it is treated specially. No buffer needs to be
102
* pre-filled, the zeroes can be set directly. */
103
break;
104
@@ -XXX,XX +XXX,XX @@ dmg_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
105
/* Special case: current chunk is all zeroes. Do not perform a memcpy as
106
* s->uncompressed_chunk may be too small to cover the large all-zeroes
107
* section. dmg_read_chunk is called to find s->current_chunk */
108
- if (s->types[s->current_chunk] == UDIG) { /* all zeroes block entry */
109
+ if (s->types[s->current_chunk] == UDZE
110
+ || s->types[s->current_chunk] == UDIG) { /* all zeroes block entry */
111
qemu_iovec_memset(qiov, i * 512, 0, 512);
112
continue;
113
}
114
--
115
2.20.1
116
117
diff view generated by jsdifflib
[Qemu-devel] [PULL 3/4] dmg: use enumeration type instead of hard coding number
[PULL 2/2] migrate-bitmaps-postcopy-test: check that we can't remove in-flight bitmaps
1
From: yuchenlin <npes87184@gmail.com>
1
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
2
2
3
Signed-off-by: yuchenlin <npes87184@gmail.com>
3
Check that we can't remove bitmaps being migrated on destination vm.
4
Reviewed-by: Julio Faracco <jcfaracco@gmail.com>
4
The new check proves that previous commit helps.
5
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
5
6
Message-id: 20190103114700.9686-3-npes87184@gmail.com
6
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
7
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
7
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
8
Message-Id: <20210322094906.5079-3-vsementsov@virtuozzo.com>
8
---
9
---
9
block/dmg.c | 4 ++--
10
tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test | 10 ++++++++++
10
1 file changed, 2 insertions(+), 2 deletions(-)
11
1 file changed, 10 insertions(+)
11
12
12
diff --git a/block/dmg.c b/block/dmg.c
13
diff --git a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
13
index XXXXXXX..XXXXXXX 100644
14
index XXXXXXX..XXXXXXX 100755
14
--- a/block/dmg.c
15
--- a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
15
+++ b/block/dmg.c
16
+++ b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
16
@@ -XXX,XX +XXX,XX @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
17
@@ -XXX,XX +XXX,XX @@ class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase):
17
18
self.start_postcopy()
18
/* all-zeroes sector (type 2) does not need to be "uncompressed" and can
19
19
* therefore be unbounded. */
20
self.vm_b_events += self.vm_b.get_qmp_events()
20
- if (s->types[i] != 2 && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
21
+
21
+ if (s->types[i] != UDIG && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
22
+ # While being here, let's check that we can't remove in-flight bitmaps.
22
error_report("sector count %" PRIu64 " for chunk %" PRIu32
23
+ for vm in (self.vm_a, self.vm_b):
23
" is larger than max (%u)",
24
+ for i in range(0, nb_bitmaps):
24
s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX);
25
+ result = vm.qmp('block-dirty-bitmap-remove', node='drive0',
25
@@ -XXX,XX +XXX,XX @@ dmg_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
26
+ name=f'bitmap{i}')
26
/* Special case: current chunk is all zeroes. Do not perform a memcpy as
27
+ self.assert_qmp(result, 'error/desc',
27
* s->uncompressed_chunk may be too small to cover the large all-zeroes
28
+ f"Bitmap 'bitmap{i}' is currently in use by "
28
* section. dmg_read_chunk is called to find s->current_chunk */
29
+ "another operation and cannot be used")
29
- if (s->types[s->current_chunk] == 2) { /* all zeroes block entry */
30
+
30
+ if (s->types[s->current_chunk] == UDIG) { /* all zeroes block entry */
31
self.vm_b.shutdown()
31
qemu_iovec_memset(qiov, i * 512, 0, 512);
32
# recreate vm_b, so there is no incoming option, which prevents
32
continue;
33
# loading bitmaps from disk
33
}
34
--
34
--
35
2.20.1
35
2.30.2
36
36
37
diff view generated by jsdifflib

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.