Re: [PATCH 9/9] hw/cxl: Add QMP status query for dynamic-capacity extent release

[Markus Armbruster <armbru@redhat.com>]

I'm a near-complete CXL ignoramus, and beg your patience.

Alireza Sanaee <alireza.sanaee@huawei.com> writes:

> Add a small status query so orchestration layers can check whether a tagged
> extent is still pending, committed, or no longer present after a release
> request.

Why would they want to check?

What is a "release request"?

>
> Introduce the ExtentStatus QAPI struct and the
> cxl-release-dynamic-capacity-status command.
>
> Signed-off-by: Alireza Sanaee <alireza.sanaee@huawei.com>

[...]

> diff --git a/qapi/cxl.json b/qapi/cxl.json
> index 81d6198ba0..53325ba530 100644
> --- a/qapi/cxl.json
> +++ b/qapi/cxl.json
> @@ -636,3 +636,49 @@
>             },
>    'features': [ 'unstable' ]
>  }
> +
> +##
> +# @ExtentStatus:

Blank line here, please.

> +# This is an object that describes the status of an extent.
> +#
> +# @status:   String indicating the overall result, e.g. "success".

What are the possible values?  Are they defined in some spec?

> +# @message:  Human-readable description of the outcome.

Intended use?

> +#
> +# Since: 9.1

11.1 most likely.

> +##
> +{ 'struct': 'ExtentStatus',
> +      'data': { 'status': 'str', 'message': 'str' }
> +}
> +
> +##
> +# @cxl-release-dynamic-capacity-status:
> +#
> +# This commands checks if an extent tag has been released or not.

Use imperative mood to describe commands: "Check whether an extent tag
has been released."

> +#
> +# @path: path to the CXL Dynamic Capacity Device in the QOM tree.

We commonly call such members @qom-path.

> +#
> +# @host-id: The "Host ID" field as defined in Compute Express Link
> +#     (CXL) Specification, Revision 3.1, Table 7-71.
> +#
> +# @region: The "Region Number" field as defined in Compute Express
> +#     Link Specification, Revision 3.1, Table 7-71.  Valid range
> +#     is from 0-7.
> +#
> +# @tag: The "Tag" field as defined in Compute Express Link (CXL)
> +#     Specification, Revision 3.1, Table 7-71.

I guess the command interrogates something called "extent", and the
something is related to the CXL device at @path in the QOM tree.
Correct?

How is it related?  Is it a part of the device?

I further guess multiple extends can be so related, and @host-id,
@region, and @tag identify select the one we want to interrogate.
Correct?

> +#
> +# Features:
> +#
> +# @unstable: For now this command is subject to change.

The conventional language is "This command is experimental."

> +#
> +# Since: 9.1

11.1 most likely.

> +##
> +{ 'command': 'cxl-release-dynamic-capacity-status',
> +  'data': { 'path': 'str',
> +            'host-id': 'uint16',
> +            'region': 'uint8',
> +            'tag': 'str'
> +          },
> +  'features': [ 'unstable' ],
> +  'returns': 'ExtentStatus'

Swap these two lines, please.

> +}