Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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 > > >> +

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

On 07/04/2016 17:35, Pavel Borzenkov wrote: > > On 05/04/2016 06:05, Kevin Wolf wrote: > > > The options I can think of is adding a request field "max number of > > > descriptors" or a flag "only single descriptor" (with the assumption > > > that clients always want one or unlimited), but maybe

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

On Tue, Apr 05, 2016 at 03:43:08PM +0200, 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 > > descriptors" or a flag "only single descriptor" (with the assumption > > that clients always want one or

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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

Re: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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: [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

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