[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Selecting meta context when calling NBD_CMD_BLOCK_STATUS



26.12.2019 17:29, Nir Soffer wrote:
> Currently client need to use NBD_OPT_SET_META_CONTEXT during handshake to
> select the meta context it wants to get in NBD_CMD_BLOCK_STATUS.
> 
> This means that if you want to provide the option to get both
> "base:allocation" and
> "qemu:dirty-bitmap:x" using the same NBD client connection, you must
> pay for getting
> both during NBD_CMD_BLOCK_STATUS even if the user of the NBD client is not going
> to use both.
> 
> Or, you can use new client connection for getting meta context not
> that the current
> client did not select, which require server that supports multiple
> connections, and new
> handshake. This seems to be more complicated than needed.

Agree, that it's a wrong way

> 
> This complicates existing clients:
> - qemu-img can return either "base:allocation" or
> "qemu:dirty-bitmap:x" using complex
>    undocumentde configuration
> - ovirt-imageio is using dirty flag in the client to enable both
> "base:allocation" and
>    "qemu:dirty-bitmap:x", so we can provide merged extents reporting
> both allocation
>    status and "dirtiness" for every extent.
> 
> Should we extend NBD_CMD_BLOCK_STATUS so we can specify list of meta context
> at the time of the call?
> 
> Looking at command structure, I don't think it could be extended. We
> need to specify
> list of 32 bit context ids, so we need something like:
> 
> 32 bits, length of payload (N * 4)
> 32 bits, contex id 1
> ...
> 32 bits, context id N
> 
> But the length field of a BLOCK_STATUS command is already used to
> specify the range
> of the reply.
> 
> So maybe add a new handshake flag, NBD_OPT_EXTENDED_COMMAND. If set during
> handshake, and NBD_FLAG_EXTENDED is set, a command will support
> additional payload
> like the list of context ids.
> 
> So a BLOCK_STATUS command will look like:
> 
> C: 32 bits, NBD_REQUEST_MAGIC)
> C: 16 bits, NBD_FLAG_EXTENDED
> C: 16 bits, NBD_CMD_BLOCK_STATUS
> C: 64 bits, 42
> C: 64 bits, 0
> C: 32 bits, 1073741824
> C: 32 bits: 4
> C: 32 bit: 1
> 
> With this the server reply will be only for context id 1.
> 
> What do you think?
> 


I'm for.

If we are going to extent the command, I also want a possibility to use 64bit length for
commands with no payload: write_zero, discard, block_status.

I just don't remember what was the problems with extending the command structure,
we already discussed it but somehow it was not simple..

Aha, the problem is that it would be completely incompatible extension: we can't implement
it in a manner that server which don't support it will just reply EINVAL on extended command,
as it will lose the correct position in the command stream..

But I don't think it's the reason to never extend the command. Maybe, the first step should
be implementing cmd_length field feature, so, if it is negotiated, than all commands are
extended by cmd_length field which defines the length of the command in the stream.

Then we'll be able to extend commands as we want..



-- 
Best regards,
Vladimir

Reply to: