[libvirt] [PATCH 1/2] qemu: set jobinfo type to CANCELLED if migration is cancelled in all conditions

[Martin Kletzander]

On Fri, Nov 28, 2014 at 02:24:56PM +0800, Wang Rui wrote:
>On 2014/11/27 18:48, Martin Kletzander wrote:
>> On Thu, Nov 27, 2014 at 04:26:24PM +0800, Wang Rui wrote:
>>> The migration job status is traced in qemuMigrationUpdateJobStatus which is
>>> called in qemuMigrationRun. But if migration is cancelled before the trace
>>> such as in qemuMigrationDriveMirror, the jobinfo type won't be update to
>>> CANCELLED. After this patch, we can get jobinfo type as CANCELLED if migration
>>> is cancelled during the drive mirror migration.
>>>
>>> Signed-off-by: Wang Rui <moon.wangrui at huawei.com>
>>> ---
>>> src/qemu/qemu_migration.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
>>> index a1b1458..c678ba7 100644
>>> --- a/src/qemu/qemu_migration.c
>>> +++ b/src/qemu/qemu_migration.c
>>> @@ -1504,6 +1504,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver,
>>>                  * as this is a critical section so we are guaranteed
>>>                  * priv->job.asyncAbort will not change */
>>>                 qemuDomainObjExitMonitor(driver, vm);
>>> +                priv->job.current->type = VIR_DOMAIN_JOB_CANCELLED;
>>>                 virReportError(VIR_ERR_OPERATION_ABORTED, _("%s: %s"),
>>>                                qemuDomainAsyncJobTypeToString(priv->job.asyncJob),
>>>                                _("canceled by client"));
>>> @@ -3611,6 +3612,7 @@ qemuMigrationRun(virQEMUDriverPtr driver,
>>>          * as this is a critical section so we are guaranteed
>>>          * priv->job.asyncAbort will not change */
>>>         qemuDomainObjExitMonitor(driver, vm);
>>> +        priv->job.current->type = VIR_DOMAIN_JOB_CANCELLED;
>>>         virReportError(VIR_ERR_OPERATION_ABORTED, _("%s: %s"),
>>>                        qemuDomainAsyncJobTypeToString(priv->job.asyncJob),
>>>                        _("canceled by client"));
>>> --
>>> 1.7.12.4
>>>
>>
>> Can't we use qemuMigrationUpdateJobStatus() here?
>>
>> Martin
>
>Hi, thanks for your reply.
>
>I don't think we can.
>I tried this way but couldn't get jobinfo type as CANCELLED if I cancelled it
>during drive mirror migration. When libvirt sends qmp "migrate_cancel" to QEMU,
>QEMU only sets its state if its old state is MIG_STATE_SETUP or MIG_STATE_ACTIVE.
>But in drive mirror migration, QEMU's state is MIG_STATE_NONE.
>
>static void migrate_fd_cancel(MigrationState *s)
>{
>...
>
>        old_state = s->state;
>        if (old_state != MIG_STATE_SETUP && old_state != MIG_STATE_ACTIVE) {
>            break;
>...
>}
>
>So we haven't set QEMU's state to CANCELLED. And then if we query status in libvirt
>by qemuMigrationUpdateJobStatus, QEMU won't tell us the right status.
>
>MigrationInfo *qmp_query_migrate(Error **errp)
>{
>...
>    switch (s->state) {
>    case MIG_STATE_NONE:
>        /* no migration has happened ever */
>        break;
>...
>}
>
>Another reason why I don't use qemuMigrationUpdateJobStatus is considering job failed in
>libvirt(in my patch 2/2). If job is failed in libvirt, it's not accurate to query job
>status from QEMU.
>

Yes, you're right, that basically what I wanted to know, but was too
busy searching for.  ACK to this patch then.  Would you be OK with me
rewording the commit message as follows?

    The migration job status is traced in qemuMigrationUpdateJobStatus
    which is called in qemuMigrationRun. But if migration is cancelled
    before the trace such as in qemuMigrationDriveMirror, the jobinfo
    type won't be updated to CANCELLED. After this patch, we can get
    jobinfo type CANCELLED if migration is cancelled during drive
    mirror.  Moreover, we can't use qemuMigrationUpdateJobStatus
    because from qemu's point of view it's just the drive mirror being
    cancelled and the migration hasn't even started yet.

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141130/458e769b/attachment-0001.sig>