Before, event_match didn't always recurse if the event value was not a
dictionary, and would instead check for equality immediately.

By delaying equality checking to post-recursion, we can allow leaf
values like "5" to match "None" and take advantage of the generic
None-returns-True clause.

This makes the matching a little more obviously consistent at the
expense of being able to check for explicit None values, which is
probably not that important given what this function is used for.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/qemu/__init__.py | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/python/qemu/__init__.py b/python/qemu/__init__.py
index 98ed8a2e28..77d45f88fe 100644
--- a/python/qemu/__init__.py
+++ b/python/qemu/__init__.py
@@ -409,27 +409,30 @@ class QEMUMachine(object):
 
         The match criteria takes the form of a matching subdict. The event is
         checked to be a superset of the subdict, recursively, with matching
-        values whenever those values are not None.
+        values whenever the subdict values are not None.
+
+        This has a limitation that you cannot explicitly check for None values.
 
         Examples, with the subdict queries on the left:
          - None matches any object.
          - {"foo": None} matches {"foo": {"bar": 1}}
-         - {"foo": {"baz": None}} does not match {"foo": {"bar": 1}}
-         - {"foo": {"baz": 2}} matches {"foo": {"bar": 1, "baz": 2}}
+         - {"foo": None} matches {"foo": 5}
+         - {"foo": {"abc": None}} does not match {"foo": {"bar": 1}}
+         - {"foo": {"rab": 2}} matches {"foo": {"bar": 1, "rab": 2}}
         """
         if match is None:
             return True
 
-        for key in match:
-            if key in event:
-                if isinstance(event[key], dict):
-                    if not QEMUMachine.event_match(event[key], match[key]):
-                        return False
-                elif event[key] != match[key]:
+        try:
+            for key in match:
+                if key in event:
+                    return QEMUMachine.event_match(event[key], match[key])
+                else:
                     return False
-            else:
-                return False
-        return True
+            return True
+        except TypeError:
+            # either match or event wasn't iterable (not a dict)
+            return match == event
 
     def event_wait(self, name, timeout=60.0, match=None):
         """
-- 
2.20.1

On 24.05.19 20:02, John Snow wrote:
> Before, event_match didn't always recurse if the event value was not a
> dictionary, and would instead check for equality immediately.
> 
> By delaying equality checking to post-recursion, we can allow leaf
> values like "5" to match "None" and take advantage of the generic
> None-returns-True clause.
> 
> This makes the matching a little more obviously consistent at the
> expense of being able to check for explicit None values, which is
> probably not that important given what this function is used for.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  python/qemu/__init__.py | 27 +++++++++++++++------------
>  1 file changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/python/qemu/__init__.py b/python/qemu/__init__.py
> index 98ed8a2e28..77d45f88fe 100644
> --- a/python/qemu/__init__.py
> +++ b/python/qemu/__init__.py
> @@ -409,27 +409,30 @@ class QEMUMachine(object):
>  
>          The match criteria takes the form of a matching subdict. The event is
>          checked to be a superset of the subdict, recursively, with matching
> -        values whenever those values are not None.
> +        values whenever the subdict values are not None.
> +
> +        This has a limitation that you cannot explicitly check for None values.
>  
>          Examples, with the subdict queries on the left:
>           - None matches any object.
>           - {"foo": None} matches {"foo": {"bar": 1}}
> -         - {"foo": {"baz": None}} does not match {"foo": {"bar": 1}}
> -         - {"foo": {"baz": 2}} matches {"foo": {"bar": 1, "baz": 2}}
> +         - {"foo": None} matches {"foo": 5}
> +         - {"foo": {"abc": None}} does not match {"foo": {"bar": 1}}
> +         - {"foo": {"rab": 2}} matches {"foo": {"bar": 1, "rab": 2}}
>          """
>          if match is None:
>              return True
>  
> -        for key in match:
> -            if key in event:
> -                if isinstance(event[key], dict):
> -                    if not QEMUMachine.event_match(event[key], match[key]):
> -                        return False
> -                elif event[key] != match[key]:
> +        try:
> +            for key in match:
> +                if key in event:
> +                    return QEMUMachine.event_match(event[key], match[key])

With this change, we only check a single key that is both in @match and
@event.  I think we want to keep the "if not -- return False" pattern,
don’t we?

Max

> +                else:
>                      return False
> -            else:
> -                return False
> -        return True
> +            return True
> +        except TypeError:
> +            # either match or event wasn't iterable (not a dict)
> +            return match == event
>  
>      def event_wait(self, name, timeout=60.0, match=None):
>          """
> 

On 5/27/19 9:24 AM, Max Reitz wrote:
> On 24.05.19 20:02, John Snow wrote:
>> Before, event_match didn't always recurse if the event value was not a
>> dictionary, and would instead check for equality immediately.
>>
>> By delaying equality checking to post-recursion, we can allow leaf
>> values like "5" to match "None" and take advantage of the generic
>> None-returns-True clause.
>>
>> This makes the matching a little more obviously consistent at the
>> expense of being able to check for explicit None values, which is
>> probably not that important given what this function is used for.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>  python/qemu/__init__.py | 27 +++++++++++++++------------
>>  1 file changed, 15 insertions(+), 12 deletions(-)
>>
>> diff --git a/python/qemu/__init__.py b/python/qemu/__init__.py
>> index 98ed8a2e28..77d45f88fe 100644
>> --- a/python/qemu/__init__.py
>> +++ b/python/qemu/__init__.py
>> @@ -409,27 +409,30 @@ class QEMUMachine(object):
>>  
>>          The match criteria takes the form of a matching subdict. The event is
>>          checked to be a superset of the subdict, recursively, with matching
>> -        values whenever those values are not None.
>> +        values whenever the subdict values are not None.
>> +
>> +        This has a limitation that you cannot explicitly check for None values.
>>  
>>          Examples, with the subdict queries on the left:
>>           - None matches any object.
>>           - {"foo": None} matches {"foo": {"bar": 1}}
>> -         - {"foo": {"baz": None}} does not match {"foo": {"bar": 1}}
>> -         - {"foo": {"baz": 2}} matches {"foo": {"bar": 1, "baz": 2}}
>> +         - {"foo": None} matches {"foo": 5}
>> +         - {"foo": {"abc": None}} does not match {"foo": {"bar": 1}}
>> +         - {"foo": {"rab": 2}} matches {"foo": {"bar": 1, "rab": 2}}
>>          """
>>          if match is None:
>>              return True
>>  
>> -        for key in match:
>> -            if key in event:
>> -                if isinstance(event[key], dict):
>> -                    if not QEMUMachine.event_match(event[key], match[key]):
>> -                        return False
>> -                elif event[key] != match[key]:
>> +        try:
>> +            for key in match:
>> +                if key in event:
>> +                    return QEMUMachine.event_match(event[key], match[key])
> 
> With this change, we only check a single key that is both in @match and
> @event.  I think we want to keep the "if not -- return False" pattern,
> don’t we?
> 
> Max
> 

Whoops, way too fast on the send button. Sorry about that.