[Qemu-devel] Re: [PATCH 13/17] PCI: Convert pci_device_hot_add() to QObject

[Luiz Capitulino <lcapitulino@redhat.com>]

On Mon, 23 Nov 2009 11:44:57 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Tue, Nov 17, 2009 at 06:32:20PM -0200, Luiz Capitulino wrote:
> > Return a QDict with information about the just added device.
> > 
> > This commit should not change user output.
> > 
> > Please, note that this patch does not do error handling
> > conversion. In error conditions the handler still calls
> > monitor_printf().
> > 
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> >  hw/pci-hotplug.c |   37 +++++++++++++++++++++++++++++++++----
> >  qemu-monitor.hx  |    3 ++-
> >  sysemu.h         |    3 ++-
> >  3 files changed, 37 insertions(+), 6 deletions(-)
> > 
> > diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
> > index a254498..93802a2 100644
> > --- a/hw/pci-hotplug.c
> > +++ b/hw/pci-hotplug.c
> > @@ -33,6 +33,7 @@
> >  #include "scsi.h"
> >  #include "virtio-blk.h"
> >  #include "qemu-config.h"
> > +#include "qemu-objects.h"
> >  
> >  #if defined(TARGET_I386)
> >  static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
> > @@ -212,7 +213,36 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
> >      return dev;
> >  }
> >  
> > -void pci_device_hot_add(Monitor *mon, const QDict *qdict)
> > +void pci_device_hot_add_print(Monitor *mon, const QObject *data)
> 
> it only prints - why the strange name?

 I'm adding the _print suffix to the functions which print
handler data, that's, handle_name + _print.

 It may have strange results, indeed, but sometimes it's difficult
to come with good names as I'm converting several handlers.

 Also, I'd like to have standard names and haven't decided for
one yet.

> > +{
> > +    QDict *qdict;
> > +
> > +    assert(qobject_type(data) == QTYPE_QDICT);
> > +    qdict = qobject_to_qdict(data);
> > +
> > +    monitor_printf(mon, "OK domain %d, bus %d, slot %d, function %d\n",
> > +                   (int) qdict_get_int(qdict, "domain"),
> > +                   (int) qdict_get_int(qdict, "bus"),
> > +                   (int) qdict_get_int(qdict, "slot"),
> > +                   (int) qdict_get_int(qdict, "function"));
> > +
> > +}
> > +
> > +/**
> > + * pci_device_hot_add(): Hot add PCI device
> > + *
> > + * Return a QDict with the following device information:
> > + *
> > + * - "domain": domain number
> > + * - "bus": bus number
> > + * - "slot": slot number
> > + * - "function": function number
> > + *
> > + * Example:
> > + *
> > + * { "domain": 0, "bus": 0, "slot": 5, "function": 0 }
> > + */
> > +void pci_device_hot_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
> >  {
> >      PCIDevice *dev = NULL;
> >      const char *pci_addr = qdict_get_str(qdict, "pci_addr");
> > @@ -239,9 +269,8 @@ void pci_device_hot_add(Monitor *mon, const QDict *qdict)
> >          monitor_printf(mon, "invalid type: %s\n", type);
> >  
> >      if (dev) {
> > -        monitor_printf(mon, "OK domain %d, bus %d, slot %d, function %d\n",
> > -                       0, pci_bus_num(dev->bus), PCI_SLOT(dev->devfn),
> > -                       PCI_FUNC(dev->devfn));
> > +        *ret_data = qobject_from_jsonf("{ 'domain': 0, 'bus': %d, 'slot': %d, 'function': %d }", pci_bus_num(dev->bus), PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));
> 
> this line is waay too long.

 I can brake it, although I don't think the result is going to be
prettier.

> > +        assert(*ret_data != NULL);
> >      } else
> >          monitor_printf(mon, "failed to add %s\n", opts);
> >  }
> > diff --git a/qemu-monitor.hx b/qemu-monitor.hx
> > index 62e395b..b50a2da 100644
> > --- a/qemu-monitor.hx
> > +++ b/qemu-monitor.hx
> > @@ -809,7 +809,8 @@ ETEXI
> >          .args_type  = "pci_addr:s,type:s,opts:s?",
> >          .params     = "auto|[[<domain>:]<bus>:]<slot> nic|storage [[vlan=n][,macaddr=addr][,model=type]] [file=file][,if=type][,bus=nr]...",
> 
> 
> and this

 This line is not part of the series.