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<pbonz...@redhat.com>
Signed-off-by: Emanuele Giuseppe Esposito<eespo...@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy<vsement...@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 {
      ACTION_INJECT_ERROR,
      ACTION_SET_STATE,
      ACTION_SUSPEND,
+    ACTION__MAX,
  };
typedef struct BlkdebugRule {
@@ -791,22 +792,22 @@ static void suspend_request(BlockDriverState *bs, 
BlkdebugRule *rule)
      qemu_coroutine_yield();
  }
-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) {
      case ACTION_INJECT_ERROR:
-        if (!injected) {
+        if (action_count[ACTION_INJECT_ERROR] == 1) {
              QSIMPLEQ_INIT(&s->active_rules);

(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.)

Max

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



Reply via email to