[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v5 3/6] blkdebug: track all actions

From: Max Reitz
Subject: Re: [PATCH v5 3/6] blkdebug: track all actions
Date: Thu, 15 Jul 2021 11:59:21 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0

On 14.06.21 10:29, Emanuele Giuseppe Esposito wrote:
Add a counter for each action that a rule can trigger.
This is mainly used to keep track of how many coroutine_yield()
we need to perform after processing all rules in the list.

Co-developed-by: Paolo Bonzini<pbonzini@redhat.com>
Signed-off-by: Emanuele Giuseppe Esposito<eesposit@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy<vsementsov@virtuozzo.com>
  block/blkdebug.c | 17 ++++++++---------
  1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index e8fdf7b056..6bdeb2c7b3 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -74,6 +74,7 @@ enum {
typedef struct BlkdebugRule {
@@ -791,22 +792,22 @@ static void suspend_request(BlockDriverState *bs, 
BlkdebugRule *rule)
-static bool process_rule(BlockDriverState *bs, struct BlkdebugRule *rule,
-    bool injected)
+static void process_rule(BlockDriverState *bs, struct BlkdebugRule *rule,
+                         int *action_count)

I would have liked a comment above this function explaining that `action_count` is not merely an int pointer, but actually an int[ACTION__MAX] pointer.

But it’s too late to complain about that now. O:)

      BDRVBlkdebugState *s = bs->opaque;
/* Only process rules for the current state */
      if (rule->state && rule->state != s->state) {
-        return injected;
+        return;
/* Take the action */
+    action_count[rule->action]++;
      switch (rule->action) {
-        if (!injected) {
+        if (action_count[ACTION_INJECT_ERROR] == 1) {

(I don’t quite understand this part – why do we clear the list of active rules here?  And why only if a new error is injected?  For example, if I have an inject-error rule that should only fire on state 1, and then the state changes to state 2, it stays active until a new error is injected, which doesn’t make sense to me.  But that has nothing to do with this series, of course.  I’m just wondering.)


-            injected = true;
          QSIMPLEQ_INSERT_HEAD(&s->active_rules, rule, active_next);

reply via email to

[Prev in Thread] Current Thread [Next in Thread]