| Message ID | 20231228101232.372142-12-alexander.ivanov@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | parallels: Add full dirty bitmap support | expand |
On 12/28/23 11:12, Alexander Ivanov wrote: > Now dirty bitmaps can be loaded but there is no their saving. Add code for > dirty bitmap storage. > > Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> > --- > block/parallels-ext.c | 168 ++++++++++++++++++++++++++++++++++++++++++ > block/parallels.c | 16 +++- > block/parallels.h | 5 ++ > 3 files changed, 187 insertions(+), 2 deletions(-) > > diff --git a/block/parallels-ext.c b/block/parallels-ext.c > index b4e14c88f2..c83d1ea393 100644 > --- a/block/parallels-ext.c > +++ b/block/parallels-ext.c > @@ -24,6 +24,7 @@ > */ > > #include "qemu/osdep.h" > +#include "qemu/error-report.h" > #include "qapi/error.h" > #include "block/block-io.h" > #include "block/block_int.h" > @@ -300,3 +301,170 @@ out: > > return ret; > } > + > +static void GRAPH_RDLOCK parallels_save_bitmap(BlockDriverState *bs, > + BdrvDirtyBitmap *bitmap, > + uint8_t **buf, int *buf_size) Do we need a error? > +{ > + BDRVParallelsState *s = bs->opaque; > + ParallelsFeatureHeader *fh; > + ParallelsDirtyBitmapFeature *bh; > + uint64_t *l1_table, l1_size, granularity, limit; I would say that 'limit' here means 'bits_in_cluster' We are writing the new code and I would prefer if we would have bits, bytes, clusters, sectors etc are clearly seen in variable names. It is quite complex to track various measurables. > + int64_t bm_size, ser_size, offset, buf_used; > + int64_t alloc_size = 1; > + const char *name; > + uint8_t *bm_buf; > + QemuUUID uuid; > + int ret = 0; > + > + if (!bdrv_dirty_bitmap_get_persistence(bitmap) || > + bdrv_dirty_bitmap_inconsistent(bitmap)) { > + return; > + } > + > + bm_size = bdrv_dirty_bitmap_size(bitmap); > + granularity = bdrv_dirty_bitmap_granularity(bitmap); > + limit = bdrv_dirty_bitmap_serialization_coverage(s->cluster_size, bitmap); > + ser_size = bdrv_dirty_bitmap_serialization_size(bitmap, 0, bm_size); > + l1_size = DIV_ROUND_UP(ser_size, s->cluster_size); > + > + buf_used = l1_size * 8 + sizeof(*fh) + sizeof(*bh); As far as I can see, bdrv_dirty_bitmap_serialization_size() returns bytes. That is correct. Thus multiplying it by 8 seems fatal mistake. I am also quite unsure that we should roundup to the cluster, that will occupy more clusters than needed. Can you please take a look here https://src.openvz.org/users/ibazhitov/repos/ploop/browse/tools/ploop-cbt.c > + /* Check if there is enough space for the final section */ > + if (*buf_size - buf_used < sizeof(*fh)) { > + return; > + } > + > + name = bdrv_dirty_bitmap_name(bitmap); > + ret = qemu_uuid_parse(name, &uuid); > + if (ret < 0) { > + error_report("Can't save dirty bitmap: ID parsing error: '%s'", name); > + return; > + } > + > + fh = (ParallelsFeatureHeader *)*buf; > + bh = (ParallelsDirtyBitmapFeature *)(*buf + sizeof(*fh)); bh = fh + 1 ? > + l1_table = (uint64_t *)((uint8_t *)bh + sizeof(*bh)); l1_table = bh + 1 ? > + > + fh->magic = cpu_to_le64(PARALLELS_DIRTY_BITMAP_FEATURE_MAGIC); > + fh->data_size = cpu_to_le32(l1_size * 8 + sizeof(*bh)); I am quite concerned here. Please compare with int save_dirty_bitmap(int devfd, struct delta *delta, off_t offset, void *buf, __u32 *size, void *or_data, writer_fn wr, void *data) { int ret = 0; struct ploop_pvd_header *vh; size_t block_size; __u64 bits, bytes, *p; __u32 byte_granularity; void *block; struct ploop_pvd_dirty_bitmap_raw *raw = (struct ploop_pvd_dirty_bitmap_raw *)buf; char x[50]; vh = (struct ploop_pvd_header *)delta->hdr0; /* granularity and uuid */ if ((ret = cbt_get_dirty_bitmap_metadata(devfd, raw->m_Id, &raw->m_Granularity))) return ret; raw->m_Granularity /= SECTOR_SIZE; block_size = vh->m_Sectors * SECTOR_SIZE; if (p_memalign((void **)&block, 4096, block_size)) return SYSEXIT_MALLOC; raw->m_Size = vh->m_SizeInSectors_v2; byte_granularity = raw->m_Granularity * SECTOR_SIZE; bits = ((raw->m_Size + raw->m_Granularity - 1) / raw->m_Granularity); bytes = (bits + 7) >> 3; raw->m_L1Size = (bytes + block_size - 1) / block_size; which means that the header is rotten. In that case can you pls take a look why this has not been caught by tests? > + > + bh->l1_size = cpu_to_le32(l1_size); > + bh->size = cpu_to_le64(bm_size >> BDRV_SECTOR_BITS); > + bh->granularity = cpu_to_le32(granularity >> BDRV_SECTOR_BITS); > + memcpy(bh->id, &uuid, sizeof(uuid)); > + > + bm_buf = qemu_blockalign(bs, s->cluster_size); > + > + offset = 0; > + while ((offset = bdrv_dirty_bitmap_next_dirty(bitmap, offset, bm_size)) >= 0) { > + uint64_t idx = offset / limit; > + int64_t cluster_off, end, write_size; > + > + offset = QEMU_ALIGN_DOWN(offset, limit); > + end = MIN(bm_size, offset + limit); > + write_size = bdrv_dirty_bitmap_serialization_size(bitmap, offset, > + end - offset); > + assert(write_size <= s->cluster_size); > + > + bdrv_dirty_bitmap_serialize_part(bitmap, bm_buf, offset, end - offset); > + if (write_size < s->cluster_size) { > + memset(bm_buf + write_size, 0, s->cluster_size - write_size); > + } > + > + cluster_off = parallels_allocate_host_clusters(bs, &alloc_size); > + if (cluster_off <= 0) { > + goto end; > + } > + > + ret = bdrv_pwrite(bs->file, cluster_off, s->cluster_size, bm_buf, 0); > + if (ret < 0) { > + memset(&fh->magic, 0, sizeof(fh->magic)); > + parallels_mark_unused(bs, s->used_bmap, s->used_bmap_size, > + cluster_off, 1); That is incomplete. You have to clean all clusters inside of the extension. > + goto end; > + } > + > + l1_table[idx] = cpu_to_le64(cluster_off >> BDRV_SECTOR_BITS); > + offset = end; > + } > + > + *buf_size -= buf_used; > + *buf += buf_used; > + > +end: > + qemu_vfree(bm_buf); > +} > + > +void GRAPH_RDLOCK > +parallels_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp) > +{ > + BDRVParallelsState *s = bs->opaque; > + BdrvDirtyBitmap *bitmap; > + ParallelsFormatExtensionHeader *eh; > + int remaining = s->cluster_size; > + uint8_t *buf, *pos; > + int64_t header_off, alloc_size = 1; > + g_autofree uint8_t *hash = NULL; > + size_t hash_len = 0; > + int ret; > + > + s->header->ext_off = 0; > + > + if (!bdrv_has_named_bitmaps(bs)) { > + return; > + } > + > + buf = qemu_blockalign0(bs, s->cluster_size); > + > + eh = (ParallelsFormatExtensionHeader *)buf; > + pos = buf + sizeof(*eh); > + > + eh->magic = cpu_to_le64(PARALLELS_FORMAT_EXTENSION_MAGIC); > + > + FOR_EACH_DIRTY_BITMAP(bs, bitmap) { > + parallels_save_bitmap(bs, bitmap, &pos, &remaining); > + } > + > + header_off = parallels_allocate_host_clusters(bs, &alloc_size); > + if (header_off < 0) { > + error_report("Can't save dirty bitmap: cluster allocation error"); > + ret = header_off; > + goto end; > + } > + > + ret = qcrypto_hash_bytes(QCRYPTO_HASH_ALG_MD5, > + (const char *)(buf + sizeof(*eh)), > + s->cluster_size - sizeof(*eh), > + &hash, &hash_len, errp); > + if (ret < 0 || hash_len != sizeof(eh->check_sum)) { > + error_report("Can't save dirty bitmap: hash error"); > + ret = -EINVAL; > + goto end; > + } > + memcpy(eh->check_sum, hash, hash_len); > + > + ret = bdrv_pwrite(bs->file, header_off, s->cluster_size, buf, 0); > + if (ret < 0) { > + error_report("Can't save dirty bitmap: IO error"); > + parallels_mark_unused(bs, s->used_bmap, s->used_bmap_size, > + header_off, 1); > + goto end; > + } > + > + s->header->ext_off = cpu_to_le64(header_off / BDRV_SECTOR_SIZE); > +end: > + qemu_vfree(buf); > +} > + > +bool coroutine_fn parallels_co_can_store_new_dirty_bitmap(BlockDriverState *bs, > + const char *name, > + uint32_t granularity, > + Error **errp) > +{ > + if (bdrv_find_dirty_bitmap(bs, name)) { > + error_setg(errp, "Bitmap already exists: %s", name); > + return false; > + } > + > + return true; > +} > diff --git a/block/parallels.c b/block/parallels.c > index f38dd2309c..a49922c6a7 100644 > --- a/block/parallels.c > +++ b/block/parallels.c > @@ -1466,14 +1466,25 @@ fail: > static int GRAPH_RDLOCK parallels_inactivate(BlockDriverState *bs) > { > BDRVParallelsState *s = bs->opaque; > + Error *err = NULL; > int ret; > > + parallels_store_persistent_dirty_bitmaps(bs, &err); > + if (err != NULL) { For me this looks quite odd, indirect check for error. Not good > + error_reportf_err(err, "Lost persistent bitmaps during " > + "inactivation of node '%s': ", > + bdrv_get_device_or_node_name(bs)); > + } > + > s->header->inuse = 0; > parallels_update_header(bs); > > /* errors are ignored, so we might as well pass exact=true */ > - ret = bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, true, > - PREALLOC_MODE_OFF, 0, NULL); > + ret = bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, > + true, PREALLOC_MODE_OFF, 0, NULL); > + if (ret < 0) { > + error_report("Failed to truncate image: %s", strerror(-ret)); > + } > > return ret; > } > @@ -1525,6 +1536,7 @@ static BlockDriver bdrv_parallels = { > .bdrv_co_pdiscard = parallels_co_pdiscard, > .bdrv_co_pwrite_zeroes = parallels_co_pwrite_zeroes, > .bdrv_inactivate = parallels_inactivate, > + .bdrv_co_can_store_new_dirty_bitmap = parallels_co_can_store_new_dirty_bitmap, > }; > > static void bdrv_parallels_init(void) > diff --git a/block/parallels.h b/block/parallels.h > index 493c89e976..9db4f5c908 100644 > --- a/block/parallels.h > +++ b/block/parallels.h > @@ -101,5 +101,10 @@ int64_t GRAPH_RDLOCK parallels_allocate_host_clusters(BlockDriverState *bs, > int GRAPH_RDLOCK > parallels_read_format_extension(BlockDriverState *bs, int64_t ext_off, > Error **errp); > +void GRAPH_RDLOCK > +parallels_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp); > +bool coroutine_fn > +parallels_co_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name, > + uint32_t granularity, Error **errp); > > #endif
On 1/18/24 14:27, Denis V. Lunev wrote: > On 12/28/23 11:12, Alexander Ivanov wrote: >> Now dirty bitmaps can be loaded but there is no their saving. Add >> code for >> dirty bitmap storage. >> >> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> >> --- >> block/parallels-ext.c | 168 ++++++++++++++++++++++++++++++++++++++++++ >> block/parallels.c | 16 +++- >> block/parallels.h | 5 ++ >> 3 files changed, 187 insertions(+), 2 deletions(-) >> >> diff --git a/block/parallels-ext.c b/block/parallels-ext.c >> index b4e14c88f2..c83d1ea393 100644 >> --- a/block/parallels-ext.c >> +++ b/block/parallels-ext.c >> @@ -24,6 +24,7 @@ >> */ >> #include "qemu/osdep.h" >> +#include "qemu/error-report.h" >> #include "qapi/error.h" >> #include "block/block-io.h" >> #include "block/block_int.h" >> @@ -300,3 +301,170 @@ out: >> return ret; >> } >> + >> +static void GRAPH_RDLOCK parallels_save_bitmap(BlockDriverState *bs, >> + BdrvDirtyBitmap *bitmap, >> + uint8_t **buf, int >> *buf_size) > Do we need a error? I think no. We save bitmaps in a loop. If some bitmap has a problem it would be better just to print an error message and try to save other bitmaps. > >> +{ >> + BDRVParallelsState *s = bs->opaque; >> + ParallelsFeatureHeader *fh; >> + ParallelsDirtyBitmapFeature *bh; >> + uint64_t *l1_table, l1_size, granularity, limit; > I would say that 'limit' here means 'bits_in_cluster' > > We are writing the new code and I would prefer if we > would have bits, bytes, clusters, sectors etc are > clearly seen in variable names. It is quite complex > to track various measurables. OK >> + int64_t bm_size, ser_size, offset, buf_used; >> + int64_t alloc_size = 1; >> + const char *name; >> + uint8_t *bm_buf; >> + QemuUUID uuid; >> + int ret = 0; >> + >> + if (!bdrv_dirty_bitmap_get_persistence(bitmap) || >> + bdrv_dirty_bitmap_inconsistent(bitmap)) { >> + return; >> + } >> + >> + bm_size = bdrv_dirty_bitmap_size(bitmap); >> + granularity = bdrv_dirty_bitmap_granularity(bitmap); >> + limit = >> bdrv_dirty_bitmap_serialization_coverage(s->cluster_size, bitmap); >> + ser_size = bdrv_dirty_bitmap_serialization_size(bitmap, 0, >> bm_size); >> + l1_size = DIV_ROUND_UP(ser_size, s->cluster_size); >> + >> + buf_used = l1_size * 8 + sizeof(*fh) + sizeof(*bh); > As far as I can see, bdrv_dirty_bitmap_serialization_size() returns > bytes. > That is correct. Thus multiplying it by 8 seems fatal mistake. l1_size is amount of entries in l1_table. Every entry has 8 bytes size and corresponds to one cluster containing a part of a bitmap. > I am also quite unsure that we should roundup to the cluster, that > will occupy more clusters than needed. Can you please take a look > here > https://src.openvz.org/users/ibazhitov/repos/ploop/browse/tools/ploop-cbt.c > Why shouldn't we? We need to have l1_table which should be able to contain all the bitmap data. Note: there is not a ROUND_UP(), but DIV_ROUND_UP(). >> + /* Check if there is enough space for the final section */ >> + if (*buf_size - buf_used < sizeof(*fh)) { >> + return; >> + } >> + >> + name = bdrv_dirty_bitmap_name(bitmap); >> + ret = qemu_uuid_parse(name, &uuid); >> + if (ret < 0) { >> + error_report("Can't save dirty bitmap: ID parsing error: >> '%s'", name); >> + return; >> + } >> + >> + fh = (ParallelsFeatureHeader *)*buf; >> + bh = (ParallelsDirtyBitmapFeature *)(*buf + sizeof(*fh)); > bh = fh + 1 ? >> + l1_table = (uint64_t *)((uint8_t *)bh + sizeof(*bh)); > l1_table = bh + 1 ? Yes, it's better. >> + >> + fh->magic = cpu_to_le64(PARALLELS_DIRTY_BITMAP_FEATURE_MAGIC); >> + fh->data_size = cpu_to_le32(l1_size * 8 + sizeof(*bh)); > I am quite concerned here. Please compare with > > int save_dirty_bitmap(int devfd, struct delta *delta, off_t offset, > void *buf, __u32 *size, void *or_data, writer_fn wr, > void *data) > { > int ret = 0; > struct ploop_pvd_header *vh; > size_t block_size; > __u64 bits, bytes, *p; > __u32 byte_granularity; > void *block; > struct ploop_pvd_dirty_bitmap_raw *raw = (struct > ploop_pvd_dirty_bitmap_raw *)buf; > char x[50]; > > vh = (struct ploop_pvd_header *)delta->hdr0; > > /* granularity and uuid */ > if ((ret = cbt_get_dirty_bitmap_metadata(devfd, raw->m_Id, > &raw->m_Granularity))) > return ret; > raw->m_Granularity /= SECTOR_SIZE; > > block_size = vh->m_Sectors * SECTOR_SIZE; > if (p_memalign((void **)&block, 4096, block_size)) > return SYSEXIT_MALLOC; > > raw->m_Size = vh->m_SizeInSectors_v2; > > byte_granularity = raw->m_Granularity * SECTOR_SIZE; > bits = ((raw->m_Size + raw->m_Granularity - 1) / raw->m_Granularity); > bytes = (bits + 7) >> 3; > raw->m_L1Size = (bytes + block_size - 1) / block_size; > > which means that the header is rotten. In that case can you pls > take a look why this has not been caught by tests? It looks the same if block_size is cluster size in bytes. bytes - bitmap size in bytes raw->m_L1Size - bitmap size in clusters In my code fh->data_size is l1_table size (8 bytes entry for each bitmap cluster) plus dirty bitmap feature header. > >> + >> + bh->l1_size = cpu_to_le32(l1_size); >> + bh->size = cpu_to_le64(bm_size >> BDRV_SECTOR_BITS); >> + bh->granularity = cpu_to_le32(granularity >> BDRV_SECTOR_BITS); >> + memcpy(bh->id, &uuid, sizeof(uuid)); >> + >> + bm_buf = qemu_blockalign(bs, s->cluster_size); >> + >> + offset = 0; >> + while ((offset = bdrv_dirty_bitmap_next_dirty(bitmap, offset, >> bm_size)) >= 0) { >> + uint64_t idx = offset / limit; >> + int64_t cluster_off, end, write_size; >> + >> + offset = QEMU_ALIGN_DOWN(offset, limit); >> + end = MIN(bm_size, offset + limit); >> + write_size = bdrv_dirty_bitmap_serialization_size(bitmap, >> offset, >> + end - >> offset); >> + assert(write_size <= s->cluster_size); >> + >> + bdrv_dirty_bitmap_serialize_part(bitmap, bm_buf, offset, end >> - offset); >> + if (write_size < s->cluster_size) { >> + memset(bm_buf + write_size, 0, s->cluster_size - >> write_size); >> + } >> + >> + cluster_off = parallels_allocate_host_clusters(bs, >> &alloc_size); >> + if (cluster_off <= 0) { >> + goto end; >> + } > > > >> + >> + ret = bdrv_pwrite(bs->file, cluster_off, s->cluster_size, >> bm_buf, 0); >> + if (ret < 0) { >> + memset(&fh->magic, 0, sizeof(fh->magic)); >> + parallels_mark_unused(bs, s->used_bmap, s->used_bmap_size, >> + cluster_off, 1); > That is incomplete. You have to clean all clusters inside of the > extension. OK > >> + goto end; >> + } >> + >> + l1_table[idx] = cpu_to_le64(cluster_off >> BDRV_SECTOR_BITS); >> + offset = end; >> + } >> + >> + *buf_size -= buf_used; >> + *buf += buf_used; >> + >> +end: >> + qemu_vfree(bm_buf); >> +} >> + >> +void GRAPH_RDLOCK >> +parallels_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error >> **errp) >> +{ >> + BDRVParallelsState *s = bs->opaque; >> + BdrvDirtyBitmap *bitmap; >> + ParallelsFormatExtensionHeader *eh; >> + int remaining = s->cluster_size; >> + uint8_t *buf, *pos; >> + int64_t header_off, alloc_size = 1; >> + g_autofree uint8_t *hash = NULL; >> + size_t hash_len = 0; >> + int ret; >> + >> + s->header->ext_off = 0; >> + >> + if (!bdrv_has_named_bitmaps(bs)) { >> + return; >> + } >> + >> + buf = qemu_blockalign0(bs, s->cluster_size); >> + >> + eh = (ParallelsFormatExtensionHeader *)buf; >> + pos = buf + sizeof(*eh); >> + >> + eh->magic = cpu_to_le64(PARALLELS_FORMAT_EXTENSION_MAGIC); >> + >> + FOR_EACH_DIRTY_BITMAP(bs, bitmap) { >> + parallels_save_bitmap(bs, bitmap, &pos, &remaining); >> + } >> + >> + header_off = parallels_allocate_host_clusters(bs, &alloc_size); >> + if (header_off < 0) { >> + error_report("Can't save dirty bitmap: cluster allocation >> error"); >> + ret = header_off; >> + goto end; >> + } >> + >> + ret = qcrypto_hash_bytes(QCRYPTO_HASH_ALG_MD5, >> + (const char *)(buf + sizeof(*eh)), >> + s->cluster_size - sizeof(*eh), >> + &hash, &hash_len, errp); >> + if (ret < 0 || hash_len != sizeof(eh->check_sum)) { >> + error_report("Can't save dirty bitmap: hash error"); >> + ret = -EINVAL; >> + goto end; >> + } >> + memcpy(eh->check_sum, hash, hash_len); >> + >> + ret = bdrv_pwrite(bs->file, header_off, s->cluster_size, buf, 0); >> + if (ret < 0) { >> + error_report("Can't save dirty bitmap: IO error"); >> + parallels_mark_unused(bs, s->used_bmap, s->used_bmap_size, >> + header_off, 1); >> + goto end; >> + } >> + >> + s->header->ext_off = cpu_to_le64(header_off / BDRV_SECTOR_SIZE); >> +end: >> + qemu_vfree(buf); >> +} >> + >> +bool coroutine_fn >> parallels_co_can_store_new_dirty_bitmap(BlockDriverState *bs, >> + const char >> *name, >> + uint32_t granularity, >> + Error **errp) >> +{ >> + if (bdrv_find_dirty_bitmap(bs, name)) { >> + error_setg(errp, "Bitmap already exists: %s", name); >> + return false; >> + } >> + >> + return true; >> +} >> diff --git a/block/parallels.c b/block/parallels.c >> index f38dd2309c..a49922c6a7 100644 >> --- a/block/parallels.c >> +++ b/block/parallels.c >> @@ -1466,14 +1466,25 @@ fail: >> static int GRAPH_RDLOCK parallels_inactivate(BlockDriverState *bs) >> { >> BDRVParallelsState *s = bs->opaque; >> + Error *err = NULL; >> int ret; >> + parallels_store_persistent_dirty_bitmaps(bs, &err); >> + if (err != NULL) { > For me this looks quite odd, indirect check for error. Not good Will add return value; > >> + error_reportf_err(err, "Lost persistent bitmaps during " >> + "inactivation of node '%s': ", >> + bdrv_get_device_or_node_name(bs)); >> + } >> + >> s->header->inuse = 0; >> parallels_update_header(bs); >> /* errors are ignored, so we might as well pass exact=true */ >> - ret = bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, >> true, >> - PREALLOC_MODE_OFF, 0, NULL); >> + ret = bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, >> + true, PREALLOC_MODE_OFF, 0, NULL); >> + if (ret < 0) { >> + error_report("Failed to truncate image: %s", strerror(-ret)); >> + } >> return ret; >> } >> @@ -1525,6 +1536,7 @@ static BlockDriver bdrv_parallels = { >> .bdrv_co_pdiscard = parallels_co_pdiscard, >> .bdrv_co_pwrite_zeroes = parallels_co_pwrite_zeroes, >> .bdrv_inactivate = parallels_inactivate, >> + .bdrv_co_can_store_new_dirty_bitmap = >> parallels_co_can_store_new_dirty_bitmap, >> }; >> static void bdrv_parallels_init(void) >> diff --git a/block/parallels.h b/block/parallels.h >> index 493c89e976..9db4f5c908 100644 >> --- a/block/parallels.h >> +++ b/block/parallels.h >> @@ -101,5 +101,10 @@ int64_t GRAPH_RDLOCK >> parallels_allocate_host_clusters(BlockDriverState *bs, >> int GRAPH_RDLOCK >> parallels_read_format_extension(BlockDriverState *bs, int64_t ext_off, >> Error **errp); >> +void GRAPH_RDLOCK >> +parallels_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error >> **errp); >> +bool coroutine_fn >> +parallels_co_can_store_new_dirty_bitmap(BlockDriverState *bs, const >> char *name, >> + uint32_t granularity, Error >> **errp); >> #endif >
diff --git a/block/parallels-ext.c b/block/parallels-ext.c index b4e14c88f2..c83d1ea393 100644 --- a/block/parallels-ext.c +++ b/block/parallels-ext.c @@ -24,6 +24,7 @@ */ #include "qemu/osdep.h" +#include "qemu/error-report.h" #include "qapi/error.h" #include "block/block-io.h" #include "block/block_int.h" @@ -300,3 +301,170 @@ out: return ret; } + +static void GRAPH_RDLOCK parallels_save_bitmap(BlockDriverState *bs, + BdrvDirtyBitmap *bitmap, + uint8_t **buf, int *buf_size) +{ + BDRVParallelsState *s = bs->opaque; + ParallelsFeatureHeader *fh; + ParallelsDirtyBitmapFeature *bh; + uint64_t *l1_table, l1_size, granularity, limit; + int64_t bm_size, ser_size, offset, buf_used; + int64_t alloc_size = 1; + const char *name; + uint8_t *bm_buf; + QemuUUID uuid; + int ret = 0; + + if (!bdrv_dirty_bitmap_get_persistence(bitmap) || + bdrv_dirty_bitmap_inconsistent(bitmap)) { + return; + } + + bm_size = bdrv_dirty_bitmap_size(bitmap); + granularity = bdrv_dirty_bitmap_granularity(bitmap); + limit = bdrv_dirty_bitmap_serialization_coverage(s->cluster_size, bitmap); + ser_size = bdrv_dirty_bitmap_serialization_size(bitmap, 0, bm_size); + l1_size = DIV_ROUND_UP(ser_size, s->cluster_size); + + buf_used = l1_size * 8 + sizeof(*fh) + sizeof(*bh); + /* Check if there is enough space for the final section */ + if (*buf_size - buf_used < sizeof(*fh)) { + return; + } + + name = bdrv_dirty_bitmap_name(bitmap); + ret = qemu_uuid_parse(name, &uuid); + if (ret < 0) { + error_report("Can't save dirty bitmap: ID parsing error: '%s'", name); + return; + } + + fh = (ParallelsFeatureHeader *)*buf; + bh = (ParallelsDirtyBitmapFeature *)(*buf + sizeof(*fh)); + l1_table = (uint64_t *)((uint8_t *)bh + sizeof(*bh)); + + fh->magic = cpu_to_le64(PARALLELS_DIRTY_BITMAP_FEATURE_MAGIC); + fh->data_size = cpu_to_le32(l1_size * 8 + sizeof(*bh)); + + bh->l1_size = cpu_to_le32(l1_size); + bh->size = cpu_to_le64(bm_size >> BDRV_SECTOR_BITS); + bh->granularity = cpu_to_le32(granularity >> BDRV_SECTOR_BITS); + memcpy(bh->id, &uuid, sizeof(uuid)); + + bm_buf = qemu_blockalign(bs, s->cluster_size); + + offset = 0; + while ((offset = bdrv_dirty_bitmap_next_dirty(bitmap, offset, bm_size)) >= 0) { + uint64_t idx = offset / limit; + int64_t cluster_off, end, write_size; + + offset = QEMU_ALIGN_DOWN(offset, limit); + end = MIN(bm_size, offset + limit); + write_size = bdrv_dirty_bitmap_serialization_size(bitmap, offset, + end - offset); + assert(write_size <= s->cluster_size); + + bdrv_dirty_bitmap_serialize_part(bitmap, bm_buf, offset, end - offset); + if (write_size < s->cluster_size) { + memset(bm_buf + write_size, 0, s->cluster_size - write_size); + } + + cluster_off = parallels_allocate_host_clusters(bs, &alloc_size); + if (cluster_off <= 0) { + goto end; + } + + ret = bdrv_pwrite(bs->file, cluster_off, s->cluster_size, bm_buf, 0); + if (ret < 0) { + memset(&fh->magic, 0, sizeof(fh->magic)); + parallels_mark_unused(bs, s->used_bmap, s->used_bmap_size, + cluster_off, 1); + goto end; + } + + l1_table[idx] = cpu_to_le64(cluster_off >> BDRV_SECTOR_BITS); + offset = end; + } + + *buf_size -= buf_used; + *buf += buf_used; + +end: + qemu_vfree(bm_buf); +} + +void GRAPH_RDLOCK +parallels_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp) +{ + BDRVParallelsState *s = bs->opaque; + BdrvDirtyBitmap *bitmap; + ParallelsFormatExtensionHeader *eh; + int remaining = s->cluster_size; + uint8_t *buf, *pos; + int64_t header_off, alloc_size = 1; + g_autofree uint8_t *hash = NULL; + size_t hash_len = 0; + int ret; + + s->header->ext_off = 0; + + if (!bdrv_has_named_bitmaps(bs)) { + return; + } + + buf = qemu_blockalign0(bs, s->cluster_size); + + eh = (ParallelsFormatExtensionHeader *)buf; + pos = buf + sizeof(*eh); + + eh->magic = cpu_to_le64(PARALLELS_FORMAT_EXTENSION_MAGIC); + + FOR_EACH_DIRTY_BITMAP(bs, bitmap) { + parallels_save_bitmap(bs, bitmap, &pos, &remaining); + } + + header_off = parallels_allocate_host_clusters(bs, &alloc_size); + if (header_off < 0) { + error_report("Can't save dirty bitmap: cluster allocation error"); + ret = header_off; + goto end; + } + + ret = qcrypto_hash_bytes(QCRYPTO_HASH_ALG_MD5, + (const char *)(buf + sizeof(*eh)), + s->cluster_size - sizeof(*eh), + &hash, &hash_len, errp); + if (ret < 0 || hash_len != sizeof(eh->check_sum)) { + error_report("Can't save dirty bitmap: hash error"); + ret = -EINVAL; + goto end; + } + memcpy(eh->check_sum, hash, hash_len); + + ret = bdrv_pwrite(bs->file, header_off, s->cluster_size, buf, 0); + if (ret < 0) { + error_report("Can't save dirty bitmap: IO error"); + parallels_mark_unused(bs, s->used_bmap, s->used_bmap_size, + header_off, 1); + goto end; + } + + s->header->ext_off = cpu_to_le64(header_off / BDRV_SECTOR_SIZE); +end: + qemu_vfree(buf); +} + +bool coroutine_fn parallels_co_can_store_new_dirty_bitmap(BlockDriverState *bs, + const char *name, + uint32_t granularity, + Error **errp) +{ + if (bdrv_find_dirty_bitmap(bs, name)) { + error_setg(errp, "Bitmap already exists: %s", name); + return false; + } + + return true; +} diff --git a/block/parallels.c b/block/parallels.c index f38dd2309c..a49922c6a7 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -1466,14 +1466,25 @@ fail: static int GRAPH_RDLOCK parallels_inactivate(BlockDriverState *bs) { BDRVParallelsState *s = bs->opaque; + Error *err = NULL; int ret; + parallels_store_persistent_dirty_bitmaps(bs, &err); + if (err != NULL) { + error_reportf_err(err, "Lost persistent bitmaps during " + "inactivation of node '%s': ", + bdrv_get_device_or_node_name(bs)); + } + s->header->inuse = 0; parallels_update_header(bs); /* errors are ignored, so we might as well pass exact=true */ - ret = bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, true, - PREALLOC_MODE_OFF, 0, NULL); + ret = bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, + true, PREALLOC_MODE_OFF, 0, NULL); + if (ret < 0) { + error_report("Failed to truncate image: %s", strerror(-ret)); + } return ret; } @@ -1525,6 +1536,7 @@ static BlockDriver bdrv_parallels = { .bdrv_co_pdiscard = parallels_co_pdiscard, .bdrv_co_pwrite_zeroes = parallels_co_pwrite_zeroes, .bdrv_inactivate = parallels_inactivate, + .bdrv_co_can_store_new_dirty_bitmap = parallels_co_can_store_new_dirty_bitmap, }; static void bdrv_parallels_init(void) diff --git a/block/parallels.h b/block/parallels.h index 493c89e976..9db4f5c908 100644 --- a/block/parallels.h +++ b/block/parallels.h @@ -101,5 +101,10 @@ int64_t GRAPH_RDLOCK parallels_allocate_host_clusters(BlockDriverState *bs, int GRAPH_RDLOCK parallels_read_format_extension(BlockDriverState *bs, int64_t ext_off, Error **errp); +void GRAPH_RDLOCK +parallels_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp); +bool coroutine_fn +parallels_co_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name, + uint32_t granularity, Error **errp); #endif
Now dirty bitmaps can be loaded but there is no their saving. Add code for dirty bitmap storage. Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> --- block/parallels-ext.c | 168 ++++++++++++++++++++++++++++++++++++++++++ block/parallels.c | 16 +++- block/parallels.h | 5 ++ 3 files changed, 187 insertions(+), 2 deletions(-)