It's simpler to just use a C struct than it is to bundle things
into a QDict in one function just to pull them back out in the
caller. Plus, doing this gets rid of one more user of dynamic
JSON through qobject_from_jsonf(), as well as a memory leak of
the QDict.
While cleaning the code, fix things to report all errors (the
code was previously silently ignoring a failure of
pcie_aer_inject_error(), at a distance).
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v3: more cleanups suggested by Markus, drop R-b
---
hw/pci/pcie_aer.c | 44 ++++++++++++++++++++++++++------------------
1 file changed, 26 insertions(+), 18 deletions(-)
diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index a8c1820..653af86 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -44,6 +44,13 @@
#define PCI_ERR_SRC_COR_OFFS 0
#define PCI_ERR_SRC_UNCOR_OFFS 2
+typedef struct PCIEErrorDetails {
+ const char *id;
+ const char *root_bus;
+ int bus;
+ int devfn;
+} PCIEErrorDetails;
+
/* From 6.2.7 Error Listing and Rules. Table 6-2, 6-3 and 6-4 */
static uint32_t pcie_aer_uncor_default_severity(uint32_t status)
{
@@ -942,8 +949,14 @@ static int pcie_aer_parse_error_string(const char *error_name,
return -EINVAL;
}
+/*
+ * Inject an error described by @qdict.
+ * On success, set @details to show where error was sent.
+ * Return negative errno if injection failed and a message was emitted.
+ */
static int do_pcie_aer_inject_error(Monitor *mon,
- const QDict *qdict, QObject **ret_data)
+ const QDict *qdict,
+ PCIEErrorDetails *details)
{
const char *id = qdict_get_str(qdict, "id");
const char *error_name;
@@ -1005,33 +1018,28 @@ static int do_pcie_aer_inject_error(Monitor *mon,
err.prefix[3] = qdict_get_try_int(qdict, "prefix3", 0);
ret = pcie_aer_inject_error(dev, &err);
- *ret_data = qobject_from_jsonf("{'id': %s, "
- "'root_bus': %s, 'bus': %d, 'devfn': %d, "
- "'ret': %d}",
- id, pci_root_bus_path(dev),
- pci_bus_num(dev->bus), dev->devfn,
- ret);
- assert(*ret_data);
+ if (ret < 0) {
+ monitor_printf(mon, "failed to inject error: %s\n",
+ strerror(-ret));
+ return ret;
+ }
+ details->id = id;
+ details->root_bus = pci_root_bus_path(dev);
+ details->bus = pci_bus_num(dev->bus);
+ details->devfn = dev->devfn;
return 0;
}
void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict)
{
- QObject *data;
- int devfn;
+ PCIEErrorDetails data;
if (do_pcie_aer_inject_error(mon, qdict, &data) < 0) {
return;
}
- qdict = qobject_to_qdict(data);
- assert(qdict);
-
- devfn = (int)qdict_get_int(qdict, "devfn");
monitor_printf(mon, "OK id: %s root bus: %s, bus: %x devfn: %x.%x\n",
- qdict_get_str(qdict, "id"),
- qdict_get_str(qdict, "root_bus"),
- (int) qdict_get_int(qdict, "bus"),
- PCI_SLOT(devfn), PCI_FUNC(devfn));
+ data.id, data.root_bus, data.bus,
+ PCI_SLOT(data.devfn), PCI_FUNC(data.devfn));
}
--
2.9.3
On Wed, Apr 05, 2017 at 02:47:29PM -0500, Eric Blake wrote:
> It's simpler to just use a C struct than it is to bundle things
> into a QDict in one function just to pull them back out in the
> caller. Plus, doing this gets rid of one more user of dynamic
> JSON through qobject_from_jsonf(), as well as a memory leak of
> the QDict.
>
> While cleaning the code, fix things to report all errors (the
> code was previously silently ignoring a failure of
> pcie_aer_inject_error(), at a distance).
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> v3: more cleanups suggested by Markus, drop R-b
> ---
> hw/pci/pcie_aer.c | 44 ++++++++++++++++++++++++++------------------
> 1 file changed, 26 insertions(+), 18 deletions(-)
>
> diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
> index a8c1820..653af86 100644
> --- a/hw/pci/pcie_aer.c
> +++ b/hw/pci/pcie_aer.c
> @@ -44,6 +44,13 @@
> #define PCI_ERR_SRC_COR_OFFS 0
> #define PCI_ERR_SRC_UNCOR_OFFS 2
>
> +typedef struct PCIEErrorDetails {
> + const char *id;
> + const char *root_bus;
> + int bus;
> + int devfn;
> +} PCIEErrorDetails;
> +
> /* From 6.2.7 Error Listing and Rules. Table 6-2, 6-3 and 6-4 */
> static uint32_t pcie_aer_uncor_default_severity(uint32_t status)
> {
> @@ -942,8 +949,14 @@ static int pcie_aer_parse_error_string(const char *error_name,
> return -EINVAL;
> }
>
> +/*
> + * Inject an error described by @qdict.
> + * On success, set @details to show where error was sent.
> + * Return negative errno if injection failed and a message was emitted.
> + */
> static int do_pcie_aer_inject_error(Monitor *mon,
> - const QDict *qdict, QObject **ret_data)
> + const QDict *qdict,
> + PCIEErrorDetails *details)
> {
> const char *id = qdict_get_str(qdict, "id");
> const char *error_name;
> @@ -1005,33 +1018,28 @@ static int do_pcie_aer_inject_error(Monitor *mon,
> err.prefix[3] = qdict_get_try_int(qdict, "prefix3", 0);
>
> ret = pcie_aer_inject_error(dev, &err);
> - *ret_data = qobject_from_jsonf("{'id': %s, "
> - "'root_bus': %s, 'bus': %d, 'devfn': %d, "
> - "'ret': %d}",
> - id, pci_root_bus_path(dev),
> - pci_bus_num(dev->bus), dev->devfn,
> - ret);
> - assert(*ret_data);
> + if (ret < 0) {
> + monitor_printf(mon, "failed to inject error: %s\n",
> + strerror(-ret));
> + return ret;
> + }
> + details->id = id;
> + details->root_bus = pci_root_bus_path(dev);
> + details->bus = pci_bus_num(dev->bus);
> + details->devfn = dev->devfn;
>
> return 0;
> }
>
> void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict)
> {
> - QObject *data;
> - int devfn;
> + PCIEErrorDetails data;
>
> if (do_pcie_aer_inject_error(mon, qdict, &data) < 0) {
> return;
> }
>
> - qdict = qobject_to_qdict(data);
> - assert(qdict);
> -
> - devfn = (int)qdict_get_int(qdict, "devfn");
> monitor_printf(mon, "OK id: %s root bus: %s, bus: %x devfn: %x.%x\n",
> - qdict_get_str(qdict, "id"),
> - qdict_get_str(qdict, "root_bus"),
> - (int) qdict_get_int(qdict, "bus"),
> - PCI_SLOT(devfn), PCI_FUNC(devfn));
> + data.id, data.root_bus, data.bus,
> + PCI_SLOT(data.devfn), PCI_FUNC(data.devfn));
> }
> --
> 2.9.3
On 04/05/2017 10:47 PM, Eric Blake wrote:
> It's simpler to just use a C struct than it is to bundle things
> into a QDict in one function just to pull them back out in the
> caller. Plus, doing this gets rid of one more user of dynamic
> JSON through qobject_from_jsonf(), as well as a memory leak of
> the QDict.
>
> While cleaning the code, fix things to report all errors (the
> code was previously silently ignoring a failure of
> pcie_aer_inject_error(), at a distance).
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> ---
> v3: more cleanups suggested by Markus, drop R-b
> ---
> hw/pci/pcie_aer.c | 44 ++++++++++++++++++++++++++------------------
> 1 file changed, 26 insertions(+), 18 deletions(-)
>
> diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
> index a8c1820..653af86 100644
> --- a/hw/pci/pcie_aer.c
> +++ b/hw/pci/pcie_aer.c
> @@ -44,6 +44,13 @@
> #define PCI_ERR_SRC_COR_OFFS 0
> #define PCI_ERR_SRC_UNCOR_OFFS 2
>
> +typedef struct PCIEErrorDetails {
> + const char *id;
> + const char *root_bus;
> + int bus;
> + int devfn;
> +} PCIEErrorDetails;
> +
> /* From 6.2.7 Error Listing and Rules. Table 6-2, 6-3 and 6-4 */
> static uint32_t pcie_aer_uncor_default_severity(uint32_t status)
> {
> @@ -942,8 +949,14 @@ static int pcie_aer_parse_error_string(const char *error_name,
> return -EINVAL;
> }
>
> +/*
> + * Inject an error described by @qdict.
> + * On success, set @details to show where error was sent.
> + * Return negative errno if injection failed and a message was emitted.
> + */
> static int do_pcie_aer_inject_error(Monitor *mon,
> - const QDict *qdict, QObject **ret_data)
> + const QDict *qdict,
> + PCIEErrorDetails *details)
> {
> const char *id = qdict_get_str(qdict, "id");
> const char *error_name;
> @@ -1005,33 +1018,28 @@ static int do_pcie_aer_inject_error(Monitor *mon,
> err.prefix[3] = qdict_get_try_int(qdict, "prefix3", 0);
>
> ret = pcie_aer_inject_error(dev, &err);
> - *ret_data = qobject_from_jsonf("{'id': %s, "
> - "'root_bus': %s, 'bus': %d, 'devfn': %d, "
> - "'ret': %d}",
> - id, pci_root_bus_path(dev),
> - pci_bus_num(dev->bus), dev->devfn,
> - ret);
> - assert(*ret_data);
> + if (ret < 0) {
> + monitor_printf(mon, "failed to inject error: %s\n",
> + strerror(-ret));
> + return ret;
> + }
> + details->id = id;
> + details->root_bus = pci_root_bus_path(dev);
> + details->bus = pci_bus_num(dev->bus);
> + details->devfn = dev->devfn;
>
> return 0;
> }
>
> void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict)
> {
> - QObject *data;
> - int devfn;
> + PCIEErrorDetails data;
>
> if (do_pcie_aer_inject_error(mon, qdict, &data) < 0) {
> return;
> }
>
> - qdict = qobject_to_qdict(data);
> - assert(qdict);
> -
> - devfn = (int)qdict_get_int(qdict, "devfn");
> monitor_printf(mon, "OK id: %s root bus: %s, bus: %x devfn: %x.%x\n",
> - qdict_get_str(qdict, "id"),
> - qdict_get_str(qdict, "root_bus"),
> - (int) qdict_get_int(qdict, "bus"),
> - PCI_SLOT(devfn), PCI_FUNC(devfn));
> + data.id, data.root_bus, data.bus,
> + PCI_SLOT(data.devfn), PCI_FUNC(data.devfn));
> }
>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Thanks,
Marcel
© 2016 - 2026 Red Hat, Inc.