Re: [Qemu-devel] Re: [PATCH] vnc: Fix password expiration through 'change vnc ""'

[Markus Armbruster]

Anthony Liguori <address@hidden> writes:

> On 02/03/2011 11:02 AM, Daniel P. Berrange wrote:
>> On Thu, Feb 03, 2011 at 10:35:51AM -0600, Anthony Liguori wrote:
>>    
>>> On 02/03/2011 10:29 AM, Daniel P. Berrange wrote:
>>>      
>>>> On Mon, Jan 31, 2011 at 02:43:19PM -0600, Anthony Liguori wrote:
>>>>        
>>>>> commit 52c18be9e99dabe295321153fda7fce9f76647ac introduced a regression 
>>>>> in the
>>>>> change vnc password command that changed the behavior of setting the VNC
>>>>> password to an empty string from disabling login to disabling 
>>>>> authentication.
>>>>>
>>>>> This commit refactors the code to eliminate this overloaded semantics in
>>>>> vnc_display_password and instead introduces the 
>>>>> vnc_display_disable_login.   The
>>>>> monitor implementation then determines the behavior of an empty or missing
>>>>> string.
>>>>>          
>>>> Personally I think this is a little overkill&   just reverting the
>>>> original patch was fine, but from a functional POV your patch
>>>> produces the same results, so I won't argue.
>>>>        
>>> For 0.15, I'd like to introduce a new set of commands such that we
>>> don't multiplex the change command anymore.   This refactoring lays
>>> the ground work for that.
>>>
>>> For instance, if you created a block device with the name 'vnc',
>>> you'd get very unexpected results!  Multiplexing based on special
>>> values on top of existing commands is pretty evil.
>>>      
>> Doesn't Gerd's 'set_password' command already replace the functionality
>> of the 'change vnc' command. So we should likely declare 'change vnc'
>> as deprecated in 0.14 and remove it in 0.16
>>    
>
> Yup.  But it doesn't let you disable login.  Since that was a feature
> of 'change vnc', I think we need to provide a proper interface to do
> this.
>
> Likewise, we need a new interface for changing the block device.  The
> way password setting is handled is fubar right now.

Not just that.

> I've got some new commands documented in a git tree if you're interested.
>
> http://repo.or.cz/w/qemu/aliguori.git/blob/refs/heads/glib:/qmp-schema.json

The command to insert media into an empty block backend needs to support
same range of media options as the command to create a block backend
with media (that command is currently caught within drive_add,
struggling to get out).

My efforts to provide exactly that got bogged down last year by
protracted design discussions on the finer points of its QMP interface,
and the need to clean up and fix N+1 things before I can even start
attacking the goal problem.  Fortunately, much of the preliminary work
got in.  Unfortunately, I haven't been able to work on the rest, lately
:(

Media change is just a convenience command fusing eject and insert.  For
even more convenience, we can make it inherit option defaults from the
media being replaced.