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