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.

@@ -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,
                      request->len);
-    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.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Reply via email to