Am 08.11.2016 um 06:41 hat John Snow geschrieben: > On 11/03/2016 09:17 AM, Kevin Wolf wrote: > >Am 02.11.2016 um 18:50 hat John Snow geschrieben: > >>Refactor backup_start as backup_job_create, which only creates the job, > >>but does not automatically start it. The old interface, 'backup_start', > >>is not kept in favor of limiting the number of nearly-identical interfaces > >>that would have to be edited to keep up with QAPI changes in the future. > >> > >>Callers that wish to synchronously start the backup_block_job can > >>instead just call block_job_start immediately after calling > >>backup_job_create. > >> > >>Transactions are updated to use the new interface, calling block_job_start > >>only during the .commit phase, which helps prevent race conditions where > >>jobs may finish before we even finish building the transaction. This may > >>happen, for instance, during empty block backup jobs. > >> > >>Reported-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > >>Signed-off-by: John Snow <js...@redhat.com> > > > >>+static void drive_backup_commit(BlkActionState *common) > >>+{ > >>+ DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common); > >>+ if (state->job) { > >>+ block_job_start(state->job); > >>+ } > >> } > > > >How could state->job ever be NULL? > > > > Mechanical thinking. It can't. (I definitely didn't copy paste from > the .abort routines. Definitely.) > > >Same question for abort, and for blockdev_backup_commit/abort. > > > > Abort ... we may not have created the job successfully. Abort gets > called whether or not we made it to or through the matching > .prepare.
Ah, yes, I always forget about this. It's so counterintuitive (and bdrv_reopen() actually works differently, it only aborts entries that have successfully been prepared). Is there a good reason why qmp_transaction() works this way, especially since we have a separate .clean function? Kevin