Re: [Qemu-devel] [PATCH] add "info ioapic" monitor command

Re: [Qemu-devel] [PATCH] add "info ioapic" monitor command

[Gleb Natapov]

On Thu, Dec 31, 2009 at 12:57:44PM -0600, Anthony Liguori wrote:
> On 12/31/2009 09:33 AM, Gleb Natapov wrote:
> >On Thu, Dec 31, 2009 at 08:15:29AM -0600, Anthony Liguori wrote:
> >>On 12/30/2009 06:16 PM, Gleb Natapov wrote:
> >>>It helps debug problems and it is not intrusive. Since 0.12 will be used
> >>>for a long time I want to add it there for convenience.
> >>
> >>stable is for bug fixes only.
> >>
> >This is to close minded. Not able to debug hard to reproduce problems when they appear
> >at the environment you can't replicate is a bug.
> 
> 
> Every feature is important to someone and there is at least someone
> who wants just about every feature backported to stable.
> 
Agree and that is why practical judgment is needed. Humans, not binary
algorithm decides what to port and what not too.

> Looking at this patch, it adds a new monitor command which
> potentially means that the QMP protocol is changing (as there is now
> a new command).  This is definitely not something we want to do in
> stable.
> 
Does QMP expose list of available command through QMP protocol? Because if
it is not, there is no such issues as you describe at all, but event if it
is isn't the goal of QMP to make handling of such cases transparent for
management software. There will be one more element returned in command
list that is all.

--
			Gleb.

Re: [Qemu-devel] [PATCH] add "info ioapic" monitor command

[Gleb Natapov]

On Thu, Dec 31, 2009 at 01:01:50PM -0600, Anthony Liguori wrote:
> On 12/31/2009 09:39 AM, Gleb Natapov wrote:
> >>The common case is where you just want to see the state of the
> >>device. We support hundreds of devices.  We can have one code path
> >>that displays 95% of them with a couple devices that have exceptions
> >>or we could have hundreds of these commands that aren't consistent
> >>because they're all open-coded.
> >I am not against having common code path that prints out state of all
> >devices. It just different from what I am trying to achieve with my patch.
> >My patch adds parsing and pretty-printing of device state and that's the
> >part that is not addressed with your common code scenario.
> 
> A common device printing mechanism is probably no more code than
> your current patch.
> 
Have you looked at my patch at all? What part of it can be made generic
in your opinion? All it does is transforms ioapic state to human
readable way (and that requires device internals kwoulage) and pot it
into QObject form.
 
> My position is that if that generic mechanism is not good enough for
> you, then we should look at fixing that problem instead of
> introducing a completely redundant mechanism.
> 
Generic mechanism is by definition cannot decode device state with
knowledge of device specific details.

> Heck, even if you had a generic mechanism and then a big if in the
> middle of it that treated ioapic better than other devices, it would
> be 100x better than introducing a 1-off command.
> 
The 'if' you are talking about (which is crazy by the way 'if' for each
device in a common code?) will call the function my patch adds.

--
			Gleb.