[Qemu-devel] NBD BLOCK_STATUS

[Qemu-devel] NBD BLOCK_STATUS

[Vladimir Sementsov-Ogievskiy]

Hi!

Interesting fact: list/set_meta_context options are per-export,
so, in the server we should keep context selection per client per export.

And it is possible for client to set contexts for one export and than 
proceed
to transmission phase with another one.

This is not really touched in spec, for example
"Change the set of active metadata contexts. Issuing this command 
replaces all previously-set metadata contexts"

looks like lacking "for specified export" suffix. May be there are other 
places.

-- 
Best regards,
Vladimir

Re: [Qemu-devel] NBD BLOCK_STATUS

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2133 bytes --]

On 11/09/2017 06:42 AM, Vladimir Sementsov-Ogievskiy wrote:
> Hi!
> 
> Interesting fact: list/set_meta_context options are per-export,
> so, in the server we should keep context selection per client per export.
> 
> And it is possible for client to set contexts for one export and than
> proceed
> to transmission phase with another one.

However, we also documented in the spec that

+    A client MUST NOT send `NBD_CMD_BLOCK_STATUS` unless
+    within the negotiation phase it sent `NBD_OPT_SET_META_CONTEXT`
+    at least once, and the final time it was sent, it referred
+    to the export name that was ultimately selected, the server
+    responded without an error, and returned at least one metadata
+    context.

My take on this is if the client calls:

NBD_OPT_SET_META_CONTEXT (export name "foo")
NBD_OPT_GO (export name "bar")

then the server has no contexts to remember, and the client must not
call NBD_CMD_BLOCK_STATUS.  That is, a server is free to track only a
SINGLE list of context ids (namely, the context ids it replied in the
last response to NBD_OPT_SET_META_CONTEXT), and then toss that list on
any further NBD_OPT_SET_META_CONTEXT  or if NBD_OPT_EXPORT_NAME/GO does
not select the same export name, rather than having to track multiple
separate per-export lists while waiting for the client to pick which
export to finally go with.

> 
> This is not really touched in spec, for example
> "Change the set of active metadata contexts. Issuing this command
> replaces all previously-set metadata contexts"
> 
> looks like lacking "for specified export" suffix. May be there are other
> places.

No, it is intentional that it replaces ALL previous contexts (not just
the per-export context), because of the argument that the only context
that matters as a client is the one you use with NBD_OPT_GO, and if you
choose GO with a different export than your last SET_META_CONTEXT, then
you shouldn't be calling NBD_CMD_BLOCK_STATUS.


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] NBD BLOCK_STATUS

[Vladimir Sementsov-Ogievskiy]

10.11.2017 19:06, Eric Blake wrote:
> On 11/09/2017 06:42 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Hi!
>>
>> Interesting fact: list/set_meta_context options are per-export,
>> so, in the server we should keep context selection per client per export.
>>
>> And it is possible for client to set contexts for one export and than
>> proceed
>> to transmission phase with another one.
> However, we also documented in the spec that
>
> +    A client MUST NOT send `NBD_CMD_BLOCK_STATUS` unless
> +    within the negotiation phase it sent `NBD_OPT_SET_META_CONTEXT`
> +    at least once, and the final time it was sent, it referred
> +    to the export name that was ultimately selected, the server
> +    responded without an error, and returned at least one metadata
> +    context.

I've missed this, then it's OK.

maybe "ultimately selected for transmission phase" would be a bit better.

>
> My take on this is if the client calls:
>
> NBD_OPT_SET_META_CONTEXT (export name "foo")
> NBD_OPT_GO (export name "bar")
>
> then the server has no contexts to remember, and the client must not
> call NBD_CMD_BLOCK_STATUS.  That is, a server is free to track only a
> SINGLE list of context ids (namely, the context ids it replied in the
> last response to NBD_OPT_SET_META_CONTEXT), and then toss that list on
> any further NBD_OPT_SET_META_CONTEXT  or if NBD_OPT_EXPORT_NAME/GO does
> not select the same export name, rather than having to track multiple
> separate per-export lists while waiting for the client to pick which
> export to finally go with.
>
>> This is not really touched in spec, for example
>> "Change the set of active metadata contexts. Issuing this command
>> replaces all previously-set metadata contexts"
>>
>> looks like lacking "for specified export" suffix. May be there are other
>> places.
> No, it is intentional that it replaces ALL previous contexts (not just
> the per-export context), because of the argument that the only context
> that matters as a client is the one you use with NBD_OPT_GO, and if you
> choose GO with a different export than your last SET_META_CONTEXT, then
> you shouldn't be calling NBD_CMD_BLOCK_STATUS.
>
>

Thank you, it's clear for me now.

-- 
Best regards,
Vladimir

Re: [Qemu-devel] NBD BLOCK_STATUS

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1215 bytes --]

On 11/10/2017 10:51 AM, Vladimir Sementsov-Ogievskiy wrote:
> 10.11.2017 19:06, Eric Blake wrote:
>> On 11/09/2017 06:42 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> Hi!
>>>
>>> Interesting fact: list/set_meta_context options are per-export,
>>> so, in the server we should keep context selection per client per
>>> export.
>>>
>>> And it is possible for client to set contexts for one export and than
>>> proceed
>>> to transmission phase with another one.
>> However, we also documented in the spec that
>>
>> +    A client MUST NOT send `NBD_CMD_BLOCK_STATUS` unless
>> +    within the negotiation phase it sent `NBD_OPT_SET_META_CONTEXT`
>> +    at least once, and the final time it was sent, it referred
>> +    to the export name that was ultimately selected, the server
>> +    responded without an error, and returned at least one metadata
>> +    context.
> 
> I've missed this, then it's OK.
> 
> maybe "ultimately selected for transmission phase" would be a bit better.

Sure, I'll queue that up for my next round of doc tweaks.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]