[Qemu-devel] Re: [PATCH, RFC 0/4] monitor device info infrastructure

[Blue Swirl]

On 5/13/10, Jan Kiszka <address@hidden> wrote:
> Blue Swirl wrote:
>  > On 5/13/10, Jan Kiszka <address@hidden> wrote:
>  >> Blue Swirl wrote:
>  >>  > Hi all,
>  >>  >
>  >>  > I finally refreshed some of my monitor patches. PCI and HPET patches
>  >>  > (attached, they don't apply anymore) need more work because of the new
>  >>  > monitor design.
>  >>  >
>  >>  > The patches provide a method for devices to register new monitor
>  >>  > commands. This fixes some design problems, like useless
>  >>  > pic_info/irq_info functions for most architectures.
>  >>  >
>  >>  > I think automation via qdev field was requested the last time. I added
>  >>  > something like this, though it doesn't fit the cases where several
>  >>  > functions need to be registered.
>  >>  >
>  >>  > Comments?
>  >>
>  >>
>  >> I'll soon send out a series that adds "device_show <qdev-path>" to
>  >>  visualize the full state, something that will at least obsolete "info
>  >>  pic" and "info hpet" sooner or later, maybe even more. Also, I would
>  >>  like to qdev'ify CPUs in order to make them reachable for device_show
>  >>  (which evaluates qdev->info.vmstate).
>  >
>  > That sounds like much better design. But for example 'info pci' shows
>  > different things compared to 'info qdev'. And what about 'info
>  > usbhost', there is no qdev?
>
>
> That should be addressed differently (if there is a need to change it).
>  My point is basically about things that are (or will be) qdev related -
>  just as dev_info was suggesting.
>
>
>  >
>  >>  I'm not sure: How many use cases for a "dev_info" would remain?
>  >>  Moreover, to dump statistics, we should rather add some "device_stats
>  >>  <qdev-path>" command instead of mixing both usages under an info command.
>  >
>  > Not too many. I think the VMState structure (with no connection to
>  > savevm/loadvm/migration) would be useful to define and dump also
>  > statistics. But because most statistics need to be enabled at compile
>  > phase, they are probably not very widely used.
>
>
> If they are special, then I would vote for a separate stats
>  infrastructure with its own data types where required. If certain stats
>  should rather be enabled by default, then there is actually the question
>  if they shouldn't be migrated as well, thus become part of the related
>  VMState definitions. Just thoughts, I haven't done any investigations on
>  the current situation.

Currently for example PIC statistics (like i8259.c and
slavio_intctl.c) are not saved or migrated. Statistics generation need
a specially compiled QEMU, so I don't think statistics saving or
migration will be any normal use case.

>  However, I'm a fan of a plug-in interface for the existing info monitor
>  command. That would allow to register things dynamically that do not fit
>  in any regular structure without requiring stubs or tons of #ifdefs.
>  What about this as a first step?

I think qdev dump approach would be better for current and future qdev
objects. But I'll try if my version makes non-qdev stuff ('info
usbhost' or something) simpler.

The main benefit with the registration is IMHO that the caller can
specify a state variable when registering and this variable is also
available at callback time. This allows many cleanups.