Re: [PATCH 18/20] python/qemu/qmp.py: re-raise OSError when encountered

[John Snow <jsnow@redhat.com>]

On 10/7/20 3:17 PM, John Snow wrote:
> On 10/7/20 7:30 AM, Kevin Wolf wrote:
>> Am 07.10.2020 um 01:58 hat John Snow geschrieben:
>>> Nested if conditions don't change when the exception block fires; we
>>> need to explicitly re-raise the error if we didn't intend to capture and
>>> suppress it.
>>>
>>> Signed-off-by: John Snow <jsnow@redhat.com>
>>> ---
>>>   python/qemu/qmp.py | 6 +++---
>>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/python/qemu/qmp.py b/python/qemu/qmp.py
>>> index d911999da1f..bdbd1e9bdbb 100644
>>> --- a/python/qemu/qmp.py
>>> +++ b/python/qemu/qmp.py
>>> @@ -169,9 +169,9 @@ def __get_events(self, wait: Union[bool, float] = 
>>> False) -> None:
>>>           try:
>>>               self.__json_read()
>>>           except OSError as err:
>>> -            if err.errno == errno.EAGAIN:
>>> -                # No data available
>>> -                pass
>>> +            # EAGAIN: No data available; not critical
>>> +            if err.errno != errno.EAGAIN:
>>> +                raise
>>
>> Hm, if we re-raise the exception here, the socket remains non-blocking.
>> I think we intended to have it like that only temporarily.
>>
> 
> Whoops. Yep, no good to go from one kind of broken to a different kind 
> of broken.
> 

Actually, wanna know a funny story?

I think the reason this never broke anything is because sockfiles aren't 
suppose to be used in nonblocking mode -- their behavior is not 
documented in this case.

In practice, what actually seems to happen when you set non-blocking 
mode and then call sockfile.readline() is that you get an empty string.

This means you get 'None' from __json_read, and so on.

Why doesn't this bite us more often? Well, we don't actually check the 
return value when we're using this in non-blocking mode, so I suppose 
that's the primary reason.

I don't know if the behavior of Python changed at some point, I can try 
to patch this up for how Python *seems* to work, but we should probably 
do a more meaningful fix to get away from undefined behavior sometime soon.

I had some async prototypes hanging around, maybe it's time to try and 
give that a more solid effort ...



Related note:

Even in blocking mode, you might get an empty reply from readline which 
means EOF and not just "try again."

You'll see this in the case where you already have QEMU running from a 
previous failed test, and you start a new iotest up. When QMP calls 
accept(), the QMP capabilities handshake fails because it gets "None" 
from __json_read.

Confusing error for what's actually going on there. It's actually that 
the socket is at EOF.