On Mon, Nov 23, 2009 at 11:30:44AM -0200, Luiz Capitulino wrote:
> On Mon, 23 Nov 2009 11:44:57 +0200
> "Michael S. Tsirkin" <m...@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 <lcapitul...@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.
Here's one that looks decent to me.
*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));
> > > + assert(*ret_data != NULL);
BTW, pls make this assert(*ret_data);
> > > } 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.
>
