Am 26.03.2016 um 21:44 hat Max Reitz geschrieben: > On 18.03.2016 19:21, Kevin Wolf wrote: > > This replaces the existing hack in the iscsi driver that sent the FUA > > bit in writethrough mode and ignored the following flush in order to > > optimise the number of roundtrips (see commit 73b5394e). > > > > Signed-off-by: Kevin Wolf <kw...@redhat.com> > > --- > > block/iscsi.c | 24 +++++++----------------- > > 1 file changed, 7 insertions(+), 17 deletions(-) > > > > diff --git a/block/iscsi.c b/block/iscsi.c > > index 3b54536..4f75204 100644 > > --- a/block/iscsi.c > > +++ b/block/iscsi.c > > [...] > > > @@ -1851,7 +1840,8 @@ static BlockDriver bdrv_iscsi = { > > .bdrv_co_discard = iscsi_co_discard, > > .bdrv_co_write_zeroes = iscsi_co_write_zeroes, > > .bdrv_co_readv = iscsi_co_readv, > > - .bdrv_co_writev = iscsi_co_writev, > > + .bdrv_co_writev_flags = iscsi_co_writev_flags, > > + .supported_write_flags = BDRV_REQ_FUA, > > .bdrv_co_flush_to_disk = iscsi_co_flush, > > > > #ifdef __linux__ > > > > Hm, wait, maybe not R-b. I can see three places in block/io.c which call > bdrv_co_writev(), and only one of them diverts to bdrv_co_writev_flags() > if that is available. Maybe we don't need to care about the > bounce-buffer case for write_zeroes, but I do think we need to care > about the COR case. > > Of course bdrv_co_writev() can trivially be forwarded to > bdrv_co_writev_flags(), but I'm not sure who is supposed to do this > forwarding. I can imagine three ways: > > (1) Keep a wrapper per block driver. Simple, but not so elegant. > (2) Make all bdrv_co_writev() callers call bdrv_co_writev_flags() if > the former is not available but the latter is. > (3) Introduce a generic function replacing every drv->bdrv_co_writev() > call which then decides which driver function to invoke.
Good catch, thanks! Going for (1) for now, because I think (2) is even less elegant and while I have a slight preference for (3) from the code perspective, it could be argued that it impacts the hot write path of raw images and I don't want to deal with potential performance changes that late in the cycle. And now that I'm writing this, I realise that the hot path already calls .bdrv_co_writev_flags, so that's not a real argument. But I've already implemented (1), so I'll leave it at that... The long term plan is anyway to convert everything to .bdrv_co_writev_flags. Kevin
pgpAS2CJ_nJB6.pgp
Description: PGP signature