On Tue, Dec 29, 2009 at 03:39:29PM -0200, Luiz Capitulino wrote:
> On Tue, 29 Dec 2009 18:53:44 +0200
> Gleb Natapov <g...@redhat.com> wrote:
>
> > On Tue, Dec 29, 2009 at 02:37:20PM -0200, Luiz Capitulino wrote:
> > > On Thu, 24 Dec 2009 17:02:42 +0200
> > > Gleb Natapov <g...@redhat.com> wrote:
> > >
> > > > +
> > > > +static const char *delivery_mode_string[] = {"fixed", "lowprio",
> > > > "smi", "res",
> > > > + "nmi", "init", "res",
> > > > "extint"};
> > > > +
> > > > +void do_info_ioapic(Monitor *mon)
> > > > +{
> > > > + int i;
> > > > +
> > > > + if (!ioapic)
> > > > + return;
> > > > + for (i = 0; i < IOAPIC_NUM_PINS; i++) {
> > > > + uint64 e = ioapic->ioredtbl[i];
> > > > + monitor_printf(mon, "%2d: ", i);
> > > > + if (e & IOAPIC_LVT_MASKED) {
> > > > + monitor_printf(mon, "masked\n");
> > > > + } else {
> > > > + uint8_t vec = e & 0xff;
> > > > + uint8_t trig_mode = ((e >> 15) & 1);
> > > > + uint8_t dest = e >> 56;
> > > > + uint8_t dest_mode = (e >> 11) & 1;
> > > > + uint8_t delivery_mode = (e >> 8) & 7;
> > > > + uint8_t polarity = (e >> 13) & 1;
> > > > + monitor_printf(mon, "vec=%3d %s %s acive-%s %s dest=%d\n",
> > > > + vec,
> > > > + delivery_mode_string[delivery_mode],
> > > > + dest_mode ? "logical":"physical",
> > > > + polarity ? "low" : "high",
> > > > + trig_mode ? "level": "edge",
> > > > + dest);
> > > > + }
> > > > + }
> > > > +}
> > >
> > > New Monitor handlers should return QObjects, monitor_printf()
> > > can only be used in the print handler.
> > >
> > So I want to add only print handler. This is debug info only, no
> > management should ever care about it.
>
> Well, this is possible (at least for now) but there are plans to
> have an external shell.
>
> Also, I don't want people to avoid writing handlers because
> they feel it's difficult.
>
> > > You can take a look at qemu_chr_info() for an example, as it does
> > > what I think you should do here: build a qdict for each pin and
> > > return a qlist or return another qdict if it makes sense (or will
> > > make in the future) to have more the one ioapic.
> > I don't understand a single word from what you are saying :(, but
> > qemu_chr_info() looks scary. Actually I don't understand what it does at
> > all.
>
> Ouch. :((
>
> > > I'm still thinking in ways to make the work of writing the new
> > > Monitor handlers easier..
> > Something more simple is definitely needed. If I need to pars the data
> > structure to some intermediate language and then parse it back again just
> > to add
> > debug command I will not event consider adding it. One more tracepoint
> > in the kernel will take 10min to add and will accomplish the same goal
> > to me albeit in less elegant way.
>
> Actually, you're building objects and iterating over them to get them
> printed, it's not parsing and it's common to do that in high level
> languages.
>
> But I agree that it's not as easy as calling monitor_printf().
>
> Something that is on my TODO list is to add a default printing
> function. This will make things easier a bit, but you'll have to
> build the objects and the user output won't be pretty.
>
> We could consider allowing monitor_printf() in certain handlers,
> but as I said above this has a number of problems and doing this
> because writing handlers became harder only hides the real
> problem (which will come up again, soon or later).
>
> So, we'll have to wait for people to come back from vacations
> to discuss this issue.
Maybe we need a "debug" monitor command.
That would just print a string, no objects,
and no guarantees for management about format.
--
MST
