Re: [Nbd] [PATCH 1/2] NBD proto: add WRITE_ZEROES extension

2016-03-24 Thread Paolo Bonzini
On 24/03/2016 09:26, Wouter Verhelst wrote: >> > >> > No, there is no specific reason. Looks like NBD_CMD_FLAG_ZEROES fits the >> > spec and implementations nicely. So I'll rewrite the extension and add >> > the flag instead of the whole command. > Actually, having given this some more

Re: [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension

2016-03-24 Thread Paolo Bonzini
On 24/03/2016 11:32, Alex Bligh wrote: >> > Now I'm not saying we >> > need to fully define what it means for a part of the backend to be >> > "dirty" or not. It's okay to leave part of the meaning in the dark, >> > leaving that implementation-defined. > Well, the 3 possible states are: > > *

Re: [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension

2016-03-24 Thread Wouter Verhelst
On Thu, Mar 24, 2016 at 11:43:18AM +0300, Pavel Borzenkov wrote: > On Wed, Mar 23, 2016 at 06:58:34PM +0100, Wouter Verhelst wrote: > > On Wed, Mar 23, 2016 at 05:16:02PM +0300, Denis V. Lunev wrote: > > > +2. Block dirtiness state > > > + > > > +Upon receiving an `NBD_CMD_GET_LBA_STATUS`

Re: [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension

2016-03-24 Thread Pavel Borzenkov
On Thu, Mar 24, 2016 at 09:41:29AM +0100, Wouter Verhelst wrote: > On Thu, Mar 24, 2016 at 11:25:52AM +0300, Pavel Borzenkov wrote: > > On Wed, Mar 23, 2016 at 07:14:54PM +0100, Kevin Wolf wrote: > > > Am 23.03.2016 um 18:58 hat Wouter Verhelst geschrieben: > > > > On Wed, Mar 23, 2016 at

Re: [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension

2016-03-24 Thread Alex Bligh
On 24 Mar 2016, at 11:58, Paolo Bonzini wrote: > > On 24/03/2016 11:32, Alex Bligh wrote: Now I'm not saying we need to fully define what it means for a part of the backend to be "dirty" or not. It's okay to leave part of the meaning in the dark,

Re: [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension

2016-03-24 Thread Paolo Bonzini
On 23/03/2016 18:58, Wouter Verhelst wrote: >> +To provide such class of information, `GET_LBA_STATUS` extension adds new >> +`NBD_CMD_GET_LBA_STATUS` command which returns a list of LBA ranges with >> +their respective states. >> + >> +* `NBD_CMD_GET_LBA_STATUS` (7) >> + >> +An LBA range

Re: [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension

2016-03-24 Thread Alex Bligh
On 24 Mar 2016, at 12:32, Paolo Bonzini wrote: > On 24/03/2016 13:17, Alex Bligh wrote: >> * unallocated >> * zero >> * non-zero >> >> So the possible replies are a bitfield of those, with a '1' if it 'might' >> be in that state, i.e. >> >>

Re: [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension

2016-03-24 Thread Paolo Bonzini
On 24/03/2016 14:31, Alex Bligh wrote: > Sorry, I should have been clearer on the states: > > bits > 210 > > 1-- Unallocated, and hence reads as zero > -1- Allocated, and reads as zero > --1 Allocated, and reads as non-zero > > So 100 means 'definitely unallocated, will read as zero'. > > If

Re: [Nbd] [Qemu-devel] [PATCH 1/2] NBD proto: add WRITE_ZEROES extension

2016-03-24 Thread Eric Blake
On 03/24/2016 02:26 AM, Wouter Verhelst wrote: >> No, there is no specific reason. Looks like NBD_CMD_FLAG_ZEROES fits the >> spec and implementations nicely. So I'll rewrite the extension and add >> the flag instead of the whole command. > > Actually, having given this some more thought... > >

Re: [Nbd] [Qemu-devel] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension

2016-03-24 Thread Eric Blake
On 03/24/2016 06:30 AM, Pavel Borzenkov wrote: >> Conversely, it would be possible to send less data over the wire, as >> long as we require that all LBA status descriptors cover consecutive >> offsets. That is, having the server reply with offsets is pointless, >> since they can be reconstructed

Re: [Nbd] [Qemu-devel] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension

2016-03-24 Thread Kevin Wolf
Am 24.03.2016 um 17:04 hat Eric Blake geschrieben: > On 03/24/2016 09:53 AM, Wouter Verhelst wrote: > > On Thu, Mar 24, 2016 at 04:33:42PM +0100, Paolo Bonzini wrote: > >> On 24/03/2016 16:25, Eric Blake wrote: > However, let's make these bits, so that > > NBD_STATE_ALLOCATED (0x1),

Re: [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension

2016-03-24 Thread Wouter Verhelst
Hi Paolo, On Thu, Mar 24, 2016 at 12:55:51PM +0100, Paolo Bonzini wrote: > > > On 23/03/2016 18:58, Wouter Verhelst wrote: > >> +To provide such class of information, `GET_LBA_STATUS` extension adds new > >> +`NBD_CMD_GET_LBA_STATUS` command which returns a list of LBA ranges with > >> +their

Re: [Nbd] [PATCH 1/2] NBD proto: add WRITE_ZEROES extension

2016-03-24 Thread Wouter Verhelst
On Thu, Mar 24, 2016 at 12:37:33PM +0100, Paolo Bonzini wrote: > > > On 24/03/2016 09:26, Wouter Verhelst wrote: > >> > > >> > No, there is no specific reason. Looks like NBD_CMD_FLAG_ZEROES fits the > >> > spec and implementations nicely. So I'll rewrite the extension and add > >> > the flag

Re: [Nbd] [Qemu-devel] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension

2016-03-24 Thread Eric Blake
On 03/24/2016 05:55 AM, Paolo Bonzini wrote: >> As Eric noted, please expand LBA at least once. > > Let's just use "block" (e.g. NBD_CMD_GET_BLOCK_STATUS). Yes, avoiding the term LBA and using BLOCK everywhere also nicely solves the problem of introducing yet more terminology. > >>> + -

Re: [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension

2016-03-24 Thread Wouter Verhelst
On Thu, Mar 24, 2016 at 02:36:52PM +0300, Pavel Borzenkov wrote: > On Thu, Mar 24, 2016 at 09:41:29AM +0100, Wouter Verhelst wrote: > > Okay, that alleviates my concerns. > > > > In that case it might be useful if the server could say something along > > the lines of "I know it's allocated, but I

Re: [Nbd] [Qemu-devel] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension

2016-03-24 Thread Wouter Verhelst
On Thu, Mar 24, 2016 at 04:33:42PM +0100, Paolo Bonzini wrote: > On 24/03/2016 16:25, Eric Blake wrote: > >> However, let's make these bits, so that > >> > >> NBD_STATE_ALLOCATED (0x1), LBA extent is present on the block device > >> NBD_STATE_ZERO (0x2), LBA extent will read as zeroes > > > >

Re: [Nbd] [Qemu-devel] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension

2016-03-24 Thread Wouter Verhelst
On Thu, Mar 24, 2016 at 05:07:47PM +0100, Kevin Wolf wrote: > Am 24.03.2016 um 17:04 hat Eric Blake geschrieben: > > On 03/24/2016 09:53 AM, Wouter Verhelst wrote: > > > On Thu, Mar 24, 2016 at 04:33:42PM +0100, Paolo Bonzini wrote: > > >> On 24/03/2016 16:25, Eric Blake wrote: > > However,

Re: [Nbd] [Qemu-devel] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension

2016-03-24 Thread Pavel Borzenkov
On Thu, Mar 24, 2016 at 09:04:07AM -0600, Eric Blake wrote: > On 03/24/2016 06:30 AM, Pavel Borzenkov wrote: > >> Conversely, it would be possible to send less data over the wire, as > >> long as we require that all LBA status descriptors cover consecutive > >> offsets. That is, having the server

Re: [Nbd] [Qemu-devel] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension

2016-03-24 Thread Paolo Bonzini
On 24/03/2016 16:25, Eric Blake wrote: >> However, let's make these bits, so that >> >> NBD_STATE_ALLOCATED (0x1), LBA extent is present on the block device >> NBD_STATE_ZERO (0x2), LBA extent will read as zeroes > > Should we flip the sense and call this NBD_STATE_UNALLOCATED (0 means >

Re: [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension

2016-03-24 Thread Alex Bligh
On 24 Mar 2016, at 09:33, Wouter Verhelst wrote: > Now I'm not saying we > need to fully define what it means for a part of the backend to be > "dirty" or not. It's okay to leave part of the meaning in the dark, > leaving that implementation-defined. Well, the 3 possible states

Re: [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension

2016-03-24 Thread Paolo Bonzini
On 24/03/2016 13:17, Alex Bligh wrote: >>> >> * unallocated >>> >> * zero >>> >> * non-zero >>> >> >>> >> So the possible replies are a bitfield of those, with a '1' if it 'might' >>> >> be in that state, i.e. >>> >> >>> >> 111 = no idea >>> >> 110 = might be zero or unallocated, but isn't

Re: [Nbd] [Qemu-devel] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension

2016-03-24 Thread Eric Blake
On 03/23/2016 08:16 AM, Denis V. Lunev wrote: > From: Pavel Borzenkov > > With the availability of sparse storage formats, it is often needed to > query status of a particular LBA range and read only those blocks of > data that are actually present on the block device.

[Nbd] [PATCH] Fix doc typos

2016-03-24 Thread Eric Blake
Fix the spelling of "beef", and the case of an "NBD_REP" constant. Signed-off-by: Eric Blake --- I noticed these while reviewing the recent proposed spec additions for efficient sparse file handling doc/proto.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

Re: [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension

2016-03-24 Thread Pavel Borzenkov
On Wed, Mar 23, 2016 at 06:58:34PM +0100, Wouter Verhelst wrote: > On Wed, Mar 23, 2016 at 05:16:02PM +0300, Denis V. Lunev wrote: > > From: Pavel Borzenkov > > > > With the availability of sparse storage formats, it is often needed to > > query status of a particular

Re: [Nbd] [PATCH 1/2] NBD proto: add WRITE_ZEROES extension

2016-03-24 Thread Wouter Verhelst
On Thu, Mar 24, 2016 at 10:57:06AM +0300, Pavel Borzenkov wrote: > On Wed, Mar 23, 2016 at 06:21:16PM +0100, Wouter Verhelst wrote: > > So, the semantics of your proposed WRITE_ZEROES are exactly the same as > > the WRITE command, except that no payload is sent? > > > > In that case, I think it's

Re: [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension

2016-03-24 Thread Wouter Verhelst
On Thu, Mar 24, 2016 at 11:25:52AM +0300, Pavel Borzenkov wrote: > On Wed, Mar 23, 2016 at 07:14:54PM +0100, Kevin Wolf wrote: > > Am 23.03.2016 um 18:58 hat Wouter Verhelst geschrieben: > > > On Wed, Mar 23, 2016 at 05:16:02PM +0300, Denis V. Lunev wrote: > > > > +the provisioning state of

Re: [Nbd] [Qemu-devel] [PATCH 1/2] NBD proto: add WRITE_ZEROES extension

2016-03-24 Thread Pavel Borzenkov
On Wed, Mar 23, 2016 at 09:14:10AM -0600, Eric Blake wrote: > On 03/23/2016 08:16 AM, Denis V. Lunev wrote: > > From: Pavel Borzenkov > > > > There exist some cases when a client knows that the data it is going to > > write is all zeroes. Such cases include mirroring or

Re: [Nbd] [Qemu-devel] [PATCH 1/2] NBD proto: add WRITE_ZEROES extension

2016-03-24 Thread Wouter Verhelst
On Thu, Mar 24, 2016 at 10:16:19AM +0300, Pavel Borzenkov wrote: > So if no one objects, I'll send a patch correcting current spec > ambiguities and than patches with new proposed commands. Yes please. Thank you. -- < ron> I mean, the main *practical* problem with C++, is there's like a dozen