On 10/15/19 11:16 AM, Vladimir Sementsov-Ogievskiy wrote:
@@ -1561,6 +1569,8 @@ NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t
dev_offset,
exp->export_bitmap = bm;
exp->export_bitmap_context = g_strdup_printf("qemu:dirty-bitmap:%s",
bitmap);
+ /* See BME_MAX_NAME_SIZE in block/qcow2-bitmap.c */
Hmm. BME_MAX_NAME_SIZE is checked only when creating persistent bitmaps. But
for non-persistent
name length is actually unlimited. So, we should either limit all bitmap names
to 1023 (hope,
this will not break existing scenarios) or error out here (or earlier) instead
of assertion.
I'm seriously doubting that any existing scenarios try to use a name
that long. If no one was relying on a long name (especially since it was
inconsistent between persistent being limited to qcow2 constraints and
non-persistent having no limit), we can consider it as a bug-fix rather
than something needing a deprecation period.
I'm leaning towards limiting ALL bitmaps to the same length (as we've already
debated the idea of being able to convert an existing bitmap from transient to
persistent).
Agreed, but ..
We also may want QEMU_BUILD_BUG_ON(NBD_MAX_STRING_SIZE < BME_MAX_NAME_SIZE +
sizeof("qemu:dirty-bitmap:") - 1)
Except that BME_MAX_NAME_SIZE is not (currently) in a public .h file.
.. I think, than it should be new BLOCK_DIRTY_BITMAP_MAX_NAME_SIZE.. And we'll
have to note it in qapi doc..
Should this change go through deprecation? Or we consider non-persistent
bitmaps as something not really useful?
I'm preparing a v3 patch that just goes ahead and adds the limit on
bitmap names everywhere, as a separate patch.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
