diff mbox series

[v2,12/16] block/mirror: Distinguish active from passive ops

Message ID 20180122220806.22154-13-mreitz@redhat.com
State New
Headers show
Series block/mirror: Add active-sync mirroring | expand

Commit Message

Max Reitz Jan. 22, 2018, 10:08 p.m. UTC
Currently, the mirror block job only knows passive operations.  But once
we introduce active writes, we need to distinguish between the two; for
example, mirror_wait_for_free_in_flight_slot() should wait for a passive
operation because active writes will not use the same in-flight slots.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/mirror.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Fam Zheng Feb. 27, 2018, 9:12 a.m. UTC | #1
On Mon, 01/22 23:08, Max Reitz wrote:
> Currently, the mirror block job only knows passive operations.  But once
> we introduce active writes, we need to distinguish between the two; for
> example, mirror_wait_for_free_in_flight_slot() should wait for a passive
> operation because active writes will not use the same in-flight slots.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/mirror.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/block/mirror.c b/block/mirror.c
> index 2363e79563..bb46f3c4e9 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -89,6 +89,7 @@ struct MirrorOp {
>      int64_t *bytes_handled;
>  
>      bool is_pseudo_op;
> +    bool is_active_write;
>      CoQueue waiting_requests;
>  
>      QTAILQ_ENTRY(MirrorOp) next;
> @@ -281,8 +282,10 @@ static inline void mirror_wait_for_free_in_flight_slot(MirrorBlockJob *s)
>           * some other operation to start, which may in fact be the
>           * caller of this function.  Since there is only one pseudo op
>           * at any given time, we will always find some real operation
> -         * to wait on. */
> -        if (!op->is_pseudo_op) {
> +         * to wait on.
> +         * Also, only non-active operations use up in-flight slots, so
> +         * we can ignore active operations. */
> +        if (!op->is_pseudo_op && !op->is_active_write) {
>              qemu_co_queue_wait(&op->waiting_requests, NULL);
>              return;
>          }
> -- 
> 2.14.3
> 

I'd just squash this patch into 14 to avoid code churn.

Fam
Max Reitz Feb. 28, 2018, 3:05 p.m. UTC | #2
On 2018-02-27 10:12, Fam Zheng wrote:
> On Mon, 01/22 23:08, Max Reitz wrote:
>> Currently, the mirror block job only knows passive operations.  But once
>> we introduce active writes, we need to distinguish between the two; for
>> example, mirror_wait_for_free_in_flight_slot() should wait for a passive
>> operation because active writes will not use the same in-flight slots.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>  block/mirror.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/block/mirror.c b/block/mirror.c
>> index 2363e79563..bb46f3c4e9 100644
>> --- a/block/mirror.c
>> +++ b/block/mirror.c
>> @@ -89,6 +89,7 @@ struct MirrorOp {
>>      int64_t *bytes_handled;
>>  
>>      bool is_pseudo_op;
>> +    bool is_active_write;
>>      CoQueue waiting_requests;
>>  
>>      QTAILQ_ENTRY(MirrorOp) next;
>> @@ -281,8 +282,10 @@ static inline void mirror_wait_for_free_in_flight_slot(MirrorBlockJob *s)
>>           * some other operation to start, which may in fact be the
>>           * caller of this function.  Since there is only one pseudo op
>>           * at any given time, we will always find some real operation
>> -         * to wait on. */
>> -        if (!op->is_pseudo_op) {
>> +         * to wait on.
>> +         * Also, only non-active operations use up in-flight slots, so
>> +         * we can ignore active operations. */
>> +        if (!op->is_pseudo_op && !op->is_active_write) {
>>              qemu_co_queue_wait(&op->waiting_requests, NULL);
>>              return;
>>          }
>> -- 
>> 2.14.3
>>
> 
> I'd just squash this patch into 14 to avoid code churn.

I wanted to pull out as much as possible from 14 since it is massive as
it is, but if you think it makes reviewing even harder... Then I can put
it in there, of course.

Max
diff mbox series

Patch

diff --git a/block/mirror.c b/block/mirror.c
index 2363e79563..bb46f3c4e9 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -89,6 +89,7 @@  struct MirrorOp {
     int64_t *bytes_handled;
 
     bool is_pseudo_op;
+    bool is_active_write;
     CoQueue waiting_requests;
 
     QTAILQ_ENTRY(MirrorOp) next;
@@ -281,8 +282,10 @@  static inline void mirror_wait_for_free_in_flight_slot(MirrorBlockJob *s)
          * some other operation to start, which may in fact be the
          * caller of this function.  Since there is only one pseudo op
          * at any given time, we will always find some real operation
-         * to wait on. */
-        if (!op->is_pseudo_op) {
+         * to wait on.
+         * Also, only non-active operations use up in-flight slots, so
+         * we can ignore active operations. */
+        if (!op->is_pseudo_op && !op->is_active_write) {
             qemu_co_queue_wait(&op->waiting_requests, NULL);
             return;
         }