Re: [PATCH v2 1/2] nbd: Don't send oversize strings

[Eric Blake]

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