hmp: Update info vnc
PATCH 1/1 hmp: Update info vnc
From: "Dr. David Alan Gilbert" <[email protected]>
The QMP query-vnc interfaces have gained a lot more information that the HMP interfaces hasn't got yet. Update it.
Note the output format has changed, but this is HMP so that's OK.
In particular, this now includes client information for reverse connections:
-vnc :0
(qemu) info vnc
default:
Server: 0.0.0.0:5900 (ipv4)
Auth: none (Sub: none)
Auth: none (Sub: none)
(Now connect a client)
(qemu) info vnc
default:
Server: 0.0.0.0:5900 (ipv4)
Auth: none (Sub: none)
Client: 127.0.0.1:51828 (ipv4)
x509_dname: none
username: noneAuth: none (Sub: none)
-vnc localhost:7000,reverse
(qemu) info vnc
default:
Client: ::1:7000 (ipv6)
x509_dname: none
username: noneAuth: none (Sub: none)
-vnc :1,password,id=rev -vnc localhost:7000,reverse (qemu) info vnc
default:
Client: ::1:7000 (ipv6)
x509_dname: none
username: none Auth: none (Sub: none)
rev:
Server: 0.0.0.0:5901 (ipv4)
Auth: vnc (Sub: none)
Client: 127.0.0.1:53616 (ipv4)
x509_dname: none
username: noneAuth: vnc (Sub: none)
This was originally RH bz 1461682
--- hmp.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 67 insertions(+), 31 deletions(-)
hmp.c +67 -31
@@ -600,50 +600,86 @@ void hmp_info_blockstats(Monitor *mon, const QDict *qdict)qapi_free_BlockStatsList (stats_list); } +/* Helper for hmp_info_vnc_clients, _servers */ +static void hmp_info_VncBasicInfo (Monitor *mon,VncBasicInfo *info, +const char *name) +{ +monitor_printf (mon," %s: %s:%s (%s%s)\n" , + name, + info->host, + info->service, + NetworkAddressFamily_lookup[info->family], + info->websocket ?" (Websocket)" :"" ); +} + +/* Helper displaying and euth and crypt info */ +static void hmp_info_vnc_authcrypt (Monitor *mon,const char *indent, +VncPrimaryAuth auth, +VncVencryptSubAuth *vencrypt) +{ +monitor_printf (mon,"%sAuth: %s (Sub: %s)\n" , indent, + VncPrimaryAuth_lookup[auth], + vencrypt ? VncVencryptSubAuth_lookup[*vencrypt] :"none" ); +} + +static void hmp_info_vnc_clients (Monitor *mon,VncClientInfoList *client) +{ +while (client) { +VncClientInfo *cinfo = client->value; + +hmp_info_VncBasicInfo (mon, (VncBasicInfo *)cinfo,"Client" ); +monitor_printf (mon," x509_dname: %s\n" , + cinfo->x509_dname ? + cinfo->x509_dname :"none" ); +monitor_printf (mon," username: %s\n" , + cinfo->has_sasl_username ? + cinfo->sasl_username :"none" ); + + client = client->next; + } +} + +static void hmp_info_vnc_servers (Monitor *mon,VncServerInfo2List *server) +{ +while (server) { +VncServerInfo2 *sinfo = server->value; +hmp_info_VncBasicInfo (mon, (VncBasicInfo *)sinfo,"Server" ); +hmp_info_vnc_authcrypt (mon," " , sinfo->auth, + sinfo->has_vencrypt ? &sinfo->vencrypt :NULL ); + server = server->next; + } +} +void hmp_info_vnc (Monitor *mon,const QDict *qdict) { -VncInfo *info; +VncInfo2List *info2l;Error *err =NULL ; -VncClientInfoList *client; - info =qmp_query_vnc (&err); + info2l =qmp_query_vnc_servers (&err);if (err) {error_report_err (err);return ; } - -if (!info->enabled) { -monitor_printf (mon,"Server: disabled\n" ); - goto out; - } - -monitor_printf (mon,"Server:\n" ); -if (info->has_host && info->has_service) { -monitor_printf (mon," address: %s:%s\n" , info->host, info->service); - } -if (info->has_auth) { -monitor_printf (mon," auth: %s\n" , info->auth); +if (!info2l) { +monitor_printf (mon,"None\n" ); +return ; } -if (!info->has_clients || info->clients ==NULL ) { -monitor_printf (mon,"Client: none\n" ); - }else { -for (client = info->clients; client; client = client->next) { -monitor_printf (mon,"Client:\n" ); -monitor_printf (mon," address: %s:%s\n" , - client->value->host, - client->value->service); -monitor_printf (mon," x509_dname: %s\n" , - client->value->x509_dname ? - client->value->x509_dname :"none" ); -monitor_printf (mon," username: %s\n" , - client->value->has_sasl_username ? - client->value->sasl_username :"none" ); +while (info2l) { +VncInfo2 *info = info2l->value; +monitor_printf (mon,"%s:\n" , info->id); +hmp_info_vnc_servers (mon, info->server); +hmp_info_vnc_clients (mon, info->clients); +hmp_info_vnc_authcrypt (mon," " , info->auth, + info->has_vencrypt ? &info->vencrypt :NULL ); +if (info->has_display) { +monitor_printf (mon," Display: %s\n" , info->display); } + info2l = info2l->next; } -out: -qapi_free_VncInfo (info); +qapi_free_VncInfo2List (info2l); + }#ifdef CONFIG_SPICE
Signature · 1 line
2.13.0
----- Original Message -----
Quoted 89 lines
From: "Dr. David Alan Gilbert" <[email protected]>
The QMP query-vnc interfaces have gained a lot more information that the HMP interfaces hasn't got yet. Update it.
Note the output format has changed, but this is HMP so that's OK.
In particular, this now includes client information for reverse connections:
-vnc :0
(qemu) info vnc
default:
Server: 0.0.0.0:5900 (ipv4)
Auth: none (Sub: none)
Auth: none (Sub: none)(Now connect a client)
(qemu) info vnc
default:
Server: 0.0.0.0:5900 (ipv4)
Auth: none (Sub: none)
Client: 127.0.0.1:51828 (ipv4)x509_dname: none username: noneAuth: none (Sub: none)
-vnc localhost:7000,reverse
(qemu) info vnc
default:
Client: ::1:7000 (ipv6)x509_dname: none username: noneAuth: none (Sub: none)
-vnc :1,password,id=rev -vnc localhost:7000,reverse (qemu) info vnc
default:
Client: ::1:7000 (ipv6)x509_dname: none username: noneAuth: none (Sub: none)
rev:
Server: 0.0.0.0:5901 (ipv4)
Auth: vnc (Sub: none)
Client: 127.0.0.1:53616 (ipv4)x509_dname: none username: noneAuth: vnc (Sub: none)
This was originally RH bz 1461682
Signed-off-by Dr. David Alan Gilbert <[email protected]>--- hmp.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 67 insertions(+), 31 deletions(-)hmp.c +20
-0@@ -600,50 +600,86 @@ void hmp_info_blockstats(Monitor *mon, const QDict *qdict)qapi_free_BlockStatsList (stats_list); } +/* Helper for hmp_info_vnc_clients, _servers */ +static void hmp_info_VncBasicInfo (Monitor *mon,VncBasicInfo *info, +const char *name) +{ +monitor_printf (mon," %s: %s:%s (%s%s)\n" , + name, + info->host, + info->service, + NetworkAddressFamily_lookup[info->family], + info->websocket ?" (Websocket)" :"" ); +} + +/* Helper displaying and euth and crypt info */ +static void hmp_info_vnc_authcrypt (Monitor *mon,const char *indent, +VncPrimaryAuth auth, +VncVencryptSubAuth *vencrypt) +{ +monitor_printf (mon,"%sAuth: %s (Sub: %s)\n" , indent, + VncPrimaryAuth_lookup[auth], + vencrypt ? VncVencryptSubAuth_lookup[*vencrypt] :"none" );
hmp.c void hmp_info_blockstats(Monitor *mon, const QDict · line 634
Quoted 10 lines
hmp.c +10
-0+} + +static void hmp_info_vnc_clients (Monitor *mon,VncClientInfoList *client) +{ +while (client) { +VncClientInfo *cinfo = client->value; + +hmp_info_VncBasicInfo (mon, (VncBasicInfo *)cinfo,"Client" ); + monitor_printf(mon, " x509_dname: %s\n", + cinfo->x509_dname ?
Why not use has_x509_dname ?
hmp.c void hmp_info_blockstats(Monitor *mon, const QDict *qdict) · line 634
Quoted 2 lines
hmp.c +2
-0+ cinfo->x509_dname : "none"); + monitor_printf(mon, " username: %s\n",
sasl_username would be more clear
Quoted 85 lines
hmp.c +35
-32+ cinfo->has_sasl_username ? + cinfo->sasl_username : "none"); + + client = client->next; + } +} + +static void hmp_info_vnc_servers (Monitor *mon,VncServerInfo2List *server) +{ +while (server) { +VncServerInfo2 *sinfo = server->value; +hmp_info_VncBasicInfo (mon, (VncBasicInfo *)sinfo,"Server" ); +hmp_info_vnc_authcrypt (mon," " , sinfo->auth, + sinfo->has_vencrypt ? &sinfo->vencrypt :NULL ); + server = server->next; + } +} +void hmp_info_vnc (Monitor *mon,const QDict *qdict) { -VncInfo *info; +VncInfo2List *info2l;Error *err =NULL ; -VncClientInfoList *client; - info =qmp_query_vnc (&err); + info2l =qmp_query_vnc_servers (&err);if (err) {error_report_err (err);return ; } - -if (!info->enabled) { -monitor_printf (mon,"Server: disabled\n" ); - goto out; - } - -monitor_printf (mon,"Server:\n" ); -if (info->has_host && info->has_service) { -monitor_printf (mon," address: %s:%s\n" , info->host, info->service); - } -if (info->has_auth) { -monitor_printf (mon," auth: %s\n" , info->auth); +if (!info2l) { +monitor_printf (mon,"None\n" ); +return ; } -if (!info->has_clients || info->clients ==NULL ) { -monitor_printf (mon,"Client: none\n" ); - }else { -for (client = info->clients; client; client = client->next) { -monitor_printf (mon,"Client:\n" ); -monitor_printf (mon," address: %s:%s\n" , - client->value->host, - client->value->service); -monitor_printf (mon," x509_dname: %s\n" , - client->value->x509_dname ? - client->value->x509_dname :"none" ); -monitor_printf (mon," username: %s\n" , - client->value->has_sasl_username ? - client->value->sasl_username :"none" ); +while (info2l) { +VncInfo2 *info = info2l->value; +monitor_printf (mon,"%s:\n" , info->id); +hmp_info_vnc_servers (mon, info->server); +hmp_info_vnc_clients (mon, info->clients); +hmp_info_vnc_authcrypt (mon," " , info->auth, + info->has_vencrypt ? &info->vencrypt :NULL ); +if (info->has_display) { +monitor_printf (mon," Display: %s\n" , info->display); } + info2l = info2l->next; } -out: -qapi_free_VncInfo (info); +qapi_free_VncInfo2List (info2l); + }#ifdef CONFIG_SPICE --
Quoted 1 line
2.13.0
Looks good to me otherwise
Quoted 96 lines · * Marc-André Lureau ([email protected]) wrote:
----- Original Message -----
Quoted 93 lines
From: "Dr. David Alan Gilbert" <[email protected]>
The QMP query-vnc interfaces have gained a lot more information that the HMP interfaces hasn't got yet. Update it.
Note the output format has changed, but this is HMP so that's OK.
In particular, this now includes client information for reverse connections:
-vnc :0
(qemu) info vnc
default:
Server: 0.0.0.0:5900 (ipv4)
Auth: none (Sub: none)
Auth: none (Sub: none)(Now connect a client)
(qemu) info vnc
default:
Server: 0.0.0.0:5900 (ipv4)
Auth: none (Sub: none)
Client: 127.0.0.1:51828 (ipv4)x509_dname: none username: noneAuth: none (Sub: none)
-vnc localhost:7000,reverse
(qemu) info vnc
default:
Client: ::1:7000 (ipv6)x509_dname: none username: noneAuth: none (Sub: none)
-vnc :1,password,id=rev -vnc localhost:7000,reverse (qemu) info vnc
default:
Client: ::1:7000 (ipv6)x509_dname: none username: noneAuth: none (Sub: none)
rev:
Server: 0.0.0.0:5901 (ipv4)
Auth: vnc (Sub: none)
Client: 127.0.0.1:53616 (ipv4)x509_dname: none username: noneAuth: vnc (Sub: none)
This was originally RH bz 1461682
Signed-off-by Dr. David Alan Gilbert <[email protected]>--- hmp.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 67 insertions(+), 31 deletions(-)hmp.c +24
-0@@ -600,50 +600,86 @@ void hmp_info_blockstats(Monitor *mon, const QDict *qdict)qapi_free_BlockStatsList (stats_list); } +/* Helper for hmp_info_vnc_clients, _servers */ +static void hmp_info_VncBasicInfo (Monitor *mon,VncBasicInfo *info, +const char *name) +{ +monitor_printf (mon," %s: %s:%s (%s%s)\n" , + name, + info->host, + info->service, + NetworkAddressFamily_lookup[info->family], + info->websocket ?" (Websocket)" :"" ); +} + +/* Helper displaying and euth and crypt info */ +static void hmp_info_vnc_authcrypt (Monitor *mon,const char *indent, +VncPrimaryAuth auth, +VncVencryptSubAuth *vencrypt) +{ +monitor_printf (mon,"%sAuth: %s (Sub: %s)\n" , indent, + VncPrimaryAuth_lookup[auth], + vencrypt ? VncVencryptSubAuth_lookup[*vencrypt] :"none" ); +} + +static void hmp_info_vnc_clients (Monitor *mon,VncClientInfoList *client) +{
Quoted 8 lines
hmp.c void hmp_info_blockstats(Monitor *mon, const QDict *qdict) · line 632
Quoted 6 lines
hmp.c +6
-0+while (client) { +VncClientInfo *cinfo = client->value; + +hmp_info_VncBasicInfo (mon, (VncBasicInfo *)cinfo,"Client" ); + monitor_printf(mon, " x509_dname: %s\n", + cinfo->x509_dname ?Why not use
has_x509_dname?
Interesting; that line I just moved from the old code; fixed.
Quoted 4 lines
hmp.c void hmp_info_blockstats(Monitor *mon, const QDict *qdict) · line 634
Quoted 2 lines
hmp.c +2
-0+ cinfo->x509_dname : "none"); + monitor_printf(mon, " username: %s\n",
sasl_usernamewould be more clear
Also from the old code; also fixed.
Thanks,
Dave
Quoted 83 lines
Quoted 83 lines
hmp.c +35
-31+ cinfo->has_sasl_username ? + cinfo->sasl_username : "none"); + + client = client->next; + } +} + +static void hmp_info_vnc_servers (Monitor *mon,VncServerInfo2List *server) +{ +while (server) { +VncServerInfo2 *sinfo = server->value; +hmp_info_VncBasicInfo (mon, (VncBasicInfo *)sinfo,"Server" ); +hmp_info_vnc_authcrypt (mon," " , sinfo->auth, + sinfo->has_vencrypt ? &sinfo->vencrypt :NULL ); + server = server->next; + } +} +void hmp_info_vnc (Monitor *mon,const QDict *qdict) { -VncInfo *info; +VncInfo2List *info2l;Error *err =NULL ; -VncClientInfoList *client; - info =qmp_query_vnc (&err); + info2l =qmp_query_vnc_servers (&err);if (err) {error_report_err (err);return ; } - -if (!info->enabled) { -monitor_printf (mon,"Server: disabled\n" ); - goto out; - } - -monitor_printf (mon,"Server:\n" ); -if (info->has_host && info->has_service) { -monitor_printf (mon," address: %s:%s\n" , info->host, info->service); - } -if (info->has_auth) { -monitor_printf (mon," auth: %s\n" , info->auth); +if (!info2l) { +monitor_printf (mon,"None\n" ); +return ; } -if (!info->has_clients || info->clients ==NULL ) { -monitor_printf (mon,"Client: none\n" ); - }else { -for (client = info->clients; client; client = client->next) { -monitor_printf (mon,"Client:\n" ); -monitor_printf (mon," address: %s:%s\n" , - client->value->host, - client->value->service); -monitor_printf (mon," x509_dname: %s\n" , - client->value->x509_dname ? - client->value->x509_dname :"none" ); -monitor_printf (mon," username: %s\n" , - client->value->has_sasl_username ? - client->value->sasl_username :"none" ); +while (info2l) { +VncInfo2 *info = info2l->value; +monitor_printf (mon,"%s:\n" , info->id); +hmp_info_vnc_servers (mon, info->server); +hmp_info_vnc_clients (mon, info->clients); +hmp_info_vnc_authcrypt (mon," " , info->auth, + info->has_vencrypt ? &info->vencrypt :NULL ); +if (info->has_display) { +monitor_printf (mon," Display: %s\n" , info->display); } + info2l = info2l->next; } -out: -qapi_free_VncInfo (info); +qapi_free_VncInfo2List (info2l); + }
Quoted 6 lines
Quoted 3 lines
#ifdef
CONFIG_SPICE
--
2.13.0Looks good to me otherwise
Reviewed-by Marc-André Lureau <[email protected]>
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Quoted 10 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
The QMP query-vnc interfaces have gained a lot more information that the HMP interfaces hasn't got yet. Update it.
Note the output format has changed, but this is HMP so that's OK.
In particular, this now includes client information for reverse connections:
Quoted 6 lines
-vnc :0
(qemu) info vnc
default:
Server: 0.0.0.0:5900 (ipv4)
Auth: none (Sub: none)
Auth: none (Sub: none)
If you're reporting Auth: against the individual Server entry, there's no reason to report it at the top level too. We only keep that duplication in QMP for sake of backcompat, which is not important for HMP
Quoted 31 lines
(Now connect a client)
(qemu) info vnc
default:
Server: 0.0.0.0:5900 (ipv4)
Auth: none (Sub: none)
Client: 127.0.0.1:51828 (ipv4)x509_dname: none username: noneAuth: none (Sub: none)
-vnc localhost:7000,reverse
(qemu) info vnc
default:
Client: ::1:7000 (ipv6)x509_dname: none username: noneAuth: none (Sub: none)
-vnc :1,password,id=rev -vnc localhost:7000,reverse (qemu) info vnc
default:
Client: ::1:7000 (ipv6)x509_dname: none username: noneAuth: none (Sub: none)
rev:
Server: 0.0.0.0:5901 (ipv4)
Auth: vnc (Sub: none)
Client: 127.0.0.1:53616 (ipv4)
Quoted 7 lines
x509_dname: none username: noneAuth: vnc (Sub: none)
This was originally RH bz 1461682
Signed-off-by Dr. David Alan Gilbert <[email protected]>
Regards,
Daniel
Signature · 3 lines
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Quoted 18 lines · * Daniel P. Berrange ([email protected]) wrote:
Quoted 10 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
The QMP query-vnc interfaces have gained a lot more information that the HMP interfaces hasn't got yet. Update it.
Note the output format has changed, but this is HMP so that's OK.
In particular, this now includes client information for reverse connections:
Quoted 6 lines
-vnc :0
(qemu) info vnc
default:
Server: 0.0.0.0:5900 (ipv4)
Auth: none (Sub: none)
Auth: none (Sub: none)
Quoted 4 lines
If you're reporting Auth: against the individual Server entry, there's no reason to report it at the top level too. We only keep that duplication in QMP for sake of backcompat, which is not important for HMP
Where would it get reported for a 'reverse' connection then? That has no server entry.
Dave
Quoted 35 lines
Quoted 35 lines
(Now connect a client)
(qemu) info vnc
default:
Server: 0.0.0.0:5900 (ipv4)
Auth: none (Sub: none)
Client: 127.0.0.1:51828 (ipv4)x509_dname: none username: noneAuth: none (Sub: none)
-vnc localhost:7000,reverse
(qemu) info vnc
default:
Client: ::1:7000 (ipv6)x509_dname: none username: noneAuth: none (Sub: none)
-vnc :1,password,id=rev -vnc localhost:7000,reverse (qemu) info vnc
default:
Client: ::1:7000 (ipv6)x509_dname: none username: noneAuth: none (Sub: none)
rev:
Server: 0.0.0.0:5901 (ipv4)
Auth: vnc (Sub: none)
Client: 127.0.0.1:53616 (ipv4)x509_dname: none username: noneAuth: vnc (Sub: none)
Quoted 10 lines
Quoted 3 lines
This was originally RH bz 1461682
Signed-off-by Dr. David Alan Gilbert <[email protected]>Regards,
DanielSignature · 3 lines
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Quoted 19 lines · Dr. David Alan Gilbert
Quoted 18 lines · * Daniel P. Berrange ([email protected]) wrote:
Quoted 16 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
The QMP query-vnc interfaces have gained a lot more information that the HMP interfaces hasn't got yet. Update it.
Note the output format has changed, but this is HMP so that's OK.
In particular, this now includes client information for reverse connections:
-vnc :0
(qemu) info vnc
default:
Server: 0.0.0.0:5900 (ipv4)
Auth: none (Sub: none)
Auth: none (Sub: none)
Quoted 7 lines
Quoted 4 lines
If you're reporting Auth: against the individual Server entry, there's no reason to report it at the top level too. We only keep that duplication in QMP for sake of backcompat, which is not important for HMP
Where would it get reported for a 'reverse' connection then? That has no server entry.
Ah right. So lets only report it at the top level, if operating in reverse mode.
Regards,
Daniel
Signature · 3 lines
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Quoted 25 lines · * Daniel P. Berrange ([email protected]) wrote:
Quoted 24 lines · Dr. David Alan Gilbert
Quoted 22 lines · * Daniel P. Berrange ([email protected]) wrote:
Quoted 10 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
The QMP query-vnc interfaces have gained a lot more information that the HMP interfaces hasn't got yet. Update it.
Note the output format has changed, but this is HMP so that's OK.
In particular, this now includes client information for reverse connections:
Quoted 6 lines
-vnc :0
(qemu) info vnc
default:
Server: 0.0.0.0:5900 (ipv4)
Auth: none (Sub: none)
Auth: none (Sub: none)If you're reporting Auth: against the individual Server entry, there's no reason to report it at the top level too. We only keep that duplication in QMP for sake of backcompat, which is not important for HMP
Quoted 5 lines
Quoted 2 lines
Where would it get reported for a 'reverse' connection then? That has no server entry.
Ah right. So lets only report it at the top level, if operating in reverse mode.
OK, new version coming up.
Dave
Quoted 7 lines
Regards,
DanielSignature · 3 lines
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Quoted 63 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
The QMP query-vnc interfaces have gained a lot more information that the HMP interfaces hasn't got yet. Update it.
Note the output format has changed, but this is HMP so that's OK.
In particular, this now includes client information for reverse connections:
-vnc :0
(qemu) info vnc
default:
Server: 0.0.0.0:5900 (ipv4)
Auth: none (Sub: none)
Auth: none (Sub: none)(Now connect a client)
(qemu) info vnc
default:
Server: 0.0.0.0:5900 (ipv4)
Auth: none (Sub: none)
Client: 127.0.0.1:51828 (ipv4)x509_dname: none username: noneAuth: none (Sub: none)
-vnc localhost:7000,reverse
(qemu) info vnc
default:
Client: ::1:7000 (ipv6)x509_dname: none username: noneAuth: none (Sub: none)
-vnc :1,password,id=rev -vnc localhost:7000,reverse (qemu) info vnc
default:
Client: ::1:7000 (ipv6)x509_dname: none username: noneAuth: none (Sub: none)
rev:
Server: 0.0.0.0:5901 (ipv4)
Auth: vnc (Sub: none)
Client: 127.0.0.1:53616 (ipv4)x509_dname: none username: noneAuth: vnc (Sub: none)
This was originally RH bz 1461682
Signed-off-by Dr. David Alan Gilbert <[email protected]>--- hmp.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 67 insertions(+), 31 deletions(-)hmp.c +0
-0
hmp.c void hmp_info_blockstats(Monitor *mon, const QDict *qdict) · line 603
Quoted 4 lines
hmp.c +1
-0@@ -600,50 +600,86 @@ void hmp_info_blockstats(Monitor *mon, const QDict *qdict)qapi_free_BlockStatsList (stats_list); } +/* Helper for hmp_info_vnc_clients, _servers */
Not sure this comment is worth its keep.
hmp.c void hmp_info_blockstats(Monitor *mon, const QDict *qdict) · line 615
Quoted 12 lines
hmp.c +12
-0+static void hmp_info_VncBasicInfo (Monitor *mon,VncBasicInfo *info, +const char *name) +{ +monitor_printf (mon," %s: %s:%s (%s%s)\n" , + name, + info->host, + info->service, + NetworkAddressFamily_lookup[info->family], + info->websocket ?" (Websocket)" :"" ); +} + +/* Helper displaying and euth and crypt info */
"euth"? Do you mean "auth"?
Not sure this comment is worth its keep.
Quoted 5 lines
hmp.c +5
-0+static void hmp_info_vnc_authcrypt (Monitor *mon,const char *indent, +VncPrimaryAuth auth, +VncVencryptSubAuth *vencrypt) +{ + monitor_printf(mon, "%sAuth: %s (Sub: %s)\n", indent,
hmp.c void hmp_info_blockstats(Monitor *mon, const QDict *qdict) · line 630
Quoted 10 lines
hmp.c +10
-0+ VncPrimaryAuth_lookup[auth], + vencrypt ? VncVencryptSubAuth_lookup[*vencrypt] : "none"); +} + +static void hmp_info_vnc_clients (Monitor *mon,VncClientInfoList *client) +{ +while (client) { +VncClientInfo *cinfo = client->value; + +hmp_info_VncBasicInfo (mon, (VncBasicInfo *)cinfo,"Client" );
QAPI provides a type-safe conversion from type to base type:qapi_TYPE_base(). You could write qapi_VncClientInfo_base(cinfo) here. More conversions to base type below.
hmp.c void hmp_info_blockstats(Monitor *mon, const QDict *qdict) · line 636
Quoted 6 lines
hmp.c +6
-0+monitor_printf (mon," x509_dname: %s\n" , + cinfo->x509_dname ? + cinfo->x509_dname :"none" ); +monitor_printf (mon," username: %s\n" , + cinfo->has_sasl_username ? + cinfo->sasl_username :"none" );
When an optional QAPI member FOO is a pointer, checking FOO instead of has_FOO is totally fine. But mixing the two in one breath looks odd.
I'd like to get rid of the redundant has_FOOs some day.
hmp.c void hmp_info_blockstats(Monitor *mon, const QDict *qdict) · line 639
Quoted 3 lines
hmp.c +3
-0+ + client = client->next; + }
I prefer to keep loop control in one place, and I also prefer not to change parameters:
for (cl = client; cl; cl = cl->next) {Matter of taste; your artistic license applies. More of the same below.
Quoted 58 lines
hmp.c +19
-29+} + +static void hmp_info_vnc_servers (Monitor *mon,VncServerInfo2List *server) +{ +while (server) { +VncServerInfo2 *sinfo = server->value; +hmp_info_VncBasicInfo (mon, (VncBasicInfo *)sinfo,"Server" ); +hmp_info_vnc_authcrypt (mon," " , sinfo->auth, + sinfo->has_vencrypt ? &sinfo->vencrypt :NULL ); + server = server->next; + } +} +void hmp_info_vnc (Monitor *mon,const QDict *qdict) { -VncInfo *info; +VncInfo2List *info2l;Error *err =NULL ; -VncClientInfoList *client; - info =qmp_query_vnc (&err); + info2l =qmp_query_vnc_servers (&err);if (err) {error_report_err (err);return ; } - -if (!info->enabled) { -monitor_printf (mon,"Server: disabled\n" ); - goto out; - } - -monitor_printf (mon,"Server:\n" ); -if (info->has_host && info->has_service) { -monitor_printf (mon," address: %s:%s\n" , info->host, info->service); - } -if (info->has_auth) { -monitor_printf (mon," auth: %s\n" , info->auth); +if (!info2l) { +monitor_printf (mon,"None\n" ); +return ; } -if (!info->has_clients || info->clients ==NULL ) { -monitor_printf (mon,"Client: none\n" ); - }else { -for (client = info->clients; client; client = client->next) { -monitor_printf (mon,"Client:\n" ); -monitor_printf (mon," address: %s:%s\n" , - client->value->host, - client->value->service); -monitor_printf (mon," x509_dname: %s\n" , - client->value->x509_dname ? - client->value->x509_dname :"none" ); -monitor_printf (mon," username: %s\n" , - client->value->has_sasl_username ? - client->value->sasl_username :"none" ); +while (info2l) {
hmp.c void hmp_info_blockstats(Monitor *mon, const QDict *qdict) · line 678
Quoted 10 lines
hmp.c +9
-0+VncInfo2 *info = info2l->value; +monitor_printf (mon,"%s:\n" , info->id); +hmp_info_vnc_servers (mon, info->server); +hmp_info_vnc_clients (mon, info->clients); +hmp_info_vnc_authcrypt (mon," " , info->auth, + info->has_vencrypt ? &info->vencrypt :NULL ); +if (info->has_display) { +monitor_printf (mon," Display: %s\n" , info->display); } + info2l = info2l->next;
Looks like you've squashed two things together: factoring out helpers (without changing output), and extending output. I'd keep them separate. You decide.
Quoted 9 lines
hmp.c +2
-2} -out: -qapi_free_VncInfo (info); +qapi_free_VncInfo2List (info2l); + }#ifdef CONFIG_SPICE
Quoted 68 lines · * Markus Armbruster ([email protected]) wrote:
Quoted 66 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
The QMP query-vnc interfaces have gained a lot more information that the HMP interfaces hasn't got yet. Update it.
Note the output format has changed, but this is HMP so that's OK.
In particular, this now includes client information for reverse connections:
-vnc :0
(qemu) info vnc
default:
Server: 0.0.0.0:5900 (ipv4)
Auth: none (Sub: none)
Auth: none (Sub: none)(Now connect a client)
(qemu) info vnc
default:
Server: 0.0.0.0:5900 (ipv4)
Auth: none (Sub: none)
Client: 127.0.0.1:51828 (ipv4)x509_dname: none username: noneAuth: none (Sub: none)
-vnc localhost:7000,reverse
(qemu) info vnc
default:
Client: ::1:7000 (ipv6)x509_dname: none username: noneAuth: none (Sub: none)
-vnc :1,password,id=rev -vnc localhost:7000,reverse (qemu) info vnc
default:
Client: ::1:7000 (ipv6)x509_dname: none username: noneAuth: none (Sub: none)
rev:
Server: 0.0.0.0:5901 (ipv4)
Auth: vnc (Sub: none)
Client: 127.0.0.1:53616 (ipv4)x509_dname: none username: noneAuth: vnc (Sub: none)
This was originally RH bz 1461682
Signed-off-by Dr. David Alan Gilbert <[email protected]>--- hmp.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 67 insertions(+), 31 deletions(-)hmp.c +0
-0@@ -600,50 +600,86 @@ void hmp_info_blockstats(Monitor *mon, const QDict *qdict)qapi_free_BlockStatsList (stats_list); }
Quoted 3 lines
hmp.c void hmp_info_blockstats(Monitor *mon, const QDict *qdict) · line 603
Quoted 1 line
hmp.c +1
-0+/* Helper for hmp_info_vnc_clients, _servers */ Not sure this comment is worth its keep.
I prefer over rather than under commenting and I was worried that the hmp_info_ prefix would make it look like another command handler.
Quoted 15 lines
hmp.c void hmp_info_blockstats(Monitor *mon, const QDict *qdict) · line 615
Quoted 12 lines
hmp.c +12
-0+static void hmp_info_VncBasicInfo (Monitor *mon,VncBasicInfo *info, +const char *name) +{ +monitor_printf (mon," %s: %s:%s (%s%s)\n" , + name, + info->host, + info->service, + NetworkAddressFamily_lookup[info->family], + info->websocket ?" (Websocket)" :"" ); +} + +/* Helper displaying and euth and crypt info */ "euth"? Do you mean "auth"?
Quoted 1 line
Not sure this comment is worth its keep.
Ah well spotted; fixed.
Quoted 11 lines
Quoted 11 lines
hmp.c +11
-0+static void hmp_info_vnc_authcrypt (Monitor *mon,const char *indent, +VncPrimaryAuth auth, +VncVencryptSubAuth *vencrypt) +{ +monitor_printf (mon,"%sAuth: %s (Sub: %s)\n" , indent, + VncPrimaryAuth_lookup[auth], + vencrypt ? VncVencryptSubAuth_lookup[*vencrypt] :"none" ); +} + +static void hmp_info_vnc_clients (Monitor *mon,VncClientInfoList *client) +{
Quoted 8 lines
hmp.c void hmp_info_blockstats(Monitor *mon, const QDict *qdict) · line 630
Quoted 4 lines
hmp.c +4
-0+while (client) { +VncClientInfo *cinfo = client->value; + +hmp_info_VncBasicInfo (mon, (VncBasicInfo *)cinfo,"Client" );QAPI provides a type-safe conversion from type to base type:
qapi_TYPE_base(). You could writeqapi_VncClientInfo_base(cinfo)here. More conversions to base type below.
OK, used.
Quoted 9 lines
hmp.c void hmp_info_blockstats(Monitor *mon, const QDict *qdict) · line 636
Quoted 6 lines
hmp.c +6
-0+monitor_printf (mon," x509_dname: %s\n" , + cinfo->x509_dname ? + cinfo->x509_dname :"none" ); +monitor_printf (mon," username: %s\n" , + cinfo->has_sasl_username ? + cinfo->sasl_username :"none" );When an optional QAPI member FOO is a pointer, checking FOO instead of
has_FOOis totally fine. But mixing the two in one breath looks odd.
Yep, that's already fixed becased on previous reviews; that code is just a few lines moved from the old code.
Quoted 6 lines
I'd like to get rid of the redundant
has_FOOssome day.hmp.c void hmp_info_blockstats(Monitor *mon, const QDict *qdict) · line 639
Quoted 3 lines
hmp.c +3
-0+ + client = client->next; + }
Quoted 6 lines
I prefer to keep loop control in one place, and I also prefer not to change parameters:
for (cl = client; cl; cl = cl->next) {Matter of taste; your artistic license applies. More of the same below.
I'd rather leave as is; they're trivial loops.
Quoted 64 lines
Quoted 64 lines
hmp.c +25
-29+} + +static void hmp_info_vnc_servers (Monitor *mon,VncServerInfo2List *server) +{ +while (server) { +VncServerInfo2 *sinfo = server->value; +hmp_info_VncBasicInfo (mon, (VncBasicInfo *)sinfo,"Server" ); +hmp_info_vnc_authcrypt (mon," " , sinfo->auth, + sinfo->has_vencrypt ? &sinfo->vencrypt :NULL ); + server = server->next; + } +} +void hmp_info_vnc (Monitor *mon,const QDict *qdict) { -VncInfo *info; +VncInfo2List *info2l;Error *err =NULL ; -VncClientInfoList *client; - info =qmp_query_vnc (&err); + info2l =qmp_query_vnc_servers (&err);if (err) {error_report_err (err);return ; } - -if (!info->enabled) { -monitor_printf (mon,"Server: disabled\n" ); - goto out; - } - -monitor_printf (mon,"Server:\n" ); -if (info->has_host && info->has_service) { -monitor_printf (mon," address: %s:%s\n" , info->host, info->service); - } -if (info->has_auth) { -monitor_printf (mon," auth: %s\n" , info->auth); +if (!info2l) { +monitor_printf (mon,"None\n" ); +return ; } -if (!info->has_clients || info->clients ==NULL ) { -monitor_printf (mon,"Client: none\n" ); - }else { -for (client = info->clients; client; client = client->next) { -monitor_printf (mon,"Client:\n" ); -monitor_printf (mon," address: %s:%s\n" , - client->value->host, - client->value->service); -monitor_printf (mon," x509_dname: %s\n" , - client->value->x509_dname ? - client->value->x509_dname :"none" ); -monitor_printf (mon," username: %s\n" , - client->value->has_sasl_username ? - client->value->sasl_username :"none" ); +while (info2l) { +VncInfo2 *info = info2l->value; +monitor_printf (mon,"%s:\n" , info->id); +hmp_info_vnc_servers (mon, info->server); +hmp_info_vnc_clients (mon, info->clients); +hmp_info_vnc_authcrypt (mon," " , info->auth, + info->has_vencrypt ? &info->vencrypt :NULL );
Quoted 8 lines
hmp.c void hmp_info_blockstats(Monitor *mon, const QDict *qdict) · line 678
Quoted 4 lines
hmp.c +3
-0+if (info->has_display) { +monitor_printf (mon," Display: %s\n" , info->display); } + info2l = info2l->next;Looks like you've squashed two things together: factoring out helpers (without changing output), and extending output. I'd keep them separate. You decide.
What I did was rewrite it for the new structure; I borrowed ~ 2 statements from the old code, both of which got changed).
Dave
Quoted 9 lines
hmp.c void hmp_info_blockstats(Monitor *mon, const QDict *qdict) · line 685
Quoted 9 lines
hmp.c +2
-2} -out: -qapi_free_VncInfo (info); +qapi_free_VncInfo2List (info2l); + }#ifdef CONFIG_SPICE
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK