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

2016-04-04 Thread Wouter Verhelst
On Mon, Apr 04, 2016 at 12:18:37PM +0200, Markus Pargmann wrote: > Hi, > > back from my easter vacation. A bit surprised to find 200 mails in the > NBD mailing list ;). I'm sure you were :-) [...] > > Yes. This has been discussed on the nbd-general list in the past. There > > is also the

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

2016-04-04 Thread Eric Blake
On 04/04/2016 02:16 PM, Denis V. Lunev wrote: >> +The following request types are currently defined for the command: >> + >> +1. Block provisioning state >> + >> +Upon receiving an `NBD_CMD_GET_LBA_STATUS` command with command flags >> +field set to `NBD_FLAG_GET_ALLOCATED` (0x0),

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

2016-04-04 Thread Eric Blake
On 04/04/2016 04:18 AM, Markus Pargmann wrote: > Hi, > > back from my easter vacation. A bit surprised to find 200 mails in the > NBD mailing list ;). > >> Yes. This has been discussed on the nbd-general list in the past. There >> is also the (significant) problem of the server having maybe

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

2016-04-04 Thread Markus Pargmann
Hi, On Monday 28 March 2016 09:58:52 Eric Blake wrote: > On 03/25/2016 02:49 AM, Wouter Verhelst wrote: > > >> You may also want to add a rule that for all future extensions, any > >> command that requires data in the server response (other than the server > >> response to NBD_CMD_READ) must

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

2016-04-04 Thread Markus Pargmann
Hi, back from my easter vacation. A bit surprised to find 200 mails in the NBD mailing list ;). On Friday 25 March 2016 09:49:29 Wouter Verhelst wrote: > On Thu, Mar 24, 2016 at 04:08:13PM -0600, Eric Blake wrote: > > On 03/23/2016 08:16 AM, Denis V. Lunev wrote: > > > From: Pavel Borzenkov

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

2016-03-29 Thread Wouter Verhelst
On Tue, Mar 29, 2016 at 11:38:35AM +0200, Kevin Wolf wrote: > Am 24.03.2016 um 17:47 hat Wouter Verhelst geschrieben: > > 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: > >

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

2016-03-29 Thread Paolo Bonzini
On 29/03/2016 11:38, Kevin Wolf wrote: > > > How about NBD_STATE_HOLE? > > > > Both will work, although I like NBD_STATE_TRIM slightly better because > > it indeed nicely references NBD_CMD_TRIM. > > I just thought that "trim" sounds more like an action than a status, and > while the reason

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

2016-03-29 Thread Kevin Wolf
Am 24.03.2016 um 17:47 hat Wouter Verhelst geschrieben: > 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: >

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

2016-03-28 Thread Eric Blake
On 03/25/2016 02:49 AM, Wouter Verhelst wrote: >> You may also want to add a rule that for all future extensions, any >> command that requires data in the server response (other than the server >> response to NBD_CMD_READ) must include a 32-bit length as the first >> field of its data payload, so

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

2016-03-25 Thread Alex Bligh
On 25 Mar 2016, at 08:49, Wouter Verhelst wrote: > Yes. This has been discussed on the nbd-general list in the past. There > is also the (significant) problem of the server having maybe already > sent out the header before discovering there is an error, at which point > it can

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

2016-03-25 Thread Wouter Verhelst
On Thu, Mar 24, 2016 at 04:08:13PM -0600, Eric Blake wrote: > 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

Re: [Qemu-devel] [Nbd] [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: [Qemu-devel] [Nbd] [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: [Qemu-devel] [Nbd] [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: [Qemu-devel] [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: [Qemu-devel] [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension

2016-03-24 Thread Eric Blake
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), LBA extent is present on the block device NBD_STATE_ZERO

Re: [Qemu-devel] [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: [Qemu-devel] [Nbd] [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: [Qemu-devel] [Nbd] [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: [Qemu-devel] [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: [Qemu-devel] [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: [Qemu-devel] [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: [Qemu-devel] [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: [Qemu-devel] [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: [Qemu-devel] [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: [Qemu-devel] [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: [Qemu-devel] [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: [Qemu-devel] [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: [Qemu-devel] [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: [Qemu-devel] [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: [Qemu-devel] [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension

2016-03-24 Thread Pavel Borzenkov
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 type of information required by the client is passed to server > > > in the > > > +command

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

2016-03-23 Thread Kevin Wolf
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 type of information required by the client is passed to server in > > the > > +command flags field. If the server does not implement requested type or > > +

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

2016-03-23 Thread Wouter Verhelst
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 LBA range and read only those blocks of > data that are actually present on