On Wed, Feb 18, 2026 at 01:10:09PM +0100, Markus Armbruster wrote:
> Daniel P. Berrangé <[email protected]> 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 <[email protected]>
> > Signed-off-by: Daniel P. Berrangé <[email protected]>
> > ---
> > 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 :|
