[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] nbd/server: introduce NBD_CMD_CACHE

From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-devel] [PATCH] nbd/server: introduce NBD_CMD_CACHE
Date: Thu, 21 Jun 2018 16:13:07 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0

17.05.2018 16:34, Eric Blake wrote:
On 05/17/2018 04:52 AM, Vladimir Sementsov-Ogievskiy wrote:
Finally, what about this?

13.04.2018 17:31, Vladimir Sementsov-Ogievskiy wrote:
Handle nbd CACHE command. Just do read, without sending read data back.
Cache mechanism should be done by exported node driver chain.

Still waiting on the NBD spec review, which I've pinged on the NBD list. But as mentioned there, I'll probably go ahead and accept this (possibly with slight tweaks) on Monday, after giving one more weekend for any last-minute review comments.

Hmm, Eric?

@@ -1826,7 +1826,9 @@ static int nbd_co_receive_request(NBDRequestData *req, NBDRequest *request,
          return -EIO;
-    if (request->type == NBD_CMD_READ || request->type == NBD_CMD_WRITE) { +    if (request->type == NBD_CMD_READ || request->type == NBD_CMD_WRITE ||
+        request->type == NBD_CMD_CACHE)
+    {
          if (request->len > NBD_MAX_BUFFER_SIZE) {
              error_setg(errp, "len (%" PRIu32" ) is larger than max len (%u)",

I'm not sure I agree with this one. Since we aren't passing the cached data over the wire, we can reject the command with EINVAL instead of killing the connection entirely.

(As it is, I wonder if we can be nicer about rejecting a read request > 32M. For a write request, we have to disconnect; but for a read request, we can keep the connection alive by just returning EINVAL for a too-large read, instead of our current behavior of disconnecting)

request->len, NBD_MAX_BUFFER_SIZE);
@@ -1911,7 +1913,7 @@ static coroutine_fn int nbd_do_cmd_read(NBDClient *client, NBDRequest *request,
      int ret;
      NBDExport *exp = client->exp;
-    assert(request->type == NBD_CMD_READ);
+    assert(request->type == NBD_CMD_READ || request->type == NBD_CMD_CACHE);
      /* XXX: NBD Protocol only documents use of FUA with WRITE */
      if (request->flags & NBD_CMD_FLAG_FUA) {
@@ -1930,7 +1932,7 @@ static coroutine_fn int nbd_do_cmd_read(NBDClient *client, NBDRequest *request,
      ret = blk_pread(exp->blk, request->from + exp->dev_offset, data,
-    if (ret < 0) {
+    if (ret < 0 || request->type == NBD_CMD_CACHE) {
          return nbd_send_generic_reply(client, request->handle, ret,
                                        "reading from file failed", errp);

As for the implementation on the server side, yes, this looks reasonable, given the proposed spec wording being considered on the NBD list.

Best regards,

reply via email to

[Prev in Thread] Current Thread [Next in Thread]