Markus,
On 22/09/2016 17:49, Markus Armbruster wrote:
> 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?
Sure
Thanks
Eric
>
>> @@ -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 */
>
