Patchwork [RFC,08/11] qdev: Add usb_bus_dev_info

login
register
mail settings
Submitter Nathan Baum
Date Dec. 26, 2009, 9:11 p.m.
Message ID <1261861899-1984-9-git-send-email-nathan@parenthephobia.org.uk>
Download mbox | patch
Permalink /patch/41822/
State New
Headers show

Comments

Nathan Baum - Dec. 26, 2009, 9:11 p.m.
Returns a QObject with information about a USB device.

Signed-off-by: Nathan Baum <nathan@parenthephobia.org.uk>
---
 hw/usb-bus.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)
Markus Armbruster - Jan. 15, 2010, 6:14 p.m.
Nathan Baum <nathan@parenthephobia.org.uk> writes:

> Returns a QObject with information about a USB device.
>
> Signed-off-by: Nathan Baum <nathan@parenthephobia.org.uk>
> ---
>  hw/usb-bus.c |   13 +++++++++++++
>  1 files changed, 13 insertions(+), 0 deletions(-)
>
> diff --git a/hw/usb-bus.c b/hw/usb-bus.c
> index 54027df..6d02807 100644
> --- a/hw/usb-bus.c
> +++ b/hw/usb-bus.c
> @@ -3,6 +3,7 @@
>  #include "qdev.h"
>  #include "sysemu.h"
>  #include "monitor.h"
> +#include "qjson.h"
>  
>  static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent);
>  
> @@ -232,6 +233,18 @@ static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent)
>                     dev->attached ? ", attached" : "");
>  }
>  
> +static QObject *usb_bus_dev_info(Monitor *mon, DeviceState *qdev)
> +{
> +    USBDevice *dev = DO_UPCAST(USBDevice, qdev, qdev);
> +    USBBus *bus = usb_bus_from_device(dev);
> +    return qobject_from_jsonf("{'busnr': %d, 'addr':%d, 'speed': %s, 'desc': %s, 'attached': %i}",
> +                              bus->busnr,

As for PCI, 'busnr' belongs to the bus, not the device.

Hmm, we don't have the infrastructure to return bus information, yet.
"info qtree" hardcodes printing of name and type.  Gerd, what do you
think?

> +                              dev->addr,
> +                              usb_speed(dev->speed),
> +                              dev->product_desc,
> +                              dev->attached);
> +}
> +
>  void usb_info(Monitor *mon)
>  {
>      USBBus *bus;
Nathan Baum - Jan. 15, 2010, 10:14 p.m.
On Fri, 2010-01-15 at 19:14 +0100, Markus Armbruster wrote:
> Nathan Baum <nathan@parenthephobia.org.uk> writes:
> 
> > Returns a QObject with information about a USB device.
> >
> > Signed-off-by: Nathan Baum <nathan@parenthephobia.org.uk>
> > ---
> >  hw/usb-bus.c |   13 +++++++++++++
> >  1 files changed, 13 insertions(+), 0 deletions(-)
> >
> > diff --git a/hw/usb-bus.c b/hw/usb-bus.c
> > index 54027df..6d02807 100644
> > --- a/hw/usb-bus.c
> > +++ b/hw/usb-bus.c
> > @@ -3,6 +3,7 @@
> >  #include "qdev.h"
> >  #include "sysemu.h"
> >  #include "monitor.h"
> > +#include "qjson.h"
> >  
> >  static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent);
> >  
> > @@ -232,6 +233,18 @@ static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent)
> >                     dev->attached ? ", attached" : "");
> >  }
> >  
> > +static QObject *usb_bus_dev_info(Monitor *mon, DeviceState *qdev)
> > +{
> > +    USBDevice *dev = DO_UPCAST(USBDevice, qdev, qdev);
> > +    USBBus *bus = usb_bus_from_device(dev);
> > +    return qobject_from_jsonf("{'busnr': %d, 'addr':%d, 'speed': %s, 'desc': %s, 'attached': %i}",
> > +                              bus->busnr,
> 
> As for PCI, 'busnr' belongs to the bus, not the device.

Hmm. In cases like this, is it appropriate to modify the output of the
existing "info qtree" when it is modified to use the QObject data?

Would it be sensible to go the (probably small amount of) effort to
change the print functions to print exactly they do now, and put changes
to their output in different patches so they can easily be dropped if
necessary?

> Hmm, we don't have the infrastructure to return bus information, yet.
> "info qtree" hardcodes printing of name and type.  Gerd, what do you
> think?
> 
> > +                              dev->addr,
> > +                              usb_speed(dev->speed),
> > +                              dev->product_desc,
> > +                              dev->attached);
> > +}
> > +
> >  void usb_info(Monitor *mon)
> >  {
> >      USBBus *bus;
Markus Armbruster - Jan. 18, 2010, 10:15 a.m.
Nathan Baum <nathan@parenthephobia.org.uk> writes:

> On Fri, 2010-01-15 at 19:14 +0100, Markus Armbruster wrote:
>> Nathan Baum <nathan@parenthephobia.org.uk> writes:
>> 
>> > Returns a QObject with information about a USB device.
>> >
>> > Signed-off-by: Nathan Baum <nathan@parenthephobia.org.uk>
>> > ---
>> >  hw/usb-bus.c |   13 +++++++++++++
>> >  1 files changed, 13 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/hw/usb-bus.c b/hw/usb-bus.c
>> > index 54027df..6d02807 100644
>> > --- a/hw/usb-bus.c
>> > +++ b/hw/usb-bus.c
>> > @@ -3,6 +3,7 @@
>> >  #include "qdev.h"
>> >  #include "sysemu.h"
>> >  #include "monitor.h"
>> > +#include "qjson.h"
>> >  
>> >  static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent);
>> >  
>> > @@ -232,6 +233,18 @@ static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent)
>> >                     dev->attached ? ", attached" : "");
>> >  }
>> >  
>> > +static QObject *usb_bus_dev_info(Monitor *mon, DeviceState *qdev)
>> > +{
>> > +    USBDevice *dev = DO_UPCAST(USBDevice, qdev, qdev);
>> > +    USBBus *bus = usb_bus_from_device(dev);
>> > +    return qobject_from_jsonf("{'busnr': %d, 'addr':%d, 'speed': %s, 'desc': %s, 'attached': %i}",
>> > +                              bus->busnr,
>> 
>> As for PCI, 'busnr' belongs to the bus, not the device.
>
> Hmm. In cases like this, is it appropriate to modify the output of the
> existing "info qtree" when it is modified to use the QObject data?
>
> Would it be sensible to go the (probably small amount of) effort to
> change the print functions to print exactly they do now, and put changes
> to their output in different patches so they can easily be dropped if
> necessary?

We might want to change info qtree output anyway if we show bus
information separately there...

>> Hmm, we don't have the infrastructure to return bus information, yet.
>> "info qtree" hardcodes printing of name and type.  Gerd, what do you
>> think?

... as I implied here.

Regardless, I think we should first decide what data we want to transmit
over QMP, and how to structure it, then figure out if and how to change
its human readable presentation.

[...]
Gerd Hoffmann - Jan. 18, 2010, 10:35 a.m.
On 01/18/10 11:15, Markus Armbruster wrote:
> Nathan Baum<nathan@parenthephobia.org.uk>  writes:
>
>>>> +static QObject *usb_bus_dev_info(Monitor *mon, DeviceState *qdev)
>>>> +{
>>>> +    USBDevice *dev = DO_UPCAST(USBDevice, qdev, qdev);
>>>> +    USBBus *bus = usb_bus_from_device(dev);
>>>> +    return qobject_from_jsonf("{'busnr': %d, 'addr':%d, 'speed': %s, 'desc': %s, 'attached': %i}",
>>>> +                              bus->busnr,
>>>
>>> As for PCI, 'busnr' belongs to the bus, not the device.

You want be able to figure which bus the device is attached to.

I think you actually can because the command returns the device tree 
converted into a qobject tree, correct?

Note: busnr is *not* fixed, it can be changed by the guest (maybe not 
for the primary, but surely for any secondary by writing to a pci bridge 
register).

>> Hmm. In cases like this, is it appropriate to modify the output of the
>> existing "info qtree" when it is modified to use the QObject data?
 >>
>> Would it be sensible to go the (probably small amount of) effort to
>> change the print functions to print exactly they do now, and put changes
>> to their output in different patches so they can easily be dropped if
>> necessary?
>
> We might want to change info qtree output anyway if we show bus
> information separately there...

Indeed.

>>> Hmm, we don't have the infrastructure to return bus information, yet.
>>> "info qtree" hardcodes printing of name and type.  Gerd, what do you
>>> think?

Adding a ->print_bus() function (or whatever the qmp aequivalent will 
be) to BusInfo is fine with me.

> Regardless, I think we should first decide what data we want to transmit
> over QMP, and how to structure it, then figure out if and how to change
> its human readable presentation.

Sounds like a plan ;)

cheers,
   Gerd
Markus Armbruster - Jan. 18, 2010, 12:44 p.m.
Gerd Hoffmann <kraxel@redhat.com> writes:

> On 01/18/10 11:15, Markus Armbruster wrote:
>> Nathan Baum<nathan@parenthephobia.org.uk>  writes:
>>
>>>>> +static QObject *usb_bus_dev_info(Monitor *mon, DeviceState *qdev)
>>>>> +{
>>>>> +    USBDevice *dev = DO_UPCAST(USBDevice, qdev, qdev);
>>>>> +    USBBus *bus = usb_bus_from_device(dev);
>>>>> +    return qobject_from_jsonf("{'busnr': %d, 'addr':%d, 'speed': %s, 'desc': %s, 'attached': %i}",
>>>>> +                              bus->busnr,
>>>>
>>>> As for PCI, 'busnr' belongs to the bus, not the device.
>
> You want be able to figure which bus the device is attached to.
>
> I think you actually can because the command returns the device tree
> converted into a qobject tree, correct?

Correct.

[...]

Patch

diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index 54027df..6d02807 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c
@@ -3,6 +3,7 @@ 
 #include "qdev.h"
 #include "sysemu.h"
 #include "monitor.h"
+#include "qjson.h"
 
 static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent);
 
@@ -232,6 +233,18 @@  static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent)
                    dev->attached ? ", attached" : "");
 }
 
+static QObject *usb_bus_dev_info(Monitor *mon, DeviceState *qdev)
+{
+    USBDevice *dev = DO_UPCAST(USBDevice, qdev, qdev);
+    USBBus *bus = usb_bus_from_device(dev);
+    return qobject_from_jsonf("{'busnr': %d, 'addr':%d, 'speed': %s, 'desc': %s, 'attached': %i}",
+                              bus->busnr,
+                              dev->addr,
+                              usb_speed(dev->speed),
+                              dev->product_desc,
+                              dev->attached);
+}
+
 void usb_info(Monitor *mon)
 {
     USBBus *bus;