[libvirt] [PATCH] qemu: Fix all callers of qemuMonitorGetBlockJobInfo()
Andrea Bolognani
abologna at redhat.com
Mon Jan 4 15:10:56 UTC 2016
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
More information about the libvir-list
mailing list