From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58539) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1angTX-00085V-1f for qemu-devel@nongnu.org; Wed, 06 Apr 2016 01:58:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1angTT-0006Wm-QU for qemu-devel@nongnu.org; Wed, 06 Apr 2016 01:57:58 -0400 Received: from mail-db3on0104.outbound.protection.outlook.com ([157.55.234.104]:7680 helo=emea01-db3-obe.outbound.protection.outlook.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1angTT-0006WV-64 for qemu-devel@nongnu.org; Wed, 06 Apr 2016 01:57:55 -0400 References: <1459787950-15286-1-git-send-email-eblake@redhat.com> <20160404224039.GB32049@grep.be> <5702F2B5.8060206@redhat.com> From: "Denis V. Lunev" Message-ID: <5704A55B.40804@openvz.org> Date: Wed, 6 Apr 2016 08:57:47 +0300 MIME-Version: 1.0 In-Reply-To: <5702F2B5.8060206@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , Wouter Verhelst Cc: nbd-general@lists.sourceforge.net, Kevin Wolf , qemu-devel@nongnu.org, Pavel Borzenkov , Stefan Hajnoczi , Paolo Bonzini On 04/05/2016 02:03 AM, Eric Blake wrote: > On 04/04/2016 04:40 PM, Wouter Verhelst wrote: >> Hi, >> >> Need to look into this in some detail, for which I don't have the time >> (or the non-tiredness ;-) right now, but these two caught my eye: >> >>> + The payload is structured as a list of one or more descriptors, >>> + each with this layout: >>> + >>> + * 32 bits, length (unsigned, MUST NOT be zero) >>> + * 32 bits, status flags >>> + >>> + The definition of the status flags is determined based on the >>> + flags present in the original request. >> Might be a good idea to specify what a client can do with flags it >> doesn't know about; ignore them, probably? > Sounds correct - so a server can subdivide into more descriptors with > additional status bits, and clients just have to ignore those extra bits > and coalesce things itself when dealing with the bits it cares about. > >> >> [...] >>> +The extension adds the following new command flag: >>> + >>> +- `NBD_CMD_FLAG_STATUS_DIRTY`; valid during `NBD_CMD_BLOCK_STATUS`. >>> + SHOULD be set to 1 if the client wants to request dirtiness status >>> + rather than provisioning status. >> Why only one flag here? I could imagine a client might want to query for >> both states at the same time. Obviously that means a client needs to >> query for *at least* one of the statuses, otherwise the server should >> reply with EINVAL. >> >> Though I'm undecided on whether a bit being set to 0 should mean "give >> me that information" or whether 1 should. > Hmm. Based on that reading, maybe we want TWO flags: > > NBD_CMD_FLAG_STATUS_ALLOCATED > NBD_CMD_FLAG_STATUS_DIRTY > > and require that the client MUST set at least one flag (setting no flags > at all is boring), but similarly allow that the server MAY reject > attempts to set multiple flags with EINVAL (if there is no efficient way > to provide the information for all flags on a single pass), in which > case clients have to fall back to querying one flag at a time. > > But while Alex and Denis were arguing that no one would ever query both > things at once (and therefore, it might be better to make > NBD_STATUS_HOLE and NBD_STATUS_CLEAN both be bit 0), your approach of > having two separate request flags and allowing both at once would mean > we do need to keep the status information separate (NBD_STATUS_HOLE is > bit 0, NBD_STATUS_CLEAN is bit 2). > > I also see that I failed to reserve a bit for NBD_CMD_FLAG_STATUS_DIRTY. > > Looks like there will still be some more conversation and at least a v3 > needed, but I'll wait a couple days for that to happen so that more > reviewers can chime in, without being too tired during the review process. > that looks correct to me. I agree that we should set only one flag and reject the request with two of them. Actually this is like "bitmap number", but we have limitation with 32 numbers only. We could specify that the server MUST reply with "all 1" for unknown flag. This would provide nice forward compatibility. Den