From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Fabian Ebner <f.ebner@proxmox.com>
Cc: w.bumiller@proxmox.com, berrange@redhat.com,
qemu-devel@nongnu.org, armbru@redhat.com,
marcandre.lureau@gmail.com, kraxel@redhat.com,
pbonzini@redhat.com, marcandre.lureau@redhat.com,
eblake@redhat.com, t.lamprecht@proxmox.com
Subject: Re: [PATCH v8 0/3] VNC-related HMP/QMP fixes
Date: Wed, 2 Mar 2022 11:39:31 +0000 [thread overview]
Message-ID: <Yh9Xc766JwFT5ySD@work-vm> (raw)
In-Reply-To: <20220204101220.343526-1-f.ebner@proxmox.com>
* Fabian Ebner (f.ebner@proxmox.com) wrote:
> Original cover letter by Stefan R.:
>
> Since the removal of the generic 'qmp_change' command, one can no
> longer replace the 'default' VNC display listen address at runtime
> (AFAIK). For our users who need to set up a secondary VNC access port,
> this means configuring a second VNC display (in addition to our
> standard one for web-access), but it turns out one cannot set a
> password on this second display at the moment, as the 'set_password'
> call only operates on the 'default' display.
>
> Additionally, using secret objects, the password is only read once at
> startup. This could be considered a bug too, but is not touched in
> this series and left for a later date.
Queued
> v7 -> v8:
> * drop last patch deprecating SetPasswordAction values besides 'keep'
> for VNC (unfortunately, I don't have enough time to try implementing
> 'disconnect' and 'fail' for VNC in the near future)
> * drop if conditionals for DisplayProtocol enum to make compilation
> with --disable-spice and/or --disable-vnc work
> * order 'keep' first in enum, to fix how patch #3 uses it as an
> implicit default
> * also set connected and has_connected for the VNC options in
> hmp_set_password
> * fix typo in patch #1
> * add missing '#' for description in patch #3
>
> v6 -> v7:
> * remove g_strdup and g_free, use strings directly
> * squash in last patch
>
> v5 -> v6:
> * consider feedback from Markus' review, mainly:
> * fix crash bug in patch 1 (sorry, artifact of patch-splitting)
> * rely on '!has_param => param == NULL' to shorten code
> * add note to 'docs/about/deprecated.rst' and touch up comments a bit
> * go back to g_free instead of qapi_free_* since the latter apparently tries to
> free the passed in pointer which lives on the stack...
> * fix bug in HMP parsing (see patch 1)
>
> v4 -> v5:
> * add comment to patch 1 in "monitor-internal.h"
> * use qapi_free_SetPasswordOptions and friends, don't leak strdups
> * split QAPI change into 3 seperate patches
>
> v3 -> v4:
> * drop previously patch 1, this was fixed here instead:
> https://lists.gnu.org/archive/html/qemu-devel/2021-09/msg02529.html
> * patch 1: add Eric's R-b
> * patch 2: remove if-assignment, use 'deprecated' feature in schema
>
> v2 -> v3:
> * refactor QMP schema for set/expire_password as suggested by Eric Blake and
> Markus Armbruster
>
> v1 -> v2:
> * add Marc-André's R-b on patch 1
> * use '-d' flag as suggested by Eric Blake and Gerd Hoffmann
> * I didn't see a way to do this yet, so I added a "flags with values" arg type
>
> Stefan Reiter (3):
> monitor/hmp: add support for flag argument with value
> qapi/monitor: refactor set/expire_password with enums
> qapi/monitor: allow VNC display id in set/expire_password
>
> hmp-commands.hx | 24 ++++---
> monitor/hmp-cmds.c | 52 +++++++++++++-
> monitor/hmp.c | 19 +++++-
> monitor/monitor-internal.h | 3 +-
> monitor/qmp-cmds.c | 49 ++++----------
> qapi/ui.json | 134 ++++++++++++++++++++++++++++++++-----
> 6 files changed, 213 insertions(+), 68 deletions(-)
>
> --
> 2.30.2
>
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
prev parent reply other threads:[~2022-03-02 13:19 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-04 10:12 [PATCH v8 0/3] VNC-related HMP/QMP fixes Fabian Ebner
2022-02-04 10:12 ` [PATCH v8 1/3] monitor/hmp: add support for flag argument with value Fabian Ebner
2022-02-09 14:12 ` Markus Armbruster
2022-02-24 9:17 ` Fabian Ebner
2022-02-24 10:04 ` Markus Armbruster
2022-02-24 11:30 ` Dr. David Alan Gilbert
2022-02-04 10:12 ` [PATCH v8 2/3] qapi/monitor: refactor set/expire_password with enums Fabian Ebner
2022-02-09 13:43 ` Markus Armbruster
2022-02-04 10:12 ` [PATCH v8 3/3] qapi/monitor: allow VNC display id in set/expire_password Fabian Ebner
2022-02-09 14:07 ` Markus Armbruster
2022-02-17 7:42 ` Fabian Ebner
2022-02-17 8:04 ` Markus Armbruster
2022-02-23 13:45 ` Dr. David Alan Gilbert
2022-03-02 11:39 ` Dr. David Alan Gilbert [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Yh9Xc766JwFT5ySD@work-vm \
--to=dgilbert@redhat.com \
--cc=armbru@redhat.com \
--cc=berrange@redhat.com \
--cc=eblake@redhat.com \
--cc=f.ebner@proxmox.com \
--cc=kraxel@redhat.com \
--cc=marcandre.lureau@gmail.com \
--cc=marcandre.lureau@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=t.lamprecht@proxmox.com \
--cc=w.bumiller@proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).