Re: [libvirt] [PATCH REBASE 1/5] qemu: erase synchronous block job cancel mentions in comments

On 04/18/2018 10:44 AM, Nikolay Shirokovskiy wrote: > Commit [1] dropped support for synchronous block job cancel. > This patch erases remnants from comments. > > [1] commit 2350d101 "qemu: Remove support for legacy block jobs" > > Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com&gt; > --- > 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 4e06c9c..0dd6032 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -16914,13 +16914,12 @@ qemuDomainBlockJobAbort(virDomainPtr dom, > if (save) > ignore_value(virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps)); > > - /* With synchronous block cancel, we must synthesize an event, and > - * we silently ignore the ABORT_ASYNC flag. With asynchronous > - * block cancel, the event will come from qemu and will update the > - * XML as appropriate, but without the ABORT_ASYNC flag, we must > - * block to guarantee synchronous operation. We do the waiting > - * while still holding the VM job, to prevent newly scheduled > - * block jobs from confusing us. */ > + /* > + * With asynchronous block cancel, the event will come from qemu and will /* With... IOW: don't have a separate line for the open comment... > + * update the XML as appropriate, but without the ABORT_ASYNC flag, we must > + * block to guarantee synchronous operation. We do the waiting while still > + * holding the VM job, to prevent newly scheduled block jobs from confusing > + * us. */ This works, but the lead-in doesn't mean as much without the "With synchronous..." as the alternative... So perhaps even simpler: Wait for the QEMU event to update the the blockjob with the domain lock to prevent newly scheduled block jobs from confusing us. The event will update the XML as appropriate. Without the ABORT_ASYNC flag, we must block to guarantee synchronous completion. With at least the comment entry fixup... feel free to use my wording as well (as long as it makes sense to you)..

Reviewed-by: John Ferlan <jferlan(a)redhat.com&gt; but please wait for 4.4.0 to open before pushing. John > if (!async) { > qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); > qemuBlockJobUpdate(driver, vm, QEMU_ASYNC_JOB_NONE, disk, NULL); >