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

[Markus Armbruster <armbru@redhat.com>]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> 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)
> +{
> +    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"));

Despite its name, qdict_get_int() doesn't return int, but int64_t (trap
for the unwary, in my opinion), so these casts are actually needed.
The're safe, because the values put into the dictionary never overflow
the cast.

Would be nice if we could avoid this non-local argument and make the
code more obviously correct, but it's probably not worth the bother.

> +
> +}
> +
> +/**
> + * 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));
> +        assert(*ret_data != NULL);
>      } else
>          monitor_printf(mon, "failed to add %s\n", opts);
>  }
[...]