All of lore.kernel.org
 help / color / mirror / Atom feed
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



      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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.