On Tue, Feb 21, 2017 at 15:38:22 +0100, Milan Zamazal wrote: > [Starting to move to the development list.] > > Milan Zamazal <mzamazal(a)redhat.com&gt; writes: > > > Jiri Denemark <jdenemar(a)redhat.com&gt; writes: > > > >> On Fri, Feb 17, 2017 at 12:38:24 +0100, Milan Zamazal wrote: > >>> > >>> There are basically two problems: > >>> > >>> - When the job completion callback is called, I need to distinguish what > >>> kind of job was it to perform the appropriate actions. It would be > >>> easier if I knew the job type directly in the callback (no need to > >>> coordinate anything), but "external" job tracking is also possible. > >> > >> An immediate answer would be: "don't rely on the completion callback and > >> just check the return value of the API which started the job", but I > >> guess you want it because checking the return value is not possible when > >> the process which started the job is not running anymore as described > >> below. > > > > Well, avoiding using the completion callback is probably OK for me. > > Thinking about it more, it's not very nice: I have to use the callback > to get the completed job stats (I'm not guaranteed the domain still > exists on the source host when I ask it for the stats explicitly) *and* > to track the jobs outside the callback to know whether the callback is > related to the type of domain jobs I'm going to handle. > > Although not absolutely necessary, it would be much nicer if the job > type was identified in the callback. The job completed event uses type parameters so adding a new parameter describing the just completed job should not be a problem.

On Mon, Apr 10, 2017 at 12:58:09PM +0200, Milan Zamazal wrote: > Jiri Denemark <jdenemar(a)redhat.com&gt; writes: > > > On Tue, Feb 21, 2017 at 15:38:22 +0100, Milan Zamazal wrote: > >> [Starting to move to the development list.] > >> > >> Milan Zamazal <mzamazal(a)redhat.com&gt; writes: > >> > >> > Jiri Denemark <jdenemar(a)redhat.com&gt; writes: > >> > > >> >> On Fri, Feb 17, 2017 at 12:38:24 +0100, Milan Zamazal wrote: > >> >>> > >> >>> There are basically two problems: > >> >>> > >> >>> - When the job completion callback is called, I need to distinguish what > >> >>> kind of job was it to perform the appropriate actions. It would be > >> >>> easier if I knew the job type directly in the callback (no need to > >> >>> coordinate anything), but "external" job tracking is also possible. > >> >> > >> >> An immediate answer would be: "don't rely on the completion callback and > >> >> just check the return value of the API which started the job", but I > >> >> guess you want it because checking the return value is not possible when > >> >> the process which started the job is not running anymore as described > >> >> below. > >> > > >> > Well, avoiding using the completion callback is probably OK for me. > >> > >> Thinking about it more, it's not very nice: I have to use the callback > >> to get the completed job stats (I'm not guaranteed the domain still > >> exists on the source host when I ask it for the stats explicitly) *and* > >> to track the jobs outside the callback to know whether the callback is > >> related to the type of domain jobs I'm going to handle. > >> > >> Although not absolutely necessary, it would be much nicer if the job > >> type was identified in the callback. > > > > The job completed event uses type parameters so adding a new parameter > > describing the just completed job should not be a problem. > > Great, so all what remains to solve the problem is to add the > parameter :-). (And I'd like to have the same info available when a job > is still running for reasons already discussed.) > > I looked how the change could be implemented. Could you please help me > clarify some things? > > - I think a new member should be added to _virDomainJobInfo for the > purpose. What would be a good name for it? Maybe "operation"? > - Do I need to care about backends other than QEMU? > - Jobs are classified by qemuDomainAsyncJob, which is a QEMU specific > type. Is it OK to use such structures in virsh-domain.c or is there > any additional abstraction needed? I don't much like the idea of exposing the QEMU job operation names in the public API. Perhaps we instead need to have the method which starts the job, return an integer "job id" that is then reported against the job, so apps can match them up.

On Mon, Apr 10, 2017 at 12:01:43 +0100, Daniel P. Berrange wrote: > On Mon, Apr 10, 2017 at 12:58:09PM +0200, Milan Zamazal wrote: > > I looked how the change could be implemented. Could you please help me > > clarify some things? > > > > - I think a new member should be added to _virDomainJobInfo for the > > purpose. What would be a good name for it? Maybe "operation"? > > - Do I need to care about backends other than QEMU? > > - Jobs are classified by qemuDomainAsyncJob, which is a QEMU specific > > type. Is it OK to use such structures in virsh-domain.c or is there > > any additional abstraction needed? > > I don't much like the idea of exposing the QEMU job operation names > in the public API. > > Perhaps we instead need to have the method which starts the job, return > an integer "job id" that is then reported against the job, so apps can > match them up. The problem with "job id" is that only the process which started the job would know what it means. Not to mention it would require a lot of API changes. I think we should just introduce a new virDomainJobSomething enum as VIR_DOMAIN_JOB_SOMETHING_INCOMING_MIGRATION, VIR_DOMAIN_JOB_SOMETHING_OUTGOING_MIGRATION, VIR_DOMAIN_JOB_SOMETHING_SAVE, VIR_DOMAIN_JOB_SOMETHING_RESTORE, ... and report it in virDomainGetJobStat (definitely not in _virDomainJobInfo as it would break ABI). I'm not sure what the best name for "Something" would be. "Operation", "Action", or something else?