qemu-devel
[Top][All Lists]
Advanced

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

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


From: John Snow
Subject: Re: [PATCH v5 7/7] scripts/ghes_inject: add a script to generate GHES error inject
Date: Fri, 9 Aug 2024 15:26:41 -0400



On Fri, Aug 9, 2024, 4:24 AM Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:
Em Thu, 8 Aug 2024 19:33:32 -0400
John Snow <jsnow@redhat.com> escreveu:

> > > 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.
> > 
>
> I might *suggest* you try to use the exception-raising interface and catch
> exceptions to interrogate expected errors as it aligns better with the
> "idiomatic python API" - I have no plans to support an external API that
> *returns* error objects except via the exception class. This approach will
> be easier to port when I drop the legacy interface in the future, see below.
>
> But, that said, whichever is easiest. We use all three interfaces in many
> places in the QEMU tree. I have no grounds to require you to use a specific
> one ;)

While a python-style exception handling is cool, I ended opting to use
cmd_obj(), as the script needs to catch the end of /machine/unattached/device[]
array, and using cmd_obj() made the conversion easier.

One of the things I missed at the documentation is a description of the
possible exceptions that cmd() could raise.

It is probably worth documenting it and placing them on a QMP-specific
error class, but a change like that would probably be incompatible with
the existing applications. Probably something to be considered on your
TODO list to move this from legacy ;-)

Good feedback, thanks! I definitely didn't spend much time polishing the "legacy" interface. I clearly thought it'd be more temporary than it became ;)

I owe the package some updates for 3.13, I'll improve the documentation and also consider adding some "you forgot to make the address a tuple" protection so that part is less of a trap. (Without the tuple, I think it likely used the address as a socket path and the port as a bool to enter server mode. mypy would catch this, but it's a design goal to not require or expect script writers to need such things.) 

Thank you! :)


Anyway, I already folded the changes at the branch I'll be using as basis
for the next submission (be careful to use it, as I'm always rebasing it):

Great, I'll review the entire script more thoroughly on v2, if that's OK with you.

Just got back from a long PTO and an illness and I'm still ramping back up and handling backlog.


~~js

reply via email to

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