Re: [PATCH v5 7/7] scripts/ghes_inject: add a script to generate GHES error inject

[Mauro Carvalho Chehab]

Em Thu, 8 Aug 2024 17:21:33 -0400
John Snow <jsnow@xxxxxxxxxx> escreveu:

> On Fri, Aug 2, 2024 at 5:44 PM Mauro Carvalho Chehab <
> mchehab+huawei@xxxxxxxxxx> wrote:  
> 

> > diff --git a/scripts/qmp_helper.py b/scripts/qmp_helper.py
> > new file mode 100644
> > index 000000000000..13fae7a7af0e
> > --- /dev/null
> > +++ b/scripts/qmp_helper.py
> >  
> 
> I'm going to admit I only glanced at this very briefly, but -- is there a
> chance you could use qemu.git/python/qemu/qmp instead of writing your own
> helpers here?
> 
> If *NOT*, is there something that I need to add to our QMP library to
> facilitate your script?

I started writing this script to be hosted outside qemu tree, when
we had a very different API.

I noticed later about the QMP, and even tried to write a patch for it,
but I gave up due to asyncio complexity...

Please notice that, on this file, I actually placed three classes:

- qmp
- util
- cper_guid

I could probably make the first one to be an override of QEMUMonitorProtocol 
(besides normal open/close/cmd communication, it also contains some
methods that are specific to error inject use case:

- to generate a CPER record;
- to search for data via qom-get.

The other two classes are just common code used by ghes_inject commands.
My idea is to have multiple commands to do different kinds of GHES
error injection, each command on a different file/class.

> > +    s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
> > +    try:
> > +        s.connect((host, port))
> > +    except ConnectionRefusedError:
> > +        sys.exit(f"Can't connect to QMP host {host}:{port}")
> >  
> 
> You should be able to use e.g.
> 
> legacy.py's QEMUMonitorProtocol class for synchronous connections, e.g.
> 
> from qemu.qmp.legacy import QEMUMonitorProtocol
> 
> qmp = QEMUMonitorProtocol((host, port))
> qmp.connect(negotiate=True)

That sounds interesting! I give it a try.

> If you want to run the script w/o setting up a virtual environment or
> installing the package, take a look at the hacks in scripts/qmp/ for how I
> support e.g. qom-get directly from the source tree.

Yeah, I saw that already. Doing: 

	sys.path.append(path.join(qemu_dir, 'python'))

the same way qom-get does should do the trick.

> > +
> > +    data = s.recv(1024)
> > +    try:
> > +        obj = json.loads(data.decode("utf-8"))
> > +    except json.JSONDecodeError as e:
> > +        print(f"Invalid QMP answer: {e}")
> > +        s.close()
> > +        return
> > +
> > +    if "QMP" not in obj:
> > +        print(f"Invalid QMP answer: {data.decode("utf-8")}")
> > +        s.close()
> > +        return
> > +
> > +    for i, command in enumerate(commands):
> >  
> 
> Then here you'd use qmp.cmd (raises exception on QMPError) or qmp.cmd_raw
> or qmp.cmd_obj (returns the QMP response as the return value even if it was
> an error.)

Good to know, I'll try and see what fits best.

> More details:
> https://qemu.readthedocs.io/projects/python-qemu-qmp/en/latest/qemu.qmp.legacy.html

I'll take a look. The name "legacy" is a little scary, as it might
imply that this has been deprecated. If there's no plans to deprecate,
then it would be great to use it and simplify the code a little bit.

> There's also an async version, but it doesn't look like you require that
> complexity, so you can ignore it.

Yes, that's the case: a serialized sync send/response logic works perfectly
for this script. No need to be burden with asyncio complexity.

Thanks,
Mauro