Re: [RFC PATCH] virtio-blk: advertise F_WCE (F_FLUSH) if F_CONFIG_WCE is also advertised

[Stefan Hajnoczi]

On Wed, Oct 09, 2019 at 03:37:35PM +0300, Evgeny Yakovlev wrote:
> On 08.10.2019 16:24, Stefan Hajnoczi wrote:
> > On Fri, Sep 20, 2019 at 02:56:30PM +0300, Evgeny Yakovlev wrote:
> > The motivation for this change looks correct but this patch may cause
> > host_features to change across live migration to a newer QEMU version.
> > If the guest accesses VIRTIO_PCI_HOST_FEATURES before and after live
> > migration it may see different values, which is unexpected.
> > 
> > The safe way of introducing guest-visible changes like this is to make
> > the change conditional on the machine type version so that old guests
> > see old behavior and new guests see new behavior.
> > 
> > Live migration compatibility can be guaranteed by adding a new property
> > to virtio_blk_properties[]:
> > 
> >    DEFINE_PROP_BOOL("enable-wce-if-config-wce", VirtIOBlock,
> >                     conf.enable_wce_if_config_wce, true),
> > 
> > Then tweak the patch:
> > 
> >    if (blk_enable_write_cache(s->blk) ||
> >        (s->conf.enable_wce_if_config_wce &&
> >         virtio_has_feature(features, VIRTIO_BLK_F_CONFIG_WCE))) {
> > 
> > And finally disable enable_wce_if_config_wce for older machine types to
> > retain compatibility:
> > 
> >    GlobalProperty hw_compat_4_2[] = {
> >        { "virtio-blk-device", "enable-wce-if-config-wce", "off" },
> >    };
> > 
> > (I have omitted some steps like defining
> > VirtIOBlkConf.enable_wce_if_config_wce field and hooking up
> > hw_compat_4_2[], but you can figure that out from the existing code.)
> 
> 
> Why don't we just migrate host_features? That will ensure they are the same.
> And messing up device config on destination is bad anyway.

Traditionally QEMU has not migrated host state/objects.  Instead it
relies on the destination QEMU command-line producing exactly the same
host state as on the source host.

A problem with migration host state is that it's impossible to offer
host features that are unavailable on the destination host (they cannot
be "migrated").

Stefan

signature.asc
Description: PGP signature