Eric Auger <eric.au...@redhat.com> writes:
> Pass an error object to prepare for migration to VFIO-PCI realize.
> The returned value will be removed later on.
>
> The case where error recovery cannot be enabled is not converted into
> an error object but directly reported through error_report, as before.
> Populating an error instead would cause the future realize function to
> fail, which is not wanted.
>
> Signed-off-by: Eric Auger <eric.au...@redhat.com>
>
> ---
>
> v1 -> v2:
> - this patch now comes before the actual realize migration
> - do not handle the error recovery issue as an error
> ---
> hw/vfio/pci.c | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index ca0293d..9fe970a 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2199,7 +2199,7 @@ int vfio_populate_vga(VFIOPCIDevice *vdev)
> return 0;
> }
>
> -static int vfio_populate_device(VFIOPCIDevice *vdev)
> +static int vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
> {
> VFIODevice *vbasedev = &vdev->vbasedev;
> struct vfio_region_info *reg_info;
> @@ -2208,18 +2208,18 @@ static int vfio_populate_device(VFIOPCIDevice *vdev)
>
> /* Sanity check device */
> if (!(vbasedev->flags & VFIO_DEVICE_FLAGS_PCI)) {
> - error_report("vfio: Um, this isn't a PCI device");
> + error_setg(errp, "this isn't a PCI device");
> goto error;
> }
>
> if (vbasedev->num_regions < VFIO_PCI_CONFIG_REGION_INDEX + 1) {
> - error_report("vfio: unexpected number of io regions %u",
> - vbasedev->num_regions);
> + error_setg(errp, "unexpected number of io regions %u",
> + vbasedev->num_regions);
> goto error;
> }
>
> if (vbasedev->num_irqs < VFIO_PCI_MSIX_IRQ_INDEX + 1) {
> - error_report("vfio: unexpected number of irqs %u",
> vbasedev->num_irqs);
> + error_setg(errp, "unexpected number of irqs %u", vbasedev->num_irqs);
> goto error;
> }
>
> @@ -2231,7 +2231,7 @@ static int vfio_populate_device(VFIOPCIDevice *vdev)
> g_free(name);
>
> if (ret) {
> - error_report("vfio: Error getting region %d info: %m", i);
> + error_setg_errno(errp, -ret, "failed to get region %d info", i);
> goto error;
> }
>
> @@ -2241,7 +2241,7 @@ static int vfio_populate_device(VFIOPCIDevice *vdev)
> ret = vfio_get_region_info(vbasedev,
> VFIO_PCI_CONFIG_REGION_INDEX, ®_info);
> if (ret) {
> - error_report("vfio: Error getting config info: %m");
> + error_setg_errno(errp, -ret, "failed to get config info");
> goto error;
> }
>
> @@ -2261,8 +2261,8 @@ static int vfio_populate_device(VFIOPCIDevice *vdev)
> if (vdev->features & VFIO_FEATURE_ENABLE_VGA) {
> ret = vfio_populate_vga(vdev);
> if (ret) {
> - error_report(
> - "vfio: Device does not support requested feature x-vga");
> + error_setg_errno(errp, -ret,
> + "device does not support requested feature x-vga");
> goto error;
> }
> }
What about the error_report() in vfio_populate_vga()? Should the
vfio_populate_vga() take an Error * argument?
> @@ -2277,7 +2277,7 @@ static int vfio_populate_device(VFIOPCIDevice *vdev)
> } else if (irq_info.count == 1) {
> vdev->pci_aer = true;
> } else {
> - error_report("vfio: %s "
> + error_report(WARN_PREFIX
> "Could not enable error recovery for the device",
> vbasedev->name);
> }
> @@ -2561,9 +2561,9 @@ static int vfio_initfn(PCIDevice *pdev)
> goto error;
> }
>
> - ret = vfio_populate_device(vdev);
> - if (ret) {
> - return ret;
> + ret = vfio_populate_device(vdev, &err);
> + if (err) {
> + goto error;
> }
>
> /* Get a copy of config space */
