[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH v5 39/52] i386/xen: add monitor commands to test event in
From: |
David Woodhouse |
Subject: |
Re: [RFC PATCH v5 39/52] i386/xen: add monitor commands to test event injection |
Date: |
Mon, 09 Jan 2023 19:49:23 +0000 |
User-agent: |
Evolution 3.44.4-0ubuntu1 |
On Mon, 2023-01-09 at 18:51 +0000, Dr. David Alan Gilbert wrote:
> * David Woodhouse (dwmw2@infradead.org) wrote:
> >
> > Thanks. Taking option 'a' then...
>
> Yes that's mostly right, some minor comments on it:
>
> > +EvtchnInfoList *qmp_xen_event_list(Error **errp)
> > +{
> > + error_setg(errp, "Xen event channel emulation not enabled\n");
>
> No \n on error_setg (and below)
Bah, thought I'd caught all those... er, in fact I *had* caught those
but just sent the penultimate version. Definitely fixed now.
> > +void qmp_xen_event_inject(uint32_t port, Error **errp)
> > +{
> > + XenEvtchnState *s = xen_evtchn_singleton;
> > +
> > + if (!s) {
> > + error_setg(errp, "Xen event channel emulation not enabled");
>
> return; ?
>
> > + }
> > +
> > + if (!valid_port(port)) {
> > + error_setg(errp, "Invalid port %u", port);
>
> and there?
Fixed now; thanks.
> > + }
> > +
> > + QEMU_LOCK_GUARD(&s->port_lock);
> > +
> > + if (set_port_pending(s, port)) {
> > + error_setg(errp, "Failed to set port %d", port);
>
> %u ?
Ack.
>
> > +##
> > +# @EvtchnInfo:
> > +#
> > +# Information about a Xen event channel port
> > +#
> > +# @port: the port number
> > +#
> > +# @type: the port type
> > +#
> > +# @remote-domain: remote domain for interdomain ports
>
> Missed vcpu?
Ah yes. In fact I originally missed it out *completely* but then the
hmp bit didn't build when I tried to print it... :)
>
Should all be fixed in
https://git.infradead.org/users/dwmw2/qemu.git/commitdiff/5cf387fe307
which is part of the xenfv branch there; I'll repost soonish after
fixing a MemoryRegion refcount leak in kvm_xen_get_vcpu_info_hva().
It might even be posted without the [RFC] tag, at *least* as far as the
parts I've been posting already. We're still working on the PV backend
driver stuff which comes in the next phase.
Thanks again for the feedback.
smime.p7s
Description: S/MIME cryptographic signature