On Sat, Apr 09, 2016 at 11:57:57AM +0100, Alex Bligh wrote: > > On 8 Apr 2016, at 23:05, Eric Blake <ebl...@redhat.com> wrote: > > > RFC because there is still discussion on the NBD list about > > adding an NBD_OPT_ to let the client suggest server defaults > > related to scanning for zeroes during NBD_CMD_WRITE, which may > > tweak this patch. > > > > Upstream NBD protocol recently added the ability to efficiently > > write zeroes without having to send the zeroes over the wire, > > along with a flag to control whether the client wants a hole. > > > > The generic block code takes care of falling back to the obvious > > write lots of zeroes if we return -ENOTSUP because the server > > does not have WRITE_ZEROES. > > > > Signed-off-by: Eric Blake <ebl...@redhat.com> > > --- > > block/nbd-client.h | 2 ++ > > block/nbd-client.c | 34 ++++++++++++++++++++++++++++++++++ > > block/nbd.c | 23 +++++++++++++++++++++++ > > 3 files changed, 59 insertions(+) > > > > diff --git a/block/nbd-client.h b/block/nbd-client.h > > index bc7aec0..2fe6654 100644 > > --- a/block/nbd-client.h > > +++ b/block/nbd-client.h > > @@ -47,6 +47,8 @@ void nbd_client_close(BlockDriverState *bs); > > int nbd_client_co_discard(BlockDriverState *bs, int64_t sector_num, > > int nb_sectors); > > int nbd_client_co_flush(BlockDriverState *bs); > > +int nbd_client_co_write_zeroes(BlockDriverState *bs, int64_t sector_num, > > + int nb_sectors, int *flags); > > int nbd_client_co_writev(BlockDriverState *bs, int64_t sector_num, > > int nb_sectors, QEMUIOVector *qiov, int *flags); > > int nbd_client_co_readv(BlockDriverState *bs, int64_t sector_num, > > diff --git a/block/nbd-client.c b/block/nbd-client.c > > index f013084..4be83a8 100644 > > --- a/block/nbd-client.c > > +++ b/block/nbd-client.c > > @@ -291,6 +291,40 @@ int nbd_client_co_readv(BlockDriverState *bs, int64_t > > sector_num, > > return nbd_co_readv_1(bs, sector_num, nb_sectors, qiov, offset); > > } > > > > +int nbd_client_co_write_zeroes(BlockDriverState *bs, int64_t sector_num, > > + int nb_sectors, int *flags) > > +{ > > + ssize_t ret; > > + NbdClientSession *client = nbd_get_client_session(bs); > > + struct nbd_request request = { .type = NBD_CMD_WRITE_ZEROES }; > > + struct nbd_reply reply; > > + > > + if (!(client->nbdflags & NBD_FLAG_SEND_WRITE_ZEROES)) { > > + return -ENOTSUP; > > + } > > + > > + if ((*flags & BDRV_REQ_FUA) && (client->nbdflags & NBD_FLAG_SEND_FUA)) > > { > > + *flags &= ~BDRV_REQ_FUA; > > + request.flags |= NBD_CMD_FLAG_FUA; > > + } > > + if (!(*flags & BDRV_REQ_MAY_UNMAP)) { > > + request.flags |= NBD_CMD_FLAG_NO_HOLE; > > + } > > + > > + request.from = sector_num * 512; > > + request.len = nb_sectors * 512; > > + > > + nbd_coroutine_start(client, &request); > > + ret = nbd_co_send_request(bs, &request, NULL, 0); > > + if (ret < 0) { > > + reply.error = -ret; > > + } else { > > + nbd_co_receive_reply(client, &request, &reply, NULL, 0); > > + } > > + nbd_coroutine_end(client, &request); > > + return -reply.error; > > +} > > + > > int nbd_client_co_writev(BlockDriverState *bs, int64_t sector_num, > > int nb_sectors, QEMUIOVector *qiov, int *flags) > > { > > diff --git a/block/nbd.c b/block/nbd.c > > index f7ea3b3..f5119c0 100644 > > --- a/block/nbd.c > > +++ b/block/nbd.c > > @@ -355,6 +355,26 @@ static int nbd_co_readv(BlockDriverState *bs, int64_t > > sector_num, > > return nbd_client_co_readv(bs, sector_num, nb_sectors, qiov); > > } > > > > +static int nbd_co_write_zeroes(BlockDriverState *bs, int64_t sector_num, > > + int nb_sectors, BdrvRequestFlags orig_flags) > > +{ > > + int flags = orig_flags; > > + int ret; > > + > > + ret = nbd_client_co_write_zeroes(bs, sector_num, nb_sectors, &flags); > > + if (ret < 0) { > > + return ret; > > + } > > + > > + /* The flag wasn't sent to the server, so we need to emulate it with an > > + * explicit flush */ > > Surely you only need to do this is the flag wasn't sent to the server, > i.e. if !(client->nbdflags & NBD_FLAG_SEND_FUA) > > If you've sent a FUA request, no need to flush the whole thing.
In this case BDRV_REQ_FUA is cleared from 'flags' by nbd_client_co_write_zeroes() and this condition becomes false. > > nbd_co_writev_flags seems to have the same issue, which is where I guess > you got that from. > > > + if (flags & BDRV_REQ_FUA) { > > + ret = nbd_client_co_flush(bs); > > + } > > + > > + return ret; > > +} > > + > > static int nbd_co_writev_flags(BlockDriverState *bs, int64_t sector_num, > > int nb_sectors, QEMUIOVector *qiov, int > > flags) > > { > > @@ -476,6 +496,7 @@ static BlockDriver bdrv_nbd = { > > .bdrv_parse_filename = nbd_parse_filename, > > .bdrv_file_open = nbd_open, > > .bdrv_co_readv = nbd_co_readv, > > + .bdrv_co_write_zeroes = nbd_co_write_zeroes, > > .bdrv_co_writev = nbd_co_writev, > > .bdrv_co_writev_flags = nbd_co_writev_flags, > > .supported_write_flags = BDRV_REQ_FUA, > > @@ -496,6 +517,7 @@ static BlockDriver bdrv_nbd_tcp = { > > .bdrv_parse_filename = nbd_parse_filename, > > .bdrv_file_open = nbd_open, > > .bdrv_co_readv = nbd_co_readv, > > + .bdrv_co_write_zeroes = nbd_co_write_zeroes, > > .bdrv_co_writev = nbd_co_writev, > > .bdrv_co_writev_flags = nbd_co_writev_flags, > > .supported_write_flags = BDRV_REQ_FUA, > > @@ -516,6 +538,7 @@ static BlockDriver bdrv_nbd_unix = { > > .bdrv_parse_filename = nbd_parse_filename, > > .bdrv_file_open = nbd_open, > > .bdrv_co_readv = nbd_co_readv, > > + .bdrv_co_write_zeroes = nbd_co_write_zeroes, > > .bdrv_co_writev = nbd_co_writev, > > .bdrv_co_writev_flags = nbd_co_writev_flags, > > .supported_write_flags = BDRV_REQ_FUA, > > -- > > 2.5.5 > > > > > > -- > Alex Bligh > > > > >