Re: [Qemu-devel] [PATCH 4/5] pr-manager: add query-pr-managers QMP command

[Michal Prívozník]

On 06/27/2018 05:44 PM, Paolo Bonzini wrote:
> On 26/06/2018 18:31, Michal Privoznik wrote:
>>>  
>>> +static bool pr_manager_helper_is_connected(PRManager *p)
>>> +{
>>> +    PRManagerHelper *pr_mgr = PR_MANAGER_HELPER(p);
>>> +    bool result;
>>> +
>>> +    qemu_mutex_lock(&pr_mgr->lock);
>>> +    result = (pr_mgr->ioc != NULL);
>> I worry it is not that easy. pr_mgr->ioc is unset only when there's
>> PR_IN/PR_OUT command coming from the guest (in pr_manager_helper_run ->
>> pr_manager_helper_write). In fact, after 5/5 that is also the time when
>> the event is delivered. But that might be too late for mgmt app to
>> restart the helper process (although pr_manager_helper_run() tries to
>> reconnect for 5 seconds before giving up).
> 
> That's true, however the important thing IMO is to have a QMP interface
> that libvirt can use; everything else is just quality of implementation.
> 
> qemu-pr-helper anyway does something only when a guests sends it a PR
> command - and with libvirt's per-guest model, that would (hopefully)
> mean that the only case that remains is when someone manually kills the
> qemu-pr-helper process.  In that case there's a certain amount of PEBKAC
> involved... :)

Unless an assert() is triggered ;-)

But since you merged my suggested changes in 5/5 libvirt can catch the
event pretty soon, so in my testing qemu was still left with 3-4
connection retries which is plenty.

Michal