On Sun, Dec 04, 2016 at 11:44:57PM +0000, Yi Fang wrote:
NBD client has not implemented callback for .bdrv_has_zero_init. So
bdrv_has_zero_init always returns 0 when doing non-shared storage
migration.
This patch implemented NBD_CMD_HAS_ZERO_INIT and will avoid unnecessary
set-dirty.
Please link to the NBD spec where this new command is defined. Has it
already been accepted by the NBD community?
I think this is a weird command because it's information that doesn't
change over the lifetime of an NBD session. Your patch sends a command
and waits for the reply every time has_zero_init() is queried.
Is there a better place to put this information in the NBD protocols,
like some export information that is retried during connection? (I
haven't checked the protocol.) It seems weird to send a command and
wait for the response.
Signed-off-by: Yi Fang <address@hidden>
---
block/block-backend.c | 5 +++++
block/nbd-client.c | 28 ++++++++++++++++++++++++++++
block/nbd-client.h | 1 +
block/nbd.c | 8 ++++++++
include/block/nbd.h | 3 +++
include/sysemu/block-backend.h | 1 +
nbd/server.c | 10 +++++++++-
7 files changed, 55 insertions(+), 1 deletion(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index efbf398..4369c85 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1239,6 +1239,11 @@ void blk_drain_all(void)
bdrv_drain_all();
}
+int blk_has_zero_init(BlockBackend *blk)
+{
+ return bdrv_has_zero_init(blk_bs(blk));
Please run scripts/checkpatch.pl to check for coding style issues.
Indentation should be 4 spaces.
+}
+
void blk_set_on_error(BlockBackend *blk, BlockdevOnError on_read_error,
BlockdevOnError on_write_error)
{
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 3779c6c..8b1d98d 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -376,6 +376,34 @@ void nbd_client_attach_aio_context(BlockDriverState *bs,
false, nbd_reply_ready, NULL, bs);
}
+int nbd_client_co_has_zero_init(BlockDriverState *bs)
Coroutine functions must be marked:
int coroutine_fn nbd_client_co_has_zero_init()
+{
+ NBDClientSession *client = nbd_get_client_session(bs);
+ NBDRequest request = { .type = NBD_CMD_HAS_ZERO_INIT };
+ NBDReply reply;
+ ssize_t ret;
+
+ if (!(client->nbdflags & NBD_FLAG_HAS_ZERO_INIT)) {
+ return 0;
+ }
+
+ request.from = 0;
+ request.len = 0;
+
+ nbd_coroutine_start(client, &request);
+ ret = nbd_co_send_request(bs, &request, NULL);
+ if (ret < 0) {
+ reply.error = -ret;
+ } else {
+ nbd_co_receive_reply(client, &request, &reply, NULL);
+ }
+ nbd_coroutine_end(client, &request);
+ if (reply.error == 0) {
+ return 1;
+ }
+ return 0;
+}
+
void nbd_client_close(BlockDriverState *bs)
{
NBDClientSession *client = nbd_get_client_session(bs);
diff --git a/block/nbd-client.h b/block/nbd-client.h
index f8d6006..ec01938 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -56,5 +56,6 @@ int nbd_client_co_preadv(BlockDriverState *bs, uint64_t
offset,
void nbd_client_detach_aio_context(BlockDriverState *bs);
void nbd_client_attach_aio_context(BlockDriverState *bs,
AioContext *new_context);
+int nbd_client_co_has_zero_init(BlockDriverState *bs);
#endif /* NBD_CLIENT_H */
diff --git a/block/nbd.c b/block/nbd.c
index 35f24be..40dd9a2 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -552,6 +552,11 @@ static void nbd_refresh_filename(BlockDriverState *bs,
QDict *options)
bs->full_open_options = opts;
}
+static int nbd_co_has_zero_init(BlockDriverState *bs)
+{
+ return nbd_client_co_has_zero_init(bs);
+}
+
static BlockDriver bdrv_nbd = {
.format_name = "nbd",
.protocol_name = "nbd",
@@ -569,6 +574,7 @@ static BlockDriver bdrv_nbd = {
.bdrv_detach_aio_context = nbd_detach_aio_context,
.bdrv_attach_aio_context = nbd_attach_aio_context,
.bdrv_refresh_filename = nbd_refresh_filename,
+ .bdrv_has_zero_init = nbd_co_has_zero_init,
.bdrv_has_zero_init() is not a coroutine_fn. It is not okay to yield!
It would be best to fetch the information during connection so that we
don't have to send a command and wait for a reply every time
.bdrv_has_zero_init() is called.