On 08.11.19 10:26, Maxim Levitsky wrote: > On Fri, 2019-10-04 at 20:53 +0200, Max Reitz wrote: >> On 13.09.19 00:30, Maxim Levitsky wrote: >>> Signed-off-by: Maxim Levitsky >>> --- >>> block/Makefile.objs | 2 +- >>> block/amend.c | 116 ++++++++++++++++++++++++++++++++++++++ >>> include/block/block_int.h | 23 ++++++-- >>> qapi/block-core.json | 26 +++++++++ >>> qapi/job.json | 4 +- >>> 5 files changed, 163 insertions(+), 8 deletions(-) >>> create mode 100644 block/amend.c [...] >>> +static int coroutine_fn blockdev_amend_run(Job *job, Error **errp) >>> +{ >>> + BlockdevAmendJob *s = container_of(job, BlockdevAmendJob, common); >>> + int ret; >>> + >>> + job_progress_set_remaining(&s->common, 1); >>> + ret = s->bs->drv->bdrv_co_amend(s->bs, s->opts, s->force, errp); >>> + job_progress_update(&s->common, 1); >> >> It would be nice if the amend job could make use of the progress >> reporting that we have in place for amend. > > I also thought about it, but is it worth it? > > I looked through the status reporting of the qcow2 amend > code (which doesn't really allowed to be run through > qmp blockdev-amend, due to complexity of changing > the qcow2 format on the fly). True, and we could always add it later. I suppose I was mostly wondering because bdrv_amend_options already has all of that infrastructure and I was assuming that qcow2's bdrv_co_amend implementation would make some use of the existing function. Well, it doesn’t, so *shrug* [...] >>> + /* >>> + * Create the block job >>> + * TODO Running in the main context. Block drivers need to error out or add >>> + * locking when they use a BDS in a different AioContext. >> >> Why shouldn’t the job just run in the node’s context? > > This is shameless copy&pasta from the blockdev-create code > (which I did note in the copyright of the file) Well, you noted that it’s heavily based on it, not that it’s just C&P. So I suppose the comment is just wrong here? Max