On Fri, Jun 18, 2021 at 11:00 AM Peter Lieven <p...@kamp.de> wrote: > > Am 16.06.21 um 14:34 schrieb Ilya Dryomov: > > On Wed, May 19, 2021 at 4:28 PM Peter Lieven <p...@kamp.de> wrote: > >> Signed-off-by: Peter Lieven <p...@kamp.de> > >> --- > >> block/rbd.c | 37 ++++++++++++++++++++++++++++++++++++- > >> 1 file changed, 36 insertions(+), 1 deletion(-) > >> > >> diff --git a/block/rbd.c b/block/rbd.c > >> index 0d8612a988..ee13f08a74 100644 > >> --- a/block/rbd.c > >> +++ b/block/rbd.c > >> @@ -63,7 +63,8 @@ typedef enum { > >> RBD_AIO_READ, > >> RBD_AIO_WRITE, > >> RBD_AIO_DISCARD, > >> - RBD_AIO_FLUSH > >> + RBD_AIO_FLUSH, > >> + RBD_AIO_WRITE_ZEROES > >> } RBDAIOCmd; > >> > >> typedef struct BDRVRBDState { > >> @@ -705,6 +706,10 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict > >> *options, int flags, > >> } > >> } > >> > >> +#ifdef LIBRBD_SUPPORTS_WRITE_ZEROES > >> + bs->supported_zero_flags = BDRV_REQ_MAY_UNMAP; > > I wonder if we should also set BDRV_REQ_NO_FALLBACK here since librbd > > does not really have a notion of non-efficient explicit zeroing. > > > This is only true if thick provisioning is supported which is in Octopus > onwards, right?
Since Pacific, I think. > > So it would only be correct to set this if thick provisioning is supported > otherwise we could > > fail with ENOTSUP and then qemu emulates the zeroing with plain writes. I actually had a question about that. Why are you returning ENOTSUP in case BDRV_REQ_MAY_UNMAP is not specified and that can't be fulfilled because librbd is too old for RBD_WRITE_ZEROES_FLAG_THICK_PROVISION? My understanding has always been that BDRV_REQ_MAY_UNMAP is just a hint. Deallocating if BDRV_REQ_MAY_UNMAP is specified is not nice but should be perfectly acceptable. It is certainly better than returning ENOTSUP, particularly if ENOTSUP causes Qemu to do plain zeroing. Thanks, Ilya