1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH REBASE 0/5] qemu: fix domain object wait to handle monitor errors
  4. View patch

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

Nikolay Shirokovskiy posted 5 patches 7 years ago
[libvirt] [PATCH REBASE 1/5] qemu: erase synchronous block job cancel mentions in comments
Posted by Nikolay Shirokovskiy 7 years ago 
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@virtuozzo.com>
---
 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
+     * 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. */
     if (!async) {
         qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
         qemuBlockJobUpdate(driver, vm, QEMU_ASYNC_JOB_NONE, disk, NULL);
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH REBASE 1/5] qemu: erase synchronous block job cancel mentions in comments
Posted by John Ferlan 7 years ago 

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@virtuozzo.com>
> ---
>  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@redhat.com>

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);
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH REBASE 1/5] qemu: erase synchronous block job cancel mentions in comments
Posted by Nikolay Shirokovskiy 7 years ago 

On 01.05.2018 01:03, John Ferlan wrote:
> 
> 
> 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@virtuozzo.com>
>> ---
>>  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)..
I can not use your version unchanged too... It starts saying that we wait but
we wait only if ABORT_ASYNC is not set. I'll reword the proposed version
too to eliminate the mention that we use qemu's async block job cancel
API.

> 
> Reviewed-by: John Ferlan <jferlan@redhat.com>
> 
> 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);
>>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH REBASE 1/5] qemu: erase synchronous block job cancel mentions in comments
Posted by Nikolay Shirokovskiy 6 years, 12 months ago 
pushed slightly reworded

On 03.05.2018 10:01, Nikolay Shirokovskiy wrote:
> 
> 
> On 01.05.2018 01:03, John Ferlan wrote:
>>
>>
>> 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@virtuozzo.com>
>>> ---
>>>  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)..
> I can not use your version unchanged too... It starts saying that we wait but
> we wait only if ABORT_ASYNC is not set. I'll reword the proposed version
> too to eliminate the mention that we use qemu's async block job cancel
> API.
> 
>>
>> Reviewed-by: John Ferlan <jferlan@redhat.com>
>>
>> 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);
>>>
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.