From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NF3ad-0000kc-5w for qemu-devel@nongnu.org; Mon, 30 Nov 2009 05:34:43 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NF3aY-0000jI-Tv for qemu-devel@nongnu.org; Mon, 30 Nov 2009 05:34:42 -0500 Received: from [199.232.76.173] (port=60916 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NF3aY-0000jB-Ju for qemu-devel@nongnu.org; Mon, 30 Nov 2009 05:34:38 -0500 Received: from mx1.redhat.com ([209.132.183.28]:11938) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NF3aY-00044i-5f for qemu-devel@nongnu.org; Mon, 30 Nov 2009 05:34:38 -0500 Received: from int-mx04.intmail.prod.int.phx2.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.17]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id nAUAYank012318 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 30 Nov 2009 05:34:37 -0500 Date: Mon, 30 Nov 2009 12:31:57 +0200 From: "Michael S. Tsirkin" Message-ID: <20091130103156.GA11556@redhat.com> References: <1258489944-12159-1-git-send-email-lcapitulino@redhat.com> <1258489944-12159-14-git-send-email-lcapitulino@redhat.com> <20091123094457.GC3748@redhat.com> <20091123113044.4db46561@doriath> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20091123113044.4db46561@doriath> Subject: [Qemu-devel] Re: [PATCH 13/17] PCI: Convert pci_device_hot_add() to QObject List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: qemu-devel@nongnu.org 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" 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 > > > --- > > > 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|[[:]:] nic|storage [[vlan=n][,macaddr=addr][,model=type]] [file=file][,if=type][,bus=nr]...", > > > > > > and this > > This line is not part of the series. >