[Qemu-devel] [PATCH] qxl: Export "mode" as a property

[Qemu-devel] [PATCH] qxl: Export "mode" as a property

[Alexander Larsson]

This way you can easily tell from qemu if the guest is using
a qxl driver or not.
---
 hw/qxl.c | 1 +
 hw/qxl.h | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 1d16863..98af0bb 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -2261,6 +2261,7 @@ static Property qxl_properties[] = {
         DEFINE_PROP_UINT32("vram64_size_mb", PCIQXLDevice, vram_size_mb, -1),
         DEFINE_PROP_UINT32("vgamem_mb", PCIQXLDevice, vgamem_size_mb, 16),
         DEFINE_PROP_INT32("surfaces", PCIQXLDevice, ssd.num_surfaces, 1024),
+        DEFINE_PROP_UINT32("mode", PCIQXLDevice, mode, 0),
         DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/qxl.h b/hw/qxl.h
index e583cfb..c4414ba 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -34,7 +34,7 @@ typedef struct PCIQXLDevice {
 
     uint32_t           guest_bug;
 
-    enum qxl_mode      mode;
+    uint32_t           mode;
     uint32_t           cmdflags;
     int                generation;
     uint32_t           revision;
-- 
1.7.12.1

Re: [Qemu-devel] [PATCH] qxl: Export "mode" as a property

[Gerd Hoffmann]

On 11/19/12 14:11, Alexander Larsson wrote:
> This way you can easily tell from qemu if the guest is using
> a qxl driver or not.

Device properties are for configuration, not inspection.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] qxl: Export "mode" as a property

[Daniel P. Berrange]

On Tue, Nov 20, 2012 at 08:50:09AM +0100, Gerd Hoffmann wrote:
> On 11/19/12 14:11, Alexander Larsson wrote:
> > This way you can easily tell from qemu if the guest is using
> > a qxl driver or not.
> 
> Device properties are for configuration, not inspection.

Sounds like we need to expose something via the monitor to let admin
apps discover if QXL has been enabled with SPICE vs plain VGA compat
mode only.  This is useful for apps to know, so that they can tell
the user that something is wrong performance wise, if the OS is
expected to have QXL present but does not.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH] qxl: Export "mode" as a property

[Gerd Hoffmann]

On 11/20/12 10:07, Daniel P. Berrange wrote:
> On Tue, Nov 20, 2012 at 08:50:09AM +0100, Gerd Hoffmann wrote:
>> On 11/19/12 14:11, Alexander Larsson wrote:
>>> This way you can easily tell from qemu if the guest is using
>>> a qxl driver or not.
>>
>> Device properties are for configuration, not inspection.
> 
> Sounds like we need to expose something via the monitor to let admin
> apps discover if QXL has been enabled with SPICE vs plain VGA compat
> mode only.  This is useful for apps to know, so that they can tell
> the user that something is wrong performance wise, if the OS is
> expected to have QXL present but does not.

Adding this to "info spice" is an option.  This has been discussed in
the past.  IIRC we didn't actually implement it as there was not clear
whenever this is actually needed or whenever management will handle this
using the guest agent anyway.  Alon?

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] qxl: Export "mode" as a property

[Alexander Larsson]

On tis, 2012-11-20 at 10:15 +0100, Gerd Hoffmann wrote:
> On 11/20/12 10:07, Daniel P. Berrange wrote:
> > On Tue, Nov 20, 2012 at 08:50:09AM +0100, Gerd Hoffmann wrote:
> >> On 11/19/12 14:11, Alexander Larsson wrote:
> >>> This way you can easily tell from qemu if the guest is using
> >>> a qxl driver or not.
> >>
> >> Device properties are for configuration, not inspection.
> > 
> > Sounds like we need to expose something via the monitor to let admin
> > apps discover if QXL has been enabled with SPICE vs plain VGA compat
> > mode only.  This is useful for apps to know, so that they can tell
> > the user that something is wrong performance wise, if the OS is
> > expected to have QXL present but does not.
> 
> Adding this to "info spice" is an option.  This has been discussed in
> the past.  IIRC we didn't actually implement it as there was not clear
> whenever this is actually needed or whenever management will handle this
> using the guest agent anyway.  Alon?

I'm working on a kind of "troubleshooting log" option for gnome-boxes
which wants to record all these kinds of data so that a helpdesk or a
developer can help figuring out a problem with a guest. Knowing if a qxl
driver is in use is an important piece of information. Additionally it
would be very nice if the driver could report a version string that I
could query (same for virtio driver, etc).

Re: [Qemu-devel] [PATCH] qxl: Export "mode" as a property

[Gerd Hoffmann]

  Hi,

> I'm working on a kind of "troubleshooting log" option for gnome-boxes
> which wants to record all these kinds of data so that a helpdesk or a
> developer can help figuring out a problem with a guest.

Understood.

> Knowing if a qxl
> driver is in use is an important piece of information. Additionally it
> would be very nice if the driver could report a version string that I
> could query (same for virtio driver, etc).

Guest driver version information isn't available.

For QXL we could track which features are in use by the driver (i.e.
whenever it uses sync/async io ports, ...) which might be good enougth
for trouble shooting.

For virtio it should be likewise easy to figure which device features
the guest has activated.

If you also wanna have this for non-qxl devices (which makes sense
indeed) going the "info spice" route doesn't make that much sense
though.  It is probably more useful to add an (optional) "report device
state" callback to devices which then returns this info in some useful
format (qdict?), then wind this up as monitor command.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] qxl: Export "mode" as a property

[Alexander Larsson]

On tis, 2012-11-20 at 11:17 +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > I'm working on a kind of "troubleshooting log" option for gnome-boxes
> > which wants to record all these kinds of data so that a helpdesk or a
> > developer can help figuring out a problem with a guest.
> 
> Understood.
> 
> > Knowing if a qxl
> > driver is in use is an important piece of information. Additionally it
> > would be very nice if the driver could report a version string that I
> > could query (same for virtio driver, etc).
> 
> Guest driver version information isn't available.

I know that, but it would be possible to add code to the driver to
register a version string.

> For QXL we could track which features are in use by the driver (i.e.
> whenever it uses sync/async io ports, ...) which might be good enougth
> for trouble shooting.

Might be useful for qxl developers, but not very useful to e.g. a
helpdesk employee trying to solve an issue for a user, as its kinda
low-level. Having the version string would be very nice here.

> For virtio it should be likewise easy to figure which device features
> the guest has activated.
> 
> If you also wanna have this for non-qxl devices (which makes sense
> indeed) going the "info spice" route doesn't make that much sense
> though.  It is probably more useful to add an (optional) "report device
> state" callback to devices which then returns this info in some useful
> format (qdict?), then wind this up as monitor command.

Sure, that would work too, as long as i can enumerate the devices and
get the data in a machine parsable format. Properties are already that,
although I realize that mixing configuration and monitoring in the same
set might be problematic.

Re: [Qemu-devel] [PATCH] qxl: Export "mode" as a property

[Gerd Hoffmann]

  Hi,

> I know that, but it would be possible to add code to the driver to
> register a version string.

Sure.  Although it is a bit more complex that just a simple string.
With qxl guest drivers moving to kms as we'll have two driver components
then.

> Might be useful for qxl developers, but not very useful to e.g. a
> helpdesk employee trying to solve an issue for a user, as its kinda
> low-level.

Depends on the problem.  $feature not working and qxl seeing the driver
not using the new ops for $feature is helpful.

> Having the version string would be very nice here.

Indeed, being able to match the driver version against a database of
known-buggy versions is nice too.  That will only work for new drivers
which actually report the version though (assuming we go that route).

But there we are again at the level where going through the guest agent
might be a better way.  "rpm -q kernel xorg-x11-drv-qxl" gives you what
you need ...

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] qxl: Export "mode" as a property

[Alexander Larsson]

> But there we are again at the level where going through the guest agent
> might be a better way.  "rpm -q kernel xorg-x11-drv-qxl" gives you what
> you need ...

Well, that relies on the agent working, and requires a lot of
per-guest-os type of probing for the driver, as well as the fact that
the driver being installed doesn't necessarily mean is in use.

Re: [Qemu-devel] [PATCH] qxl: Export "mode" as a property

[Alon Levy]

----- Original Message -----
> On 11/20/12 10:07, Daniel P. Berrange wrote:
> > On Tue, Nov 20, 2012 at 08:50:09AM +0100, Gerd Hoffmann wrote:
> >> On 11/19/12 14:11, Alexander Larsson wrote:
> >>> This way you can easily tell from qemu if the guest is using
> >>> a qxl driver or not.
> >>
> >> Device properties are for configuration, not inspection.
> > 
> > Sounds like we need to expose something via the monitor to let
> > admin
> > apps discover if QXL has been enabled with SPICE vs plain VGA
> > compat
> > mode only.  This is useful for apps to know, so that they can tell
> > the user that something is wrong performance wise, if the OS is
> > expected to have QXL present but does not.
> 
> Adding this to "info spice" is an option.  This has been discussed in
> the past.  IIRC we didn't actually implement it as there was not
> clear
> whenever this is actually needed or whenever management will handle
> this
> using the guest agent anyway.  Alon?

The last patchset got derailed because it was big and not clear if it was required. As long as info spice is already qmp'ed then I think adding mode there makes perfect sense. A guest agent could provide the same information, but since qemu already has it it saves a round trip to the guest. And management already needs to handle both information from the guest and from the vm, so this isn't complicating management either.

> 
> cheers,
>   Gerd
> 
> 

Re: [Qemu-devel] [PATCH] qxl: Export "mode" as a property

[Markus Armbruster]

[Cc'ing Anthony because this could be of general QOM interest]

Gerd Hoffmann <kraxel@redhat.com> writes:

> On 11/19/12 14:11, Alexander Larsson wrote:
>> This way you can easily tell from qemu if the guest is using
>> a qxl driver or not.
>
> Device properties are for configuration, not inspection.

Read-only properties could serve fine as generic inspection mechanism.

Re: [Qemu-devel] [PATCH] qxl: Export "mode" as a property

[Alexander Larsson]

On tis, 2012-11-20 at 10:48 +0100, Markus Armbruster wrote:
> [Cc'ing Anthony because this could be of general QOM interest]
> 
> Gerd Hoffmann <kraxel@redhat.com> writes:
> 
> > On 11/19/12 14:11, Alexander Larsson wrote:
> >> This way you can easily tell from qemu if the guest is using
> >> a qxl driver or not.
> >
> > Device properties are for configuration, not inspection.
> 
> Read-only properties could serve fine as generic inspection mechanism.

Yeah, it strikes me as more natural than a non-generic extension like
"info spice".

Re: [Qemu-devel] [PATCH] qxl: Export "mode" as a property

[Gerd Hoffmann]

On 11/20/12 10:48, Markus Armbruster wrote:
> [Cc'ing Anthony because this could be of general QOM interest]
> 
> Gerd Hoffmann <kraxel@redhat.com> writes:
> 
>> On 11/19/12 14:11, Alexander Larsson wrote:
>>> This way you can easily tell from qemu if the guest is using
>>> a qxl driver or not.
>>
>> Device properties are for configuration, not inspection.
> 
> Read-only properties could serve fine as generic inspection mechanism.

We don't have read-only properties, at least not in the qdev world.

Dunno whenever QOM is different and whenever there is some way to query
QOM properties via monitor.  All those funky qom monitor commands seem
not to be in master yet ...

cheers,
  Gerd