Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-10 Thread Markus Pargmann
On Tuesday 05 April 2016 22:50:51 Wouter Verhelst wrote: > On Tue, Apr 05, 2016 at 08:14:01AM -0600, Eric Blake wrote: > > On 04/05/2016 03:24 AM, Markus Pargmann wrote: > > > > >> +requested. > > >> + > > >> +The client SHOULD NOT read from an area that has both > > >> +`NBD_STATE

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-10 Thread Markus Pargmann
On Tuesday 05 April 2016 15:50:16 Paolo Bonzini wrote: > > On 05/04/2016 11:24, Markus Pargmann wrote: > > Also it is uncertain if these status bits may change over time through > > reorganization of backend storage, for example holes may be removed in > > the backend and so on. Is it safe to cach

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-09 Thread Wouter Verhelst
On Thu, Apr 07, 2016 at 10:10:58AM -0600, Eric Blake wrote: > On 04/07/2016 04:38 AM, Vladimir Sementsov-Ogievskiy wrote: > > On 05.04.2016 16:43, Paolo Bonzini wrote: > >> > >> On 05/04/2016 06:05, Kevin Wolf wrote: > >>> The options I can think of is adding a request field "max number of > >>> de

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-07 Thread Alex Bligh
On 7 Apr 2016, at 17:10, Eric Blake wrote: > I'm also starting to think that it is worth FIRST documenting an > extension for advertising block sizes, so that we can then couch > BLOCK_STATUS in those terms (a server MUST NOT subdivide status into > finer granularity than the advertised block si

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-06 Thread Eric Blake
On 04/05/2016 11:57 PM, Denis V. Lunev wrote: >> 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 t

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-05 Thread Denis V. Lunev
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 descrip

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-05 Thread Wouter Verhelst
On Mon, Apr 04, 2016 at 05:32:34PM -0600, Eric Blake wrote: > On 04/04/2016 05:08 PM, Wouter Verhelst wrote: > > On Mon, Apr 04, 2016 at 10:54:02PM +0300, Denis V. Lunev wrote: > >> saying about dirtiness, we would soon come to the fact, that > >> we can have several dirtiness states regarding diff

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-05 Thread Wouter Verhelst
On Tue, Apr 05, 2016 at 08:14:01AM -0600, Eric Blake wrote: > On 04/05/2016 03:24 AM, Markus Pargmann wrote: > > >> +requested. > >> + > >> +The client SHOULD NOT read from an area that has both > >> +`NBD_STATE_HOLE` set and `NBD_STATE_ZERO` clear. > > > > Why not? If we don't ex

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-05 Thread Paolo Bonzini
On 05/04/2016 17:27, Alex Bligh wrote: >>> I'm missing how the ugh-author can write the software without knowing >>> exactly what the bit does. Or are you saying "that's a matter >>> for the qemu spec, not the nbd spec"? > > > > Yes, that's it. NBD defines a safe default and a general idea of w

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-05 Thread Alex Bligh
On 5 Apr 2016, at 16:23, Paolo Bonzini wrote: >> I'm missing how the ugh-author can write the software without knowing >> exactly what the bit does. Or are you saying "that's a matter >> for the qemu spec, not the nbd spec"? > > Yes, that's it. NBD defines a safe default and a general idea of

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-05 Thread Paolo Bonzini
On 05/04/2016 17:01, Alex Bligh wrote: >>> >> So qemu-nbdserver has some idea of 'dirtiness', and you want >>> >> to publish that, and the consumer is qemu (and only qemu), >>> >> because only qemu knows what 'dirtiness' means in the >>> >> sense the server provides it? >> > >> > The consumer is

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-05 Thread Alex Bligh
On 5 Apr 2016, at 15:15, Paolo Bonzini wrote: > > On 04/04/2016 22:03, Alex Bligh wrote: >> So qemu-nbdserver has some idea of 'dirtiness', and you want >> to publish that, and the consumer is qemu (and only qemu), >> because only qemu knows what 'dirtiness' means in the >> sense the server pro

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-05 Thread Eric Blake
On 04/05/2016 03:24 AM, Markus Pargmann wrote: >> +requested. >> + >> +The client SHOULD NOT read from an area that has both >> +`NBD_STATE_HOLE` set and `NBD_STATE_ZERO` clear. > > Why not? If we don't execute CMD_BLOCK_STATUS we wouldn't know about the > situation and would simp

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-05 Thread Paolo Bonzini
On 04/04/2016 22:03, Alex Bligh wrote: > So qemu-nbdserver has some idea of 'dirtiness', and you want > to publish that, and the consumer is qemu (and only qemu), > because only qemu knows what 'dirtiness' means in the > sense the server provides it? The consumer is not QEMU; the consumer is bac

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-05 Thread Paolo Bonzini
On 05/04/2016 11:24, Markus Pargmann wrote: > Also it is uncertain if these status bits may change over time through > reorganization of backend storage, for example holes may be removed in > the backend and so on. Is it safe to cache this stuff? If there's no concurrent access, it is. Even if

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-05 Thread Paolo Bonzini
On 05/04/2016 01:03, Eric Blake wrote: > > 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

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-05 Thread Paolo Bonzini
On 04/04/2016 21:34, Eric Blake wrote: > At this point, I was just trying to rebase the proposal as originally > made by Denis and Pavel; perhaps they will have more insight on how they > envisioned using the command, or on whether we should try harder to make > this more of a two-way protocol (w

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-05 Thread Markus Pargmann
Hi, On Monday 04 April 2016 10:39:10 Eric Blake wrote: > With the availability of sparse storage formats, it is often needed > to query status of a particular range and read only those blocks of > data that are actually present on the block device. > > To provide such information, the patch adds

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-05 Thread Wouter Verhelst
On Mon, Apr 04, 2016 at 05:32:34PM -0600, Eric Blake wrote: > On 04/04/2016 05:08 PM, Wouter Verhelst wrote: > > On Mon, Apr 04, 2016 at 10:54:02PM +0300, Denis V. Lunev wrote: > >> saying about dirtiness, we would soon come to the fact, that > >> we can have several dirtiness states regarding diff

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-05 Thread Alex Bligh
On 5 Apr 2016, at 00:08, Wouter Verhelst wrote: > > if bit 0 in the latter field is set, that means the block is allocated > on the original device > if bit 1 is set, that means the block is allocated on the first-level > snapshot > if bit 2 is set, that means the block is allocated on the se

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Eric Blake
On 04/04/2016 05:08 PM, Wouter Verhelst wrote: > On Mon, Apr 04, 2016 at 10:54:02PM +0300, Denis V. Lunev wrote: >> saying about dirtiness, we would soon come to the fact, that >> we can have several dirtiness states regarding different >> lines of incremental backups. This complexity is hidden >>

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Wouter Verhelst
On Mon, Apr 04, 2016 at 10:54:02PM +0300, Denis V. Lunev wrote: > saying about dirtiness, we would soon come to the fact, that > we can have several dirtiness states regarding different > lines of incremental backups. This complexity is hidden > inside QEMU and it would be very difficult to publish

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Eric Blake
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 lay

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Wouter Verhelst
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: On Mon, Apr 04, 2016 at 10:39:10AM -0600, Eric Blake wrote: [...] > +* `NBD_REPLY_TYPE_BLOCK_STATUS` > + > +*length* MUST be a positive integer multiple

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Alex Bligh
On 4 Apr 2016, at 22:25, Eric Blake wrote: > On 04/04/2016 03:07 PM, Alex Bligh wrote: >> >> On 4 Apr 2016, at 21:26, Eric Blake wrote: >> >>> Huh? The current proposal _requires_ these to be separate queries. You >>> cannot query dirtiness at the same time as allocation, because the value >

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Denis V. Lunev
On 04/05/2016 12:17 AM, Eric Blake wrote: On 04/04/2016 03:04 PM, Denis V. Lunev wrote: In v1 we have had 'status' field, which can have the following values for dirty request: + - `NBD_STATE_DIRTY` (0x0), LBA extent is dirty; + - `NBD_STATE_CLEAN` (0x1), LBA extent is clean. in the

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Eric Blake
On 04/04/2016 03:07 PM, Alex Bligh wrote: > > On 4 Apr 2016, at 21:26, Eric Blake wrote: > >> Huh? The current proposal _requires_ these to be separate queries. You >> cannot query dirtiness at the same time as allocation, because the value >> of NBD_CMD_FLAG_DIRTY is distinct between the two q

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Eric Blake
On 04/04/2016 03:04 PM, Denis V. Lunev wrote: > In v1 we have had 'status' field, which can have the > following values for dirty request: > > + - `NBD_STATE_DIRTY` (0x0), LBA extent is dirty; > + - `NBD_STATE_CLEAN` (0x1), LBA extent is clean. > > in the extent structure: > > +

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Alex Bligh
On 4 Apr 2016, at 22:04, Denis V. Lunev wrote: > and opinion of Alex was that all 3 bits could be set in reply to > NBD_CMD_BLOCK_STATUS > with NBD_CMD_FLAG_STATUS_DIRTY set. > > This confused him. This confuses me too. Yeah that was precisely how I got confused. -- Alex Bligh

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Alex Bligh
On 4 Apr 2016, at 21:34, Eric Blake wrote: > The original design abused the 16-bit 'flags' field of each command to > instead try and treat that value as a bitmap number, instead of a > bitwise-or'd set of flags. That was one of the complaints against v1, > and was fixed in v2 by having a singl

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Alex Bligh
On 4 Apr 2016, at 21:26, Eric Blake wrote: > Huh? The current proposal _requires_ these to be separate queries. You > cannot query dirtiness at the same time as allocation, because the value > of NBD_CMD_FLAG_DIRTY is distinct between the two queries. OK, so you can ask for either (1) or (2),

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Denis V. Lunev
On 04/04/2016 11:34 PM, Eric Blake wrote: On 04/04/2016 02:08 PM, Denis V. Lunev wrote: This again makes me think this should be a different command from something which is obviously useful and comprehensible to more than one server/client (i.e. allocation). original design of this command has

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Denis V. Lunev
On 04/04/2016 11:45 PM, Eric Blake wrote: On 04/04/2016 02:27 PM, Denis V. Lunev wrote: On 04/04/2016 11:15 PM, Alex Bligh wrote: On 4 Apr 2016, at 21:13, Denis V. Lunev wrote: As far as I remember that text we have had a number in request specifying which bitmap to query and the server shou

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Eric Blake
On 04/04/2016 02:27 PM, Denis V. Lunev wrote: > On 04/04/2016 11:15 PM, Alex Bligh wrote: >> On 4 Apr 2016, at 21:13, Denis V. Lunev wrote: >> >>> As far as I remember that text we have had a number in request >>> specifying which bitmap to query and the server should reply with one >>> bitmap at

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Eric Blake
On 04/04/2016 02:08 PM, Denis V. Lunev wrote: >> >> This again makes me think this should be a different >> command from something which is obviously useful and >> comprehensible to more than one server/client (i.e. >> allocation). >> > original design of this command has used 16 number > to specif

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Denis V. Lunev
On 04/04/2016 11:15 PM, Alex Bligh wrote: On 4 Apr 2016, at 21:13, Denis V. Lunev wrote: As far as I remember that text we have had a number in request specifying which bitmap to query and the server should reply with one bitmap at a time. Would this work for you? I think that would be much

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Eric Blake
On 04/04/2016 02:08 PM, Alex Bligh wrote: > > On 4 Apr 2016, at 21:04, Denis V. Lunev wrote: > >>> Sure, but given you can't report dirtiness without also reporting >>> allocation, if they are are at different blocksize I'd rather they >>> were in different commands, because otherwise the code t

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Eric Blake
On 04/04/2016 01:58 PM, Alex Bligh wrote: > Eric, > >> Nothing requires the two uses to report at the same granularity. THe >> NBD_REPLY_TYPE_BLOCK_STATUS allows the server to divide into descriptors >> as it sees fit (so it could report holes at a 4k granularity, but >> dirtiness only at a 64k g

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Alex Bligh
On 4 Apr 2016, at 21:13, Denis V. Lunev wrote: > As far as I remember that text we have had a number in request > specifying which bitmap to query and the server should reply with one > bitmap at a time. > > Would this work for you? I think that would be much better, yes, though I'd suggest th

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Denis V. Lunev
On 04/04/2016 11:08 PM, Alex Bligh wrote: On 4 Apr 2016, at 21:04, Denis V. Lunev wrote: Sure, but given you can't report dirtiness without also reporting allocation, if they are are at different blocksize I'd rather they were in different commands, because otherwise the code to report block s

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Denis V. Lunev
On 04/04/2016 11:03 PM, Alex Bligh wrote: On 4 Apr 2016, at 20:54, Denis V. Lunev wrote: for now and for QEMU we want this to expose accumulated dirtiness of the block device, which is collected by the server. Yes, this requires external coordination. May be this COULD be the part of the proto

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Alex Bligh
On 4 Apr 2016, at 21:04, Denis V. Lunev wrote: >> Sure, but given you can't report dirtiness without also reporting >> allocation, if they are are at different blocksize I'd rather they >> were in different commands, because otherwise the code to report >> block size needs to work at two differe

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Denis V. Lunev
On 04/04/2016 10:58 PM, Alex Bligh wrote: Eric, Nothing requires the two uses to report at the same granularity. THe NBD_REPLY_TYPE_BLOCK_STATUS allows the server to divide into descriptors as it sees fit (so it could report holes at a 4k granularity, but dirtiness only at a 64k granularity) -

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Alex Bligh
On 4 Apr 2016, at 20:54, Denis V. Lunev wrote: > for now and for QEMU we want this to expose accumulated dirtiness > of the block device, which is collected by the server. Yes, this requires > external coordination. May be this COULD be the part of the protocol, > but QEMU will not use that part

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Alex Bligh
Eric, > Nothing requires the two uses to report at the same granularity. THe > NBD_REPLY_TYPE_BLOCK_STATUS allows the server to divide into descriptors > as it sees fit (so it could report holes at a 4k granularity, but > dirtiness only at a 64k granularity) - all that matters is that when all >

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Denis V. Lunev
On 04/04/2016 10:34 PM, Eric Blake wrote: On 04/04/2016 12:06 PM, Alex Bligh wrote: On 4 Apr 2016, at 17:39, Eric Blake wrote: +This command is meant to operate in tandem with other (non-NBD) +channels to the server. Generally, a "dirty" block is a block +that has been written to

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Eric Blake
On 04/04/2016 12:06 PM, Alex Bligh wrote: > > On 4 Apr 2016, at 17:39, Eric Blake wrote: > >> +This command is meant to operate in tandem with other (non-NBD) >> +channels to the server. Generally, a "dirty" block is a block >> +that has been written to by someone, but the exact mea

Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

2016-04-04 Thread Alex Bligh
On 4 Apr 2016, at 17:39, Eric Blake wrote: > +This command is meant to operate in tandem with other (non-NBD) > +channels to the server. Generally, a "dirty" block is a block > +that has been written to by someone, but the exact meaning of "has > +been written" is left to the im