Fabian Ebner <[email protected]> writes: > Am 28.10.21 um 21:37 schrieb Markus Armbruster: >> Markus Armbruster <[email protected]> writes: >> >>> Stefan Reiter <[email protected]> writes: >>> >>>> 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, thanks! >> >> Unqueued, because it fails to compile with --disable-vnc and with >> --disable-spice. I failed to catch this in review, sorry. >> >> Making it work takes a tiresome amount of #ifdeffery (sketch appended). >> Missing: removal of stubs that are no longer used, >> e.g. vnc_display_password() in ui/vnc-stubs.c. Feels like more trouble >> than it's worth. >> >> To maximize our chances to get this into 6.2, please respin without the >> 'if' conditionals. Yes, this makes introspection less useful, but it's >> no worse than before the patch. > > Unfortunately, Stefan is no longer working with Proxmox, so I would > pick up these patches instead.
Appreciated! > Since the 6.2 ship has long sailed, I suppose the way forward is using > the #ifdefs then? Maybe. We can choose to improve introspection: keep the 'if' conditionals, and fix the C code to compile with --disable-vnc and --disable-spice. Or we can leave it imperfect as it was: drop the 'if' conditionals. If we had a concrete need for better introspection here, the choice would be easy. But as far as I know, we don't. Is better introspection worth the additional work anyway? Since you're volunteering to do the work, you get to decide :) > From my understanding what should be done is: > > * In the first patch, fix the typo spotted by Eric Blake and add the > R-b tags from this round. > > * Replace "since 6.2" with "since 7.0" everywhere. > > * Incorporate the #ifdef handling from below. I had to add another one > for the when/whenstr handling in qmp_expire_password to avoid an > error with -Werror=unused-but-set-variable. > > * Add #ifdefs for the unused stubs too? If yes, how to best find them? For every call you put under #if, check whether there are are any unconditional calls left, and if not, whether there is a stub function for it. If this is too terse, just ask for more help.
