On 10/03/2017 05:20 AM, Vladimir Sementsov-Ogievskiy wrote: > 03.10.2017 06:15, John Snow wrote: >> For jobs that complete when a monitor isn't looking, there's no way to >> tell what the job's final return code was. We need to allow jobs to >> remain in the list until queried for reliable management. >> >> This is an RFC; tests are on the way. >> (Tested only manually via qmp-shell for now.) > > That's a cool feature! > What about transactions support? >
Didn't test that yet (!) but the intent is that it will be compatible. The jobs in the transaction, whether using grouped-completion mode or not, will simply hang around in the list after completion. For grouped-completion=false: The jobs will complete individually and then remain in the list. For grouped-completion=true: The jobs will remain in their ready-to-commit-or-abort state until all jobs in the transaction are ready to commit-or-abort, then all jobs will either commit or abort. After commit-or-abort, all jobs (that were created with manual-cull=true !) will remain in the query list. The intended effect here is that this property changes NOTHING except that the job will remain in the query list until it is dismissed, and should not change anything about how it behaves during its lifetime. One downside here is that since we have no universal "job creation argument list" that I can't add it to all jobs universally. In the case of transactions, though, I could at least add a property that *forces* all jobs below to become manual-cull style jobs, and that way you'd only have to specify it once instead of for each action. --js >> >> John Snow (3): >> blockjob: add manual-cull property >> qmp: add block-job-cull command >> blockjob: expose manual-cull property >> >> block/backup.c | 20 +++++++++--------- >> block/commit.c | 2 +- >> block/mirror.c | 2 +- >> block/replication.c | 5 +++-- >> block/stream.c | 2 +- >> block/trace-events | 1 + >> blockdev.c | 28 +++++++++++++++++++++---- >> blockjob.c | 46 >> +++++++++++++++++++++++++++++++++++++++-- >> include/block/block_int.h | 8 +++++--- >> include/block/blockjob.h | 21 +++++++++++++++++++ >> include/block/blockjob_int.h | 2 +- >> qapi/block-core.json | 49 >> ++++++++++++++++++++++++++++++++++++-------- >> tests/test-blockjob-txn.c | 2 +- >> tests/test-blockjob.c | 2 +- >> 14 files changed, 155 insertions(+), 35 deletions(-) >> > > Oh, while I'm here, I should point out that another downside of this patch is that it doesn't prevent "cancel" from attempting to re-enter the job. Or rather, I had to patch that out specifically. The job remains in a list of jobs that some portions of the code consider to be "active" jobs. (look for any code that checks to see if the job has started.) A (perhaps) more provably cleaner approach would be to simply move any completed job onto a different list upon completion, and patch query-jobs to query both lists, and allow the cull command to remove any jobs on that list. A downside of that approach, however, is that without multiple job support, you may launch a second job that perhaps overwrites the first job unless you're careful about how you manage that data. There are pros and cons to either way, but I'd rather not get in the business of overhauling the blockjobs API unless it's for universal jobs. --js