[libvirt] [PATCH 2/3] qemu: map suspended state reason to suspended event detail

[Nikolay Shirokovskiy]

On 16.10.2018 21:42, John Ferlan wrote:
> 
> $SUBJ:
> 
> s/map/Map
> 
> On 10/10/18 4:04 AM, Nikolay Shirokovskiy wrote:
>> Map is based on existing cases in code where we send suspended
>> event after changing domain state to paused.
>>
>> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
>> ---
>>  src/qemu/qemu_domain.c  | 34 ++++++++++++++++++++++++++++++++++
>>  src/qemu/qemu_domain.h  |  3 +++
>>  src/qemu/qemu_process.c |  9 ++++++---
>>  3 files changed, 43 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index f00f1b3..557743b 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -13558,3 +13558,37 @@ qemuDomainRunningReasonToResumeEvent(virDomainRunningReason reason)
>>  
>>      return VIR_DOMAIN_EVENT_RESUMED_UNPAUSED;
>>  }
>> +
>> +
>> +virDomainEventSuspendedDetailType
>> +qemuDomainPausedReasonToSuspendedEvent(virDomainPausedReason reason)
>> +{
>> +    switch (reason) {
>> +    case VIR_DOMAIN_PAUSED_MIGRATION:
>> +        return VIR_DOMAIN_EVENT_SUSPENDED_MIGRATED;
>> +
>> +    case VIR_DOMAIN_PAUSED_FROM_SNAPSHOT:
>> +        return VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT;
>> +
>> +    case VIR_DOMAIN_PAUSED_POSTCOPY_FAILED:
>> +        return VIR_DOMAIN_EVENT_SUSPENDED_POSTCOPY_FAILED;
>> +
>> +    case VIR_DOMAIN_PAUSED_POSTCOPY:
>> +        return VIR_DOMAIN_EVENT_SUSPENDED_POSTCOPY;
>> +
>> +    case VIR_DOMAIN_PAUSED_UNKNOWN:
>> +    case VIR_DOMAIN_PAUSED_USER:
>> +    case VIR_DOMAIN_PAUSED_SAVE:
>> +    case VIR_DOMAIN_PAUSED_DUMP:
>> +    case VIR_DOMAIN_PAUSED_IOERROR:
>> +    case VIR_DOMAIN_PAUSED_WATCHDOG:
>> +    case VIR_DOMAIN_PAUSED_SHUTTING_DOWN:
>> +    case VIR_DOMAIN_PAUSED_SNAPSHOT:
> 
> This one may cause issues in the next patch...
> 
>> +    case VIR_DOMAIN_PAUSED_CRASHED:
>> +    case VIR_DOMAIN_PAUSED_STARTING_UP:
>> +    case VIR_DOMAIN_PAUSED_LAST:
>> +        break;
>> +    }
>> +
>> +    return VIR_DOMAIN_EVENT_SUSPENDED_PAUSED;
>> +}
>> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
>> index 380ea14..ee88266 100644
>> --- a/src/qemu/qemu_domain.h
>> +++ b/src/qemu/qemu_domain.h
>> @@ -1093,4 +1093,7 @@ void qemuDomainStorageIdReset(qemuDomainObjPrivatePtr priv);
>>  virDomainEventResumedDetailType
>>  qemuDomainRunningReasonToResumeEvent(virDomainRunningReason reason);
>>  
>> +virDomainEventSuspendedDetailType
>> +qemuDomainPausedReasonToSuspendedEvent(virDomainPausedReason reason);
>> +
>>  #endif /* __QEMU_DOMAIN_H__ */
>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>> index 27021b9..6858377 100644
>> --- a/src/qemu/qemu_process.c
>> +++ b/src/qemu/qemu_process.c
>> @@ -642,7 +642,7 @@ qemuProcessHandleStop(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
>>      virQEMUDriverPtr driver = opaque;
>>      virObjectEventPtr event = NULL;
>>      virDomainPausedReason reason;
>> -    virDomainEventSuspendedDetailType detail = VIR_DOMAIN_EVENT_SUSPENDED_PAUSED;
>> +    virDomainEventSuspendedDetailType detail;
>>      virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>>      qemuDomainObjPrivatePtr priv = vm->privateData;
>>  
>> @@ -668,8 +668,11 @@ qemuProcessHandleStop(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
>>              }
>>          }
>>  
>> -        VIR_DEBUG("Transitioned guest %s to paused state, reason %s",
>> -                  vm->def->name, virDomainPausedReasonTypeToString(reason));
>> +        detail = qemuDomainPausedReasonToSuspendedEvent(reason);
> 
> This setting of @detail overrides the @detail set just above...

But overrides to the same value.

> 
> It seems we can remove the @detail setting above in this patch as well
> as the need for the { } and { } around the setting of @reason in this
> patch instead of the subsequent one. Also, may as well place the check
> on the same line since it fits, e.g.:
> 
>     if (priv->job.current->status == QEMU_DOMAIN_JOB_STATUS_POSTCOPY)
>         reason = VIR_DOMAIN_PAUSED_POSTCOPY;
>     else
>         reason = VIR_DOMAIN_PAUSED_MIGRATION;
> 
> With that adjustment,
> 
> Reviewed-by: John Ferlan <jferlan at redhat.com>
> 
> John
> 
> Similar to patch 1, I can make the changes for you...

I'm ok here too, but we can left removing setting details for QEMU_DOMAIN_JOB_STATUS_POSTCOPY
as well as this patch correct in this aspect.

Nikolay

> 
>> +        VIR_DEBUG("Transitioned guest %s to paused state, "
>> +                  "reason %s, event detail %d",
>> +                  vm->def->name, virDomainPausedReasonTypeToString(reason),
>> +                  detail);
>>  
>>          if (priv->job.current)
>>              ignore_value(virTimeMillisNow(&priv->job.current->stopped));
>>