| Message ID | 1258489944-12159-14-git-send-email-lcapitulino@redhat.com |
|---|---|
| State | New |
| Headers | show |
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); > } [...]
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? > +{ > + 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. > + 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 > .help = "hot-add PCI device", > - .mhandler.cmd = pci_device_hot_add, > + .user_print = pci_device_hot_add_print, > + .mhandler.cmd_new = pci_device_hot_add, > }, > #endif > > diff --git a/sysemu.h b/sysemu.h > index b1887ef..4d685a0 100644 > --- a/sysemu.h > +++ b/sysemu.h > @@ -201,7 +201,8 @@ extern DriveInfo *drive_init(QemuOpts *arg, void *machine, int *fatal_error); > DriveInfo *add_init_drive(const char *opts); > > /* pci-hotplug */ > -void pci_device_hot_add(Monitor *mon, const QDict *qdict); > +void pci_device_hot_add_print(Monitor *mon, const QObject *data); > +void pci_device_hot_add(Monitor *mon, const QDict *qdict, QObject **ret_data); > void drive_hot_add(Monitor *mon, const QDict *qdict); > void pci_device_hot_remove(Monitor *mon, const char *pci_addr); > void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict, > -- > 1.6.5.2.180.gc5b3e > >
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.
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" <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. 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. >
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")); + +} + +/** + * 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); } 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]...", .help = "hot-add PCI device", - .mhandler.cmd = pci_device_hot_add, + .user_print = pci_device_hot_add_print, + .mhandler.cmd_new = pci_device_hot_add, }, #endif diff --git a/sysemu.h b/sysemu.h index b1887ef..4d685a0 100644 --- a/sysemu.h +++ b/sysemu.h @@ -201,7 +201,8 @@ extern DriveInfo *drive_init(QemuOpts *arg, void *machine, int *fatal_error); DriveInfo *add_init_drive(const char *opts); /* pci-hotplug */ -void pci_device_hot_add(Monitor *mon, const QDict *qdict); +void pci_device_hot_add_print(Monitor *mon, const QObject *data); +void pci_device_hot_add(Monitor *mon, const QDict *qdict, QObject **ret_data); void drive_hot_add(Monitor *mon, const QDict *qdict); void pci_device_hot_remove(Monitor *mon, const char *pci_addr); void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict,
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(-)