|
From: | Eric Blake |
Subject: | Re: [Qemu-devel] [PATCH 1/1] nbd: fix NBD_CMD_CACHE negitiation according to the NBD specification |
Date: | Thu, 4 Oct 2018 08:49:37 -0500 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0 |
On 10/4/18 7:51 AM, Vladimir Sementsov-Ogievskiy wrote:
04.10.2018 15:27, Eric Blake wrote:On 10/4/18 5:03 AM, Denis V. Lunev wrote:Commit bc37b06a5 was made very bad thing, it has been added NBD_FLAG_SEND_CACHE flag for negotiation.Oof. Probably my fault for not doing a careful review against the upstream spec.mostly my, to introduce this =(. really too badThe problem is that the value of the flag was taken wrong and directly violates NBD specification. This value (bit 8) is used at least in the Linux kernel as a part of stable userspace-kernel API since 4.10 as NBD_FLAG_CAN_MULTI_CONN as defined in the specification:And a kernel that honors that bit can cause qemu to misbehave. Yeah, that's definitely undesirable.As I understand opposite: kernel client will assume support for multi_conn feature which declares some guarantees about FLUSH, but qemu don't give these guarantees.
Yeah, that's my biggest worry - data corruption when the kernel assumes it can make multiple connections with consistent caching, but the caching is not consistent, leading to data corruption when connected to an unpatched qemu 3.0 server. I'm rewording the commit message along those lines.
I'll probably amend this to list all NBD_FLAG_ values in the spec (even if qemu doesn't implement them yet), just to make it easier to avoid making this mistake in the future. Reviewed-by: Eric Blake <address@hidden> Will queue through my NBD tree.I vote for adding all values at least as a comment Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
[Prev in Thread] | Current Thread | [Next in Thread] |