qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH] qemu-nbd: return ENOSPC for out-of-range writes


From: Max Reitz
Subject: Re: [Qemu-devel] [PATCH] qemu-nbd: return ENOSPC for out-of-range writes
Date: Fri, 08 May 2015 14:38:44 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0

On 08.05.2015 12:21, Paolo Bonzini wrote:
This ensures that werror=enospc works fine for NBD-backed devices.
Recovery can be done through live snapshots even if the NBD server
does not support online resizing.

Suggested-by: Kevin Wolf <address@hidden>
Cc: address@hidden
Signed-off-by: Paolo Bonzini <address@hidden>
---
  nbd.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

Good thing: NBD doesn't have a very extensive manual, so in theory we can do whatever we want to. Bad thing: Because of this, our implementation generally seems to look to the reference implementation (nbd-server).

Well, and that reference implementation handles this case here: https://github.com/yoe/nbd/blob/master/nbd-server.c#l1621 - so it does always send EINVAL.

Two things follow: First, our implementation will behave differently from nbd-server. Second, even if we don't care, werror=enospc will only work with qemu-nbd, and not when using nbd-server.

One could argue that you should only use qemu-nbd with qemu, and not nbd-server, but then I'm wondering why we implemented a pre-existing protocol in the first place and didn't implement our own...

I think this should be done in the client. If a request beyond the EOF (for writes) is to be generated, ENOSPC should be returned before making the request to the server at all.

Alternatively, we could actually use my series which introduces the debatable (and debated) warning to implement this case more generally: It does introduce a flag which tells the block layer whether a BDS supports write accesses beyond EOF or not, so using that the block layer itself would be able to intercept such writes and return ENOSPC automatically.

Max

diff --git a/nbd.c b/nbd.c
index 57d71b2..a04ba80 100644
--- a/nbd.c
+++ b/nbd.c
@@ -1297,7 +1297,8 @@ static void nbd_trip(void *opaque)
                      request.from, request.len,
                      (uint64_t)exp->size, (uint64_t)exp->dev_offset);
          LOG("requested operation past EOF--bad client?");
-        goto invalid_request;
+        reply.error = (command == NBD_CMD_WRITE) ? ENOSPC : EINVAL;
+        goto error_reply;
      }
switch (command) {
@@ -1390,7 +1391,6 @@ static void nbd_trip(void *opaque)
          break;
      default:
          LOG("invalid request type (%u) received", request.type);
-    invalid_request:
          reply.error = EINVAL;
      error_reply:
          if (nbd_co_send_reply(req, &reply, 0) < 0) {




reply via email to

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