On 07/04/2018 01:42 PM, Peter Krempa wrote:
> On Wed, Jul 04, 2018 at 12:46:53 +0200, Michal Privoznik wrote:
>> This event is emitted on the monitor if one of pr-managers lost
>> connection to its pr-helper process. What libvirt needs to do is
>> restart the pr-helper process iff it corresponds to managed
>> pr-manager.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
>> ---
>> src/qemu/qemu_monitor.c | 15 +++++++++++++
>> src/qemu/qemu_monitor.h | 11 +++++++++
>> src/qemu/qemu_monitor_json.c | 23 +++++++++++++++++++
>> src/qemu/qemu_process.c | 53 ++++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 102 insertions(+)
>
> [...]
>
>
>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>> index f200729cb1..94b7de76d7 100644
>> --- a/src/qemu/qemu_process.c
>> +++ b/src/qemu/qemu_process.c
>> @@ -1615,6 +1615,58 @@ qemuProcessHandleDumpCompleted(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
>> }
>>
>>
>> +static int
>> +qemuProcessHandlePRManagerStatusChanged(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
>> + virDomainObjPtr vm,
>> + const char *prManager,
>> + bool connected,
>> + void *opaque ATTRIBUTE_UNUSED)
>> +{
>> + qemuDomainObjPrivatePtr priv;
>> + size_t i;
>> + int ret = -1;
>> +
>> + virObjectLock(vm);
>> +
>> + VIR_DEBUG("pr-manager %s status changed for domain %p %s connected=%d",
>> + prManager, vm, vm->def->name, connected);
>> +
>> + if (connected) {
>> + /* Connect events are boring. */
>> + ret = 0;
>> + goto cleanup;
>> + }
>> + /* Disconnect events are more interesting. */
>> +
>> + for (i = 0; i < vm->def->ndisks; i++) {
>> + const char *mgralias;
>> +
>> + mgralias = virStorageSourceChainGetManagedPRAlias(vm->def->disks[i]->src);
>> +
>> + if (STREQ_NULLABLE(prManager, mgralias))
>> + break;
>
> I'm not a fan of this. We always know which is the managed alias
Do we? I thought the whole point of storing it in source private data
was to make it independent of qemuDomainGetManagedPRAlias().
Imagine I started a domain with pr-manager.alias = pr-helper0. Then
restarted libvirtd to upgrade it. This new version has, however,
pr-helper1 as default alias (because reasons). Then all of a sudden I
receive this event with alias pr-helper0 (because that is how I started
the domain). I can't do plain:
STREQ(prManager, qemuDomainGetManagedPRAlias())
can I?
> and we
> also know if it is supposed to be running.
>
> The hotplug code already does not inspect disks to do this since there
> is only one instance.
Sure. But hotplug code does not have to care about old instances.
>
>> + }
>> +
>> + if (i == vm->def->ndisks) {
>> + VIR_DEBUG("pr-manager %s not managed, ignoring event",
>> + prManager);
>> + ret = 0;
>> + goto cleanup;
>> + }
>> +
>> + priv = vm->privateData;
>> + priv->prDaemonRunning = false;
>> +
>> + if (qemuProcessStartManagedPRDaemon(vm) < 0)
>
> This has a timeout built in. Thus executing this from the event loop
> will make the whole libvirtd get stuck until it starts. This should not
> be in the event loop.
Ah good point. I'll rewrite this to run it in the qemu event process
thread (or what do we call it).
>
> Also does every disconnect equal to the daemon crashing/stopping?
It should. If qemu sends us bogus events I'm not sure there's much we
can do about it.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
