[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extensi
From: |
Wouter Verhelst |
Subject: |
Re: [Qemu-devel] [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension |
Date: |
Thu, 24 Mar 2016 13:43:37 +0100 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
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 respective states.
> >> +
> >> +* `NBD_CMD_GET_LBA_STATUS` (7)
> >> +
> >> + An LBA range status query request. Length and offset define the range
> >> + of interest. The server MUST reply with a reply header, followed
> >> + immediately by the following data:
> >
> > As Eric noted, please expand LBA at least once.
>
> 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?
>
> 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.
>
> 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
>
> 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 the
> >> + 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 flags
> >> + 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 particular
> >> NBD
> >> + server operation MUST NOT make any assumption on the meaning of the
> >> + NBD_STATE_DIRTY or NBD_STATE_CLEAN states.
> >
> > 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.
> >
> > Either the spec should define what it means for a block to be in a dirty
> > state, or it should not talk about it.
>
> Here is my attempt:
>
> 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.
>
> 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.
--
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
people in the world who think they really understand all of its rules,
and pretty much all of them are just lying to themselves too.
-- #debian-devel, OFTC, 2016-02-12
signature.asc
Description: PGP signature
- Re: [Qemu-devel] [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension, (continued)
- Re: [Qemu-devel] [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension, Paolo Bonzini, 2016/03/24
- Re: [Qemu-devel] [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension, Eric Blake, 2016/03/24
- Re: [Qemu-devel] [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension, Paolo Bonzini, 2016/03/24
- Re: [Qemu-devel] [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension, Wouter Verhelst, 2016/03/24
- Re: [Qemu-devel] [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension, Eric Blake, 2016/03/24
- Re: [Qemu-devel] [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension, Kevin Wolf, 2016/03/24
- Re: [Qemu-devel] [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension, Wouter Verhelst, 2016/03/24
- Re: [Qemu-devel] [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension, Kevin Wolf, 2016/03/29
- Re: [Qemu-devel] [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension, Paolo Bonzini, 2016/03/29
- Re: [Qemu-devel] [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension, Wouter Verhelst, 2016/03/29
- Re: [Qemu-devel] [Nbd] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension,
Wouter Verhelst <=
Re: [Qemu-devel] [PATCH 2/2] NBD proto: add GET_LBA_STATUS extension, Eric Blake, 2016/03/24