On Wed, Feb 18, 2026 at 01:10:09PM +0100, Markus Armbruster wrote:
Daniel P. Berrangé <berrange@redhat.com> writes:
Neither the VNC or SPICE code for password changes provides error reporting at source, leading the callers to report a largely useless generic error message.
Fixing this removes one of the two remaining needs for the undesirable error_printf_unless_qmp() method.
While fixing this the error message hint is improved to recommend the 'password-secret' option which allows securely passing a password at startup.
Reported-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- include/ui/console.h | 2 +- include/ui/qemu-spice-module.h | 3 ++- tests/functional/generic/test_vnc.py | 4 ++-- ui/spice-core.c | 25 ++++++++++++++++++------- ui/spice-module.c | 7 ++++--- ui/ui-qmp-cmds.c | 19 ++++++------------- ui/vnc-stubs.c | 6 +++--- ui/vnc.c | 10 +++++++--- 8 files changed, 43 insertions(+), 33 deletions(-)
diff --git a/include/ui/console.h b/include/ui/console.h index 98feaa58bd..3677a9d334 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -457,7 +457,7 @@ void qemu_display_help(void); void vnc_display_init(const char *id, Error **errp); void vnc_display_open(const char *id, Error **errp); void vnc_display_add_client(const char *id, int csock, bool skipauth); -int vnc_display_password(const char *id, const char *password); +int vnc_display_password(const char *id, const char *password, Error **errp); int vnc_display_pw_expire(const char *id, time_t expires); void vnc_parse(const char *str); int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp); diff --git a/include/ui/qemu-spice-module.h b/include/ui/qemu-spice-module.h index 1f22d557ea..072efa0c83 100644 --- a/include/ui/qemu-spice-module.h +++ b/include/ui/qemu-spice-module.h @@ -29,7 +29,8 @@ struct QemuSpiceOps { void (*display_init)(void); int (*migrate_info)(const char *h, int p, int t, const char *s); int (*set_passwd)(const char *passwd, - bool fail_if_connected, bool disconnect_if_connected); + bool fail_if_connected, bool disconnect_if_connected, + Error **errp); int (*set_pw_expire)(time_t expires); int (*display_add_client)(int csock, int skipauth, int tls); #ifdef CONFIG_SPICE diff --git a/tests/functional/generic/test_vnc.py b/tests/functional/generic/test_vnc.py index f1dd1597cf..097f858ca1 100755 --- a/tests/functional/generic/test_vnc.py +++ b/tests/functional/generic/test_vnc.py @@ -48,7 +48,7 @@ def test_no_vnc_change_password(self): self.assertEqual(set_password_response['error']['class'], 'GenericError') self.assertEqual(set_password_response['error']['desc'], - 'Could not set password') + 'No VNC display is present');
def launch_guarded(self): try: @@ -73,7 +73,7 @@ def test_change_password_requires_a_password(self): self.assertEqual(set_password_response['error']['class'], 'GenericError') self.assertEqual(set_password_response['error']['desc'], - 'Could not set password') + 'VNC password authentication is disabled')
def test_change_password(self): self.set_machine('none') diff --git a/ui/spice-core.c b/ui/spice-core.c index 8a6050f4ae..cdcec34f67 100644 --- a/ui/spice-core.c +++ b/ui/spice-core.c @@ -756,7 +756,7 @@ static void qemu_spice_init(void) tls_ciphers); } if (password) { - qemu_spice.set_passwd(password, false, false); + qemu_spice.set_passwd(password, false, false, NULL);
qemu_spice.set_passwd is qemu_spice_set_passwd(). It's converted to Error below. That conversion doesn't replace error reporting, it only adds. Therefore, passing NULL does not lose error reporting here. Good.
However, why is ignoring errors okay here? Not this patch's fault, of course.
IIUC There are two scenarios qemu_spice_set_passwd can fail * The password auth method is not enabled -> minor config bug, but we should report that error * The fail_if_conn is true and a client is connected -> impossible scenario during startup, fine to ignore With regards, Daniel -- |: https://berrange.com ~~ https://hachyderm.io/@berrange :| |: https://libvirt.org ~~ https://entangle-photo.org :| |: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|