From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55156) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YqhYO-0007j4-HB for qemu-devel@nongnu.org; Fri, 08 May 2015 08:38:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YqhYN-0006HU-DV for qemu-devel@nongnu.org; Fri, 08 May 2015 08:38:56 -0400 Message-ID: <554CAE54.60409@redhat.com> Date: Fri, 08 May 2015 14:38:44 +0200 From: Max Reitz MIME-Version: 1.0 References: <1431080484-9760-1-git-send-email-pbonzini@redhat.com> In-Reply-To: <1431080484-9760-1-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=iso-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] qemu-nbd: return ENOSPC for out-of-range writes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-devel@nongnu.org Cc: kwolf@redhat.com, armbru@redhat.com, qemu-stable@nongnu.org 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 > Cc: qemu-stable@nongnu.org > Signed-off-by: Paolo Bonzini > --- > 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) {