On Mon, 2016-01-04 at 15:46 +0100, Michal Privoznik wrote: > On 04.01.2016 15:20, Andrea Bolognani wrote: > > Commit 1b43885 modified one of the callers of this function to take > > into account the possible return value of 0 when the block job can't be > > found. > > > > This commit finishes the job by updating the remaining caller. > > --- > > src/qemu/qemu_driver.c | 13 ++++++------- > > 1 file changed, 6 insertions(+), 7 deletions(-) > > > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > > index 304165c..c4573d9 100644 > > --- a/src/qemu/qemu_driver.c > > +++ b/src/qemu/qemu_driver.c > > @@ -16150,14 +16150,13 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver, > > rc = qemuMonitorGetBlockJobInfo(priv->mon, disk->info.alias, &info); > > if (qemuDomainObjExitMonitor(driver, vm) < 0) > > goto cleanup; > > - if (rc < 0) > > + if (rc <= 0) > > goto cleanup; > > - if (rc == 1 && > > - (info.ready == 1 || > > - (info.ready == -1 && > > - info.end == info.cur && > > - (info.type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY || > > - info.type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT)))) > > + if (info.ready == 1 || > > + (info.ready == -1 && > > + info.end == info.cur && > > + (info.type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY || > > + info.type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT))) > > disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY; > > } > > Interesting. So previously this code worked just right with no block > operation running on @disk. Now it will fail. I guess that's correct > approach since this function's job is to abort a block job. > > ACK I'm actually having second thoughts about this. We only call qemuMonitorGetBlockJobInfo() if !disk->mirrorState. However, having it return 0 before would skip reading from info (my main concern, as it would not have been filled in) and cause the check for disk->mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_READY immediately afterwards to fail and an error to be raised before jumping to cleanup. After my patch, the function will jump to cleanup earlier, still returning a negative error code, but not raising any error in the process. I guess what I'm trying to say is that, unless you can convince otherwise, I'm going to have to SNACK this one :) Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list