Re: [Qemu-devel] [PATCH 2/5] socket: Add a reconnect option.

[Anthony Liguori <anthony@codemonkey.ws>]

On 02/01/2010 04:44 PM, Ian Molton wrote:
> Anthony Liguori wrote:
>
>    
>> I went back and looked at the last series and found my feedback.  I had
>> suggested that instead of automatically reconnecting, a mechanism should
>> be added for a user to initiate a reconnect.
>>      
> This sounds useful
>
>    
>> Additionally, we should emit events upon disconnect through QMP (now
>> that we have that functionality).
>>      
> This also.
>
>    
>> The main reason I dislike automatic reconnecting is that there is no
>> correct way to handle the period of time while the VM is disconnected.
>>      
> This is fine for egd protocol, the guest will just run low on entropy in
> the meantime. Nothing should break.
>    

Right, but you're adding a generic piece of functionality.  For 
instance, what's the result of using reconnect with virtio-console?  Is 
this something that is going to work well?

>> Auto reconnecting is implementing a policy to handle the failure within
>> QEMU which is not universally the correct choice.  This isn't so bad
>> except for the fact that you aren't providing the mechanisms for users
>> to implement other policies which means they're stuck with this
>> particular policy.
>>      
> Perhaps this feature could be added if needed in future? It seems a bit
> ambitious to get all this 'right' with no use cases to test against.
>    

I'm all for doing things incrementally but there has to be a big picture 
that the incremental bit fits into otherwise you end up with a bunch of 
random features that don't work together well.

Honestly, I'd strongly suggest splitting the reconnect logic out of the 
series when resubmitting.  I think it's just too hacky with too weak of 
a justification.  If you really want this functionality, we can discuss 
the right approach for doing it but it's gotta be done in a way that's 
not introducing a one-off case just for the random number generator.

Regards,

Anthony Liguori

> -Ian
>