From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43667) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aj7aG-000716-RH for qemu-devel@nongnu.org; Thu, 24 Mar 2016 11:54:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aj7aA-00046V-Et for qemu-devel@nongnu.org; Thu, 24 Mar 2016 11:54:04 -0400 Received: from barbershop.grep.be ([89.106.240.122]:56523) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aj7aA-00046H-63 for qemu-devel@nongnu.org; Thu, 24 Mar 2016 11:53:58 -0400 Date: Thu, 24 Mar 2016 13:43:37 +0100 From: Wouter Verhelst Message-ID: <20160324124337.GH2870@grep.be> References: <1458742562-30624-1-git-send-email-den@openvz.org> <1458742562-30624-3-git-send-email-den@openvz.org> <20160323175834.GC2467@grep.be> <56F3D5C7.9070007@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="dFWYt1i2NyOo1oI9" Content-Disposition: inline In-Reply-To: <56F3D5C7.9070007@redhat.com> Subject: Re: [Qemu-devel] [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: nbd-general@lists.sourceforge.net, "Denis V. Lunev" , Stefan Hajnoczi , qemu-devel@nongnu.org, Kevin Wolf --dFWYt1i2NyOo1oI9 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi Paolo, On Thu, Mar 24, 2016 at 12:55:51PM +0100, Paolo Bonzini wrote: >=20 >=20 > 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 w= ith > >> +their respective states. > >> + > >> +* `NBD_CMD_GET_LBA_STATUS` (7) > >> + > >> + An LBA range status query request. Length and offset define the r= ange > >> + of interest. The server MUST reply with a reply header, followed > >> + immediately by the following data: > >=20 > > As Eric noted, please expand LBA at least once. >=20 > Let's just use "block" (e.g. NBD_CMD_GET_BLOCK_STATUS). That works too :-) [...] > > Also, this may end up being a fairly expensive call for the server to > > process. Is it really useful? >=20 > It's always okay for the server to omit NBD_STATE_ZERO, but it can be > useful if the state is known for some reason. For example, file system > holes are always zero, but unallocated blocks on a block device are not > always zero. >=20 > However, let's make these bits, so that >=20 > NBD_STATE_ALLOCATED (0x1), LBA extent is present on the block device > NBD_STATE_ZERO (0x2), LBA extent will read as zeroes >=20 > and you can have NBD_STATE_ALLOCATED|NBD_STATE_ZERO. File systems do > have the concept of unwritten extents which would be represented like > that. The API to access the information (the FIEMAP ioctl) is broken, > but perhaps in the future a non-broken API could be added---for example > SEEK_ZERO and SEEK_NONZERO values for lseek's "whence" argument. Okay, that works for me. > >> + - `NBD_STATE_DEALLOCATED` (0x2), LBA extent is not present on t= he > >> + block device. A client MUST NOT make any assumptions about the > >> + contents of the extent. > >> + > >> + 2. Block dirtiness state > >> + > >> + Upon receiving an `NBD_CMD_GET_LBA_STATUS` command with command f= lags > >> + field set to `NBD_FLAG_GET_DIRTY` (0x1), the server MUST return > >> + the dirtiness status of the device. The following dirtiness states > >> + are defined for the command: > >> + > >> + - `NBD_STATE_DIRTY` (0x0), LBA extent is dirty; > >> + - `NBD_STATE_CLEAN` (0x1), LBA extent is clean. > >> + > >> + Generic NBD client implementation without knowledge of a particul= ar NBD > >> + server operation MUST NOT make any assumption on the meaning of t= he > >> + NBD_STATE_DIRTY or NBD_STATE_CLEAN states. > >=20 > > That makes it a useless call. A server can read /dev/random to decide > > whether to send STATE_DIRTY or STATE_CLEAN, and still be compliant with > > this spec. > >=20 > > Either the spec should define what it means for a block to be in a dirty > > state, or it should not talk about it. >=20 > Here is my attempt: >=20 > 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 implementation. For example, a virtual > machine monitor could provide a (non-NBD) command to start tracking > blocks written by the virtual machine. A backup client then can > connect to an NBD server provided by the virtual machine monitor > and use NBD_CMD_GET_BLOCK_STATUS only read blocks that the virtual ^ to > machine has changed. >=20 > An implementation that doesn't track the "dirtiness" state of blocks > MUST either fail this command with EINVAL, or mark all blocks as > dirty in the descriptor that it returns. That seems saner, yes -- and I'm starting to understand what the rationale is for this protocol message :-) I suppose I could also implement that in nbd-server to send out information about changed blocks if the copy-on-write option has been switched on. It might also be possible to add an in-protocol message to start tracking changes (e.g., a "create checkpoint" message), but I'm not sure if that's worth it (and it could massively complicate the NBD state machine and protocol; not sure whether that's worth it) At any rate, your suggestion does alleviate my concerns. --=20 < ron> I mean, the main *practical* problem with C++, is there's like a doz= en people in the world who think they really understand all of its rule= s, and pretty much all of them are just lying to themselves too. -- #debian-devel, OFTC, 2016-02-12 --dFWYt1i2NyOo1oI9 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBCAAGBQJW8+D5AAoJEMKUD5Ub3wqdFz4QAMmubAo7Ll2FZvhx+Fcii61l ypeOAEnyNTQ6dmllL+3IVA3fEzYsJUTBTzfP7F7cOJgS1lPdqQdyzE0g+rK8hViW 8bmw7EQzZ2968nkjV151z3+jCYPJgI2Soo7UVP/P0rvByACzbiVy0Gx8phEr2qiD w5R6f9BGnLE7X1F9B0rM3bYpSCv7LD/FxZpxS8IbCsMsfp7WM0zy726yUi4mdASc YdXityaR0iXMYPCn7RUeuVUMC2t/VkvnYaF6gE4AwQatiurURWZ0kF0cjplTM8mJ 4b8zezr6HwTZiBIuJHXOO7sluIi2e1SC6xKRB6sdiqZHAKSZ/FlmZ4o7Yxtszgry cUSfUpm3WzJmQ0jOWg4+GjlmoSM/Tvbdwa+09I4RNSqB0yigTI/roZYF1mZDb78H /bYwHKCLk8Yg3YgekhLLw31NzYtbheGUKKy3ymWpM1+g1qcm4yukU2ejBYizkeFW cQO90RVvQidSZ9D60bpo8Cy9GJZtsgkAUOvn9qZps8F5tsXlwMExf2f2RdDRi16T uv+VVxXQp2sy247j6xJ6XOgiZLreVSZwRjacaoKcnJIyen17NGJsv+vPXIons17P pPW6mpfyh/lZSBIs5I3SYy96uuSrh0oxXU//xdRX8nPuUaSm4D75iA2uNzfj0MnQ V+24g4b+IoA/5AbJwqu4 =caur -----END PGP SIGNATURE----- --dFWYt1i2NyOo1oI9--