Message ID | 1337703438-9764-4-git-send-email-pbonzini@redhat.com |
---|---|
State | New |
Headers | show |
Am 22.05.2012 18:17, schrieb Paolo Bonzini: > Formats are entirely in charge of flushes for metadata writes. For > guest-initiated writes, a writethrough cache is faked in the block layer. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > block.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/block.c b/block.c > index 7add33c..43b3de6 100644 > --- a/block.c > +++ b/block.c > @@ -661,7 +661,7 @@ static int bdrv_open_common(BlockDriverState *bs, const char *filename, > if (drv->bdrv_file_open) { > ret = drv->bdrv_file_open(bs, filename, open_flags); Why don't you add the writeback flag here? Would the code is raw-posix that adds O_DSYNC be unused then. and can it still be used in other ways than format=file/host_* with this patch? > } else { > - ret = bdrv_file_open(&bs->file, filename, open_flags); > + ret = bdrv_file_open(&bs->file, filename, open_flags | BDRV_O_CACHE_WB); > if (ret >= 0) { > ret = drv->bdrv_open(bs, open_flags); > } Kevin
Il 31/05/2012 13:33, Kevin Wolf ha scritto: >> > diff --git a/block.c b/block.c >> > index 7add33c..43b3de6 100644 >> > --- a/block.c >> > +++ b/block.c >> > @@ -661,7 +661,7 @@ static int bdrv_open_common(BlockDriverState *bs, const char *filename, >> > if (drv->bdrv_file_open) { >> > ret = drv->bdrv_file_open(bs, filename, open_flags); > Why don't you add the writeback flag here? Would the code is raw-posix > that adds O_DSYNC be unused then. and can it still be used in other ways > than format=file/host_* with this patch? Because in this case there is no format over it, so the topmost BDS will be open with BDRV_O_CACHE_WB and no one will call bdrv_flush. We could indeed remove the BDRV_O_CACHE_WB handling in all the protocol drivers (at least those where bdrv_flush_to_disk does something sensible), and always open in writeback mode. At that point this patch could be reverted. Paolo >> > } else { >> > - ret = bdrv_file_open(&bs->file, filename, open_flags); >> > + ret = bdrv_file_open(&bs->file, filename, open_flags | BDRV_O_CACHE_WB); >> > if (ret >= 0) { >> > ret = drv->bdrv_open(bs, open_flags); >> > }
Am 31.05.2012 13:43, schrieb Paolo Bonzini: > Il 31/05/2012 13:33, Kevin Wolf ha scritto: >>>> diff --git a/block.c b/block.c >>>> index 7add33c..43b3de6 100644 >>>> --- a/block.c >>>> +++ b/block.c >>>> @@ -661,7 +661,7 @@ static int bdrv_open_common(BlockDriverState *bs, const char *filename, >>>> if (drv->bdrv_file_open) { >>>> ret = drv->bdrv_file_open(bs, filename, open_flags); >> Why don't you add the writeback flag here? Would the code is raw-posix >> that adds O_DSYNC be unused then. and can it still be used in other ways >> than format=file/host_* with this patch? > > Because in this case there is no format over it, so the topmost BDS will > be open with BDRV_O_CACHE_WB and no one will call bdrv_flush. Doesn't block.c do it now? bs->enable_write_cache still wouldn't be set, even if .bdrv_file_open() is called with BDRV_O_CACHE_WB. Kevin
Il 31/05/2012 13:56, Kevin Wolf ha scritto: >> > Because in this case there is no format over it, so the topmost BDS will >> > be open with BDRV_O_CACHE_WB and no one will call bdrv_flush. > Doesn't block.c do it now? bs->enable_write_cache still wouldn't be set, > even if .bdrv_file_open() is called with BDRV_O_CACHE_WB. You're right. Paolo
diff --git a/block.c b/block.c index 7add33c..43b3de6 100644 --- a/block.c +++ b/block.c @@ -661,7 +661,7 @@ static int bdrv_open_common(BlockDriverState *bs, const char *filename, if (drv->bdrv_file_open) { ret = drv->bdrv_file_open(bs, filename, open_flags); } else { - ret = bdrv_file_open(&bs->file, filename, open_flags); + ret = bdrv_file_open(&bs->file, filename, open_flags | BDRV_O_CACHE_WB); if (ret >= 0) { ret = drv->bdrv_open(bs, open_flags); }
Formats are entirely in charge of flushes for metadata writes. For guest-initiated writes, a writethrough cache is faked in the block layer. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- block.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)