Re: [PATCH v2] block/file-posix: Optimize for macOS

[Kevin Wolf]

Am 09.03.2021 um 05:52 hat Akihiko Odaki geschrieben:
> 2021年3月9日(火) 0:37 Akihiko Odaki <akihiko.odaki@gmail.com>:
> >
> > 2021年3月9日(火) 0:17 Stefan Hajnoczi <stefanha@redhat.com>:
> > >
> > > The live migration compatibility issue is still present. Migrating to
> > > another host might not work if the block limits are different.
> > >
> > > Here is an idea for solving it:
> > >
> > > Modify include/hw/block/block.h:DEFINE_BLOCK_PROPERTIES_BASE() to
> > > support a new value called "host". The default behavior remains
> > > unchanged for live migration compatibility but now you can use "host" if
> > > you know it's okay but don't care about migration compatibility.
> > >
> > > The downside to this approach is that users must explicitly say
> > > something like --drive ...,opt_io_size=host. But it's still better than
> > > the situation we have today where user must manually enter values for
> > > their disk.
> > >
> > > Does this sound okay to everyone?
> > >
> > > Stefan
> >
> > I wonder how that change affects other block drivers implementing
> > bdrv_probe_blocksizes. As far as I know, the values they report are
> > already used by default, which is contrary to the default not being
> > "host".
> >
> > Regards,
> > Akihiko Odaki
> 
> Let me suggest a variant of Stefan's approach:
> 
> Modify include/hw/block/block.h:DEFINE_BLOCK_PROPERTIES_BASE() to
> support a new value called "host". The default values for block size
> properties may be "host" or not, but they should be consistent. If
> they are "host" by default

I'm not sure if it's a good idea, but maybe we could make it so that the
default is "host" only as long as you didn't specify -nodefaults? Then
libvirt would automatically keep the old behaviour (because it always
sets -nodefaults) and manual invocations would usually get the new one.

Of course, when I start with "I'm not sure if it's a good idea", it's
usually not, but I wanted to share the thought anyway...

> add global properties which sets
> discard_granularity and opt_io_size to the old default to
> hw_compat_5_2 in hw/core/machine.c. Otherwise, add global properties
> which sets logical_block_size and physical_block_size to "host".

Would we have to do this for explicitly for every single block device in
the tree? That sounds like a lot of cases and therefore rather error
prone.

> Does it sound good? I'd also like to know others opinions for the
> default value ("host" or something else). I prefer "host" as the
> default a little because those who need live migration should be
> careful enough to set proper configurations for each device. We may
> also assist users who need live migration by adding a property which
> defaults all block size properties to something else "host".

Adding new requirements is always a bit problematic. Live migration
works fine today without specifying these properties, so users will
expect it to keep working. If live migration were a new feature and we
required the options from the start, it would be different.

Kevin