Re: Re: [PATCH for 8.0 8/8] hmp: add cryptodev info command

[zhenwei pi]

On 11/15/22 02:16, Dr. David Alan Gilbert wrote:
> * zhenwei pi (pizhenwei@bytedance.com) wrote:
> > Example of this command:
> >   # virsh qemu-monitor-command vm --hmp info cryptodev
> > cryptodev1: service=[akcipher|mac|hash|cipher]
> >      queue 0: type=builtin
> > cryptodev0: service=[akcipher]
> >      queue 0: type=lkcf
> >
> > Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> > ---
> >   hmp-commands-info.hx  | 14 ++++++++++++++
> >   include/monitor/hmp.h |  1 +
> >   monitor/hmp-cmds.c    | 36 ++++++++++++++++++++++++++++++++++++
> >   3 files changed, 51 insertions(+)
> >
> > diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> > index 754b1e8408..47d63d26db 100644
> > --- a/hmp-commands-info.hx
> > +++ b/hmp-commands-info.hx
> > @@ -993,3 +993,17 @@ SRST
> >     ``info virtio-queue-element`` *path* *queue* [*index*]
> >       Display element of a given virtio queue
> >   ERST
> > +
> > +    {
> > +        .name       = "cryptodev",
> > +        .args_type  = "",
> > +        .params     = "",
> > +        .help       = "show the crypto devices",
> > +        .cmd        = hmp_info_cryptodev,
> > +        .flags      = "p",
> > +    },
> > +
> > +SRST
> > +  ``info cryptodev``
> > +    Show the crypto devices.
> > +ERST
> > diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
> > index dfbc0c9a2f..b6b2b49202 100644
> > --- a/include/monitor/hmp.h
> > +++ b/include/monitor/hmp.h
> > @@ -143,5 +143,6 @@ void hmp_info_vcpu_dirty_limit(Monitor *mon, const QDict 
> > *qdict);
> >   void hmp_human_readable_text_helper(Monitor *mon,
> >                                       HumanReadableText *(*qmp_handler)(Error 
> > **));
> >   void hmp_info_stats(Monitor *mon, const QDict *qdict);
> > +void hmp_info_cryptodev(Monitor *mon, const QDict *qdict);
> >   
> >   #endif
> > diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> > index 01b789a79e..3f1054aa1e 100644
> > --- a/monitor/hmp-cmds.c
> > +++ b/monitor/hmp-cmds.c
> > @@ -33,6 +33,7 @@
> >   #include "qapi/qapi-commands-block.h"
> >   #include "qapi/qapi-commands-char.h"
> >   #include "qapi/qapi-commands-control.h"
> > +#include "qapi/qapi-commands-cryptodev.h"
> >   #include "qapi/qapi-commands-machine.h"
> >   #include "qapi/qapi-commands-migration.h"
> >   #include "qapi/qapi-commands-misc.h"
> > @@ -2761,3 +2762,38 @@ void hmp_virtio_queue_element(Monitor *mon, const QDict 
> > *qdict)
> >   
> >       qapi_free_VirtioQueueElement(e);
> >   }
> > +
> > +void hmp_info_cryptodev(Monitor *mon, const QDict *qdict)
> > +{
> > +    CryptodevInfoList *info_list;
> > +    CryptodevInfo *info;
> > +    QCryptodevBackendServiceTypeList *service_list;
> > +    CryptodevBackendClientList *client_list;
> > +    CryptodevBackendClient *client;
> > +    char services[128] = {};
>
> I'd rather avoid magic length buffers; the magic is always the wrong
> number!
>
> > +    int len;
> > +
> > +    info_list = qmp_query_cryptodev(NULL);
> > +    for ( ; info_list; info_list = info_list->next) {
>
> maybe:
>   for ( info_list = qmp_query_cryptodev(NULL);
>         info_list;
>         info_list = info_list->next) {
>
> > +        info = info_list->value;
> > +
> > +        service_list = info->service;
> > +        for (len = 0; service_list; service_list = service_list->next) {
> > +            len += snprintf(services + len, sizeof(services) - len, "%s|",
> > +                QCryptodevBackendServiceType_str(service_list->value));
>
> Consider using a dynamically allocated string and then just using
> g_strconcat or g_strjoin() to glue them all together.
>
>      new_services = g_strjoin("|", services,  NULL);   ?
>      g_free(services);
>      services = new_services;
>
> Maybe?
Hi, I'll fix these in the next version, thanks!

> > +        }
> > +        if (len) {
> > +            services[len - 1] = '\0'; /* strip last char '|' */
> > +        }
> > +        monitor_printf(mon, "%s: service=[%s]\n", info->id, services);
> > +
> > +        client_list = info->client;
> > +        for ( ; client_list; client_list = client_list->next) {
> > +            client = client_list->value;
> > +            monitor_printf(mon, "    queue %ld: type=%s\n", client->queue,
> > +                          QCryptodevBackendType_str(client->type));
> > +        }
> > +    }
> > +
> > +    qapi_free_CryptodevInfoList(info_list);
> > +}
> > --
> > 2.20.1

--
zhenwei pi