Re: [libvirt] [PATCH v2 5/8] Added job type VIR_DOMAIN_JOB_PHASE1_COMPLETED

On 07 Oct 2014, at 22:08 , Jiri Denemark <jdenemar(a)redhat.com&gt; wrote: > On Tue, Sep 30, 2014 at 16:39:26 +0200, Cristian Klein wrote: >> Some jobs feature several phases. For example, post-copy migration is >> composed of a first phase, from migration start to switching to >> post-copy, and a second phase, to migration completion. This >> job type allows to flag that the job has completed the first phase, >> but is not yet fully completed. >> >> Signed-off-by: Cristian Klein <cristian.klein(a)cs.umu.se&gt; >> --- >> include/libvirt/libvirt.h.in | 1 + >> tools/virsh-domain.c | 3 ++- >> 2 files changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in >> index 84cd5a4..81044f0 100644 >> --- a/include/libvirt/libvirt.h.in >> +++ b/include/libvirt/libvirt.h.in >> @@ -4307,6 +4307,7 @@ typedef enum { >> VIR_DOMAIN_JOB_COMPLETED = 3, /* Job has finished, but isn't cleaned up */ >> VIR_DOMAIN_JOB_FAILED = 4, /* Job hit error, but isn't cleaned up */ >> VIR_DOMAIN_JOB_CANCELLED = 5, /* Job was aborted, but isn't cleaned up */ >> + VIR_DOMAIN_JOB_PHASE1_COMPLETED = 6, /* Job completed first phase, e.g., post-copy activation */ > > This is not a job type. If we need to advertise this to libvirt clients, > we may introduce a new job statistics typed parameter but we should not > misuse job type. And I'm not entirely convinced we need to advertise > this. To me it seems the only interested thing is whether a domain is > still running on the source host or it was already resumed on the > destination host. And it's easy to get this kind of information via > existing ways, e.g., listening to domain life cycle events or by > checking domain's status. My rationale for introducing this flag was to prevent a future breakage of an internal API. I felt like none of the previous flags where accurately representing the status of this job after post-copy has started.

Sure, I could use `job->status.status` in `qemuMigrationWaitForCompletion`, but I was concerned that a future `libvirt` addition might not take this subtly into account and might break post-copy.

If you think I should go ahead, suppress this job type and use `job->status.status` instead, what should I use instead, `VIR_DOMAIN_JOB_UNBOUNDED` or `VIR_DOMAIN_JOB_BOUNDED`? To be honest, I’m not sure to have completely understood the difference between them, except that `VIR_DOMAIN_JOB_BOUNDED` seems to be used to signal that a job is almost complete.