Re: [PATCH v5 13/13] qapi/cxl.json: Add QMP interfaces to print out accepted and pending DC extents

[Jonathan Cameron]

On Mon,  4 Mar 2024 11:34:08 -0800
nifan.cxl@gmail.com wrote:

> From: Fan Ni <fan.ni@samsung.com>
> 
> With the change, we add the following two QMP interfaces to print out
> extents information in the device,
> 1. cxl-display-accepted-dc-extents: print out the accepted DC extents in
>    the device;
> 2. cxl-display-pending-to-add-dc-extents: print out the pending-to-add
>    DC extents in the device;
> The output is appended to a file passed to the command and by default
> it is /tmp/dc-extent.txt.
Hi Fan,

Is there precedence for this sort of logging to a file from a qmp
command?  I can see something like this being useful.

A few comments inline.

Jonathan


> 
> Signed-off-by: Fan Ni <fan.ni@samsung.com>
> ---
>  hw/mem/cxl_type3.c       | 80 ++++++++++++++++++++++++++++++++++++++++
>  hw/mem/cxl_type3_stubs.c | 12 ++++++
>  qapi/cxl.json            | 32 ++++++++++++++++
>  3 files changed, 124 insertions(+)
> 
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index 5bd64e604e..6a08e7ae40 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -2089,6 +2089,86 @@ void qmp_cxl_release_dynamic_capacity(const char 
> *path, uint8_t region_id,
>                                       region_id, records, errp);
>  }
>  
> +static void cxl_dcd_display_extent_list(const CXLType3Dev *dcd, const char 
> *f,
> +                                        bool accepted_list, Error **errp)
> +{
> +    const CXLDCExtentList *list;
> +    CXLDCExtent *ent;
> +    FILE *fp = NULL;
> +    int i = 0;
> +
> +    if (!dcd->dc.num_regions) {
> +        error_setg(errp, "No dynamic capacity support from the device");
> +        return;
> +    }
> +
> +    if (!f) {
> +        fp = fopen("/tmp/dc-extent.txt", "a+");
> +    } else {
> +        fp = fopen(f, "a+");
> +    }
> +
> +    if (!fp) {
> +        error_setg(errp, "Open log file failed");
> +        return;
> +    }
> +    if (accepted_list) {
> +        list = &dcd->dc.extents;
> +        fprintf(fp, "Print accepted extent info:\n");
> +    } else {
> +        list = &dcd->dc.extents_pending_to_add;
> +        fprintf(fp, "Print pending-to-add extent info:\n");
> +    }
> +
> +    QTAILQ_FOREACH(ent, list, node) {
> +        fprintf(fp, "%d: [0x%lx - 0x%lx]\n", i++, ent->start_dpa,
> +                               ent->start_dpa + ent->len);
> +    }
> +    fprintf(fp, "In total, %d extents printed!\n", i);
> +    fclose(fp);
> +}

> +void qmp_cxl_display_pending_to_add_dc_extents(const char *path, const char 
> *f,
> +                                               Error **errp)
> +{
> +    Object *obj;
> +    CXLType3Dev *dcd;
> +
> +    obj = object_resolve_path(path, NULL);

As an aside, we could probably flatten a lot of these cases into
object_resolve_path_type()

> +    if (!obj) {
> +        error_setg(errp, "Unable to resolve path");
> +        return;
> +    }
> +    if (!object_dynamic_cast(obj, TYPE_CXL_TYPE3)) {
> +        error_setg(errp, "Path not point to a valid CXL type3 device");
> +        return;
> +    }
> +
> +
> +    dcd = CXL_TYPE3(obj);
> +    cxl_dcd_display_extent_list(dcd, f, false, errp);
> +}
> +
>  static void ct3_class_init(ObjectClass *oc, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(oc);

> diff --git a/qapi/cxl.json b/qapi/cxl.json
> index 2645004666..6f10300ec6 100644
> --- a/qapi/cxl.json
> +++ b/qapi/cxl.json
> @@ -420,3 +420,35 @@
>              'extents': [ 'CXLDCExtentRecord' ]
>             }
>  }
> +
> +##
> +# @cxl-display-accepted-dc-extents:
> +#
> +# Command to print out all the accepted DC extents in the device
> +#
> +# @path: CXL DCD canonical QOM path
> +# @output: path of output file to dump the results to

We take a path, but dump to the same file whatever this is set to?
I'm not sure what precedence there is for qom commands that
dump to a debug log.  Perhaps reference any other cases in the
patch description.  

> +#
> +# Since : 9.0
> +##
> +{ 'command': 'cxl-display-accepted-dc-extents',
> +  'data': { 'path': 'str',
> +            'output': 'str'
> +           }
> +}
> +
> +##
> +# @cxl-display-pending-to-add-dc-extents:
> +#
> +# Command to print out all the pending-to-add DC extents in the device
> +#
> +# @path: CXL DCD canonical QOM path
> +# @output: path of output file to dump the results to
> +#
> +# Since : 9.0
> +##
> +{ 'command': 'cxl-display-pending-to-add-dc-extents',
> +  'data': { 'path': 'str',
> +            'output': 'str'
> +           }
> +}