Re: [Qemu-devel] [PATCH v2 05/12] vfio/pci: Pass an error object to vfio_add_capabilities

2016-09-30 Thread Auger Eric 
Hi Markus,

On 22/09/2016 18:52, Markus Armbruster wrote:
> Eric Auger  writes:
> 
>> Pass an error object to prepare for migration to VFIO-PCI realize.
>> The error is cascaded downto vfio_add_std_cap and then vfio_msi(x)_setup,
>> vfio_setup_pcie_cap.
>>
>> vfio_add_ext_cap does not return anything else than 0 so let's transform
>> it into a void function.
>>
>> Also use pci_add_capability2 which takes an error object.
>>
>> Signed-off-by: Eric Auger 
>> ---
>>  hw/vfio/pci.c | 66 
>> ---
>>  1 file changed, 36 insertions(+), 30 deletions(-)
>>
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index f67eec4..07a44f5 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -1178,7 +1178,7 @@ static void vfio_disable_interrupts(VFIOPCIDevice 
>> *vdev)
>>  }
>>  }
>>  
>> -static int vfio_msi_setup(VFIOPCIDevice *vdev, int pos)
>> +static int vfio_msi_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
>>  {
>>  uint16_t ctrl;
>>  bool msi_64bit, msi_maskbit;
>int ret, entries;
>Error *err = NULL;
> 
>if (pread(vdev->vbasedev.fd, , sizeof(ctrl),
>  vdev->config_offset + pos + PCI_CAP_FLAGS) != sizeof(ctrl)) {
>return -errno;
>}
> 
> Fails without setting error here.
Sure
> 
>> @@ -1202,8 +1202,7 @@ static int vfio_msi_setup(VFIOPCIDevice *vdev, int pos)
>>  if (ret == -ENOTSUP) {
>>  return 0;
>>  }
>> -error_prepend(, "vfio: msi_init failed: ");
>> -error_report_err(err);
>> +error_propagate(errp, err);
>>  return ret;
>>  }
>>  vdev->msi_cap_size = 0xa + (msi_maskbit ? 0xa : 0) + (msi_64bit ? 0x4 : 
>> 0);
>> @@ -1361,7 +1360,7 @@ static int vfio_msix_early_setup(VFIOPCIDevice *vdev, 
>> Error **errp)
>>  return 0;
>>  }
>>  
>> -static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos)
>> +static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
>>  {
>>  int ret;
>>  
>> @@ -1376,7 +1375,7 @@ static int vfio_msix_setup(VFIOPCIDevice *vdev, int 
>> pos)
>>  if (ret == -ENOTSUP) {
>>  return 0;
>>  }
>> -error_report("vfio: msix_init failed");
>> +error_setg(errp, "msix_init failed");
>>  return ret;
>>  }
>>  
>> @@ -1561,7 +1560,8 @@ static void vfio_add_emulated_long(VFIOPCIDevice 
>> *vdev, int pos,
>>  vfio_set_long_bits(vdev->emulated_config_bits + pos, mask, mask);
>>  }
>>  
>> -static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size)
>> +static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size,
>> +   Error **errp)
>>  {
>>  uint16_t flags;
>>  uint8_t type;
>> @@ -1573,8 +1573,8 @@ static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, 
>> int pos, uint8_t size)
>>  type != PCI_EXP_TYPE_LEG_END &&
>>  type != PCI_EXP_TYPE_RC_END) {
>>  
>> -error_report("vfio: Assignment of PCIe type 0x%x "
>> - "devices is not currently supported", type);
>> +error_setg(errp, "assignment of PCIe type 0x%x "
>> +   "devices is not currently supported", type);
>>  return -EINVAL;
>>  }
>>  
>> @@ -1708,10 +1708,11 @@ static void vfio_check_af_flr(VFIOPCIDevice *vdev, 
>> uint8_t pos)
>>  }
>>  }
>>  
>> -static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos)
>> +static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos, Error **errp)
>>  {
>>  PCIDevice *pdev = >pdev;
>>  uint8_t cap_id, next, size;
>> +Error *err = NULL;
>>  int ret;
>>  
>>  cap_id = pdev->config[pos];
>> @@ -1733,9 +1734,9 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, 
>> uint8_t pos)
>>   * will be changed as we unwind the stack.
>>   */
>>  if (next) {
>> -ret = vfio_add_std_cap(vdev, next);
>> +ret = vfio_add_std_cap(vdev, next, );
>>  if (ret) {
>> -return ret;
>> +goto out;
>>  }
>>  } else {
>>  /* Begin the rebuild, use QEMU emulated list bits */
>> @@ -1749,40 +1750,42 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, 
>> uint8_t pos)
>>  
>>  switch (cap_id) {
>>  case PCI_CAP_ID_MSI:
>> -ret = vfio_msi_setup(vdev, pos);
>> +ret = vfio_msi_setup(vdev, pos, );
>>  break;
>>  case PCI_CAP_ID_EXP:
>>  vfio_check_pcie_flr(vdev, pos);
>> -ret = vfio_setup_pcie_cap(vdev, pos, size);
>> +ret = vfio_setup_pcie_cap(vdev, pos, size, );
>>  break;
>>  case PCI_CAP_ID_MSIX:
>> -ret = vfio_msix_setup(vdev, pos);
>> +ret = vfio_msix_setup(vdev, pos, );
>>  break;
>>  case PCI_CAP_ID_PM:
>>  vfio_check_pm_reset(vdev, pos);
>>  vdev->pm_cap = pos;
>> -ret = pci_add_capability(pdev, cap_id, pos, size);
>> +ret = pci_add_capability2(pdev,

Re: [Qemu-devel] [PATCH v2 05/12] vfio/pci: Pass an error object to vfio_add_capabilities

2016-09-22 Thread Markus Armbruster 
Eric Auger  writes:

> Pass an error object to prepare for migration to VFIO-PCI realize.
> The error is cascaded downto vfio_add_std_cap and then vfio_msi(x)_setup,
> vfio_setup_pcie_cap.
>
> vfio_add_ext_cap does not return anything else than 0 so let's transform
> it into a void function.
>
> Also use pci_add_capability2 which takes an error object.
>
> Signed-off-by: Eric Auger 
> ---
>  hw/vfio/pci.c | 66 
> ---
>  1 file changed, 36 insertions(+), 30 deletions(-)
>
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index f67eec4..07a44f5 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1178,7 +1178,7 @@ static void vfio_disable_interrupts(VFIOPCIDevice *vdev)
>  }
>  }
>  
> -static int vfio_msi_setup(VFIOPCIDevice *vdev, int pos)
> +static int vfio_msi_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
>  {
>  uint16_t ctrl;
>  bool msi_64bit, msi_maskbit;
   int ret, entries;
   Error *err = NULL;

   if (pread(vdev->vbasedev.fd, , sizeof(ctrl),
 vdev->config_offset + pos + PCI_CAP_FLAGS) != sizeof(ctrl)) {
   return -errno;
   }

Fails without setting error here.

> @@ -1202,8 +1202,7 @@ static int vfio_msi_setup(VFIOPCIDevice *vdev, int pos)
>  if (ret == -ENOTSUP) {
>  return 0;
>  }
> -error_prepend(, "vfio: msi_init failed: ");
> -error_report_err(err);
> +error_propagate(errp, err);
>  return ret;
>  }
>  vdev->msi_cap_size = 0xa + (msi_maskbit ? 0xa : 0) + (msi_64bit ? 0x4 : 
> 0);
> @@ -1361,7 +1360,7 @@ static int vfio_msix_early_setup(VFIOPCIDevice *vdev, 
> Error **errp)
>  return 0;
>  }
>  
> -static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos)
> +static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
>  {
>  int ret;
>  
> @@ -1376,7 +1375,7 @@ static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos)
>  if (ret == -ENOTSUP) {
>  return 0;
>  }
> -error_report("vfio: msix_init failed");
> +error_setg(errp, "msix_init failed");
>  return ret;
>  }
>  
> @@ -1561,7 +1560,8 @@ static void vfio_add_emulated_long(VFIOPCIDevice *vdev, 
> int pos,
>  vfio_set_long_bits(vdev->emulated_config_bits + pos, mask, mask);
>  }
>  
> -static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size)
> +static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size,
> +   Error **errp)
>  {
>  uint16_t flags;
>  uint8_t type;
> @@ -1573,8 +1573,8 @@ static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int 
> pos, uint8_t size)
>  type != PCI_EXP_TYPE_LEG_END &&
>  type != PCI_EXP_TYPE_RC_END) {
>  
> -error_report("vfio: Assignment of PCIe type 0x%x "
> - "devices is not currently supported", type);
> +error_setg(errp, "assignment of PCIe type 0x%x "
> +   "devices is not currently supported", type);
>  return -EINVAL;
>  }
>  
> @@ -1708,10 +1708,11 @@ static void vfio_check_af_flr(VFIOPCIDevice *vdev, 
> uint8_t pos)
>  }
>  }
>  
> -static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos)
> +static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos, Error **errp)
>  {
>  PCIDevice *pdev = >pdev;
>  uint8_t cap_id, next, size;
> +Error *err = NULL;
>  int ret;
>  
>  cap_id = pdev->config[pos];
> @@ -1733,9 +1734,9 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, 
> uint8_t pos)
>   * will be changed as we unwind the stack.
>   */
>  if (next) {
> -ret = vfio_add_std_cap(vdev, next);
> +ret = vfio_add_std_cap(vdev, next, );
>  if (ret) {
> -return ret;
> +goto out;
>  }
>  } else {
>  /* Begin the rebuild, use QEMU emulated list bits */
> @@ -1749,40 +1750,42 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, 
> uint8_t pos)
>  
>  switch (cap_id) {
>  case PCI_CAP_ID_MSI:
> -ret = vfio_msi_setup(vdev, pos);
> +ret = vfio_msi_setup(vdev, pos, );
>  break;
>  case PCI_CAP_ID_EXP:
>  vfio_check_pcie_flr(vdev, pos);
> -ret = vfio_setup_pcie_cap(vdev, pos, size);
> +ret = vfio_setup_pcie_cap(vdev, pos, size, );
>  break;
>  case PCI_CAP_ID_MSIX:
> -ret = vfio_msix_setup(vdev, pos);
> +ret = vfio_msix_setup(vdev, pos, );
>  break;
>  case PCI_CAP_ID_PM:
>  vfio_check_pm_reset(vdev, pos);
>  vdev->pm_cap = pos;
> -ret = pci_add_capability(pdev, cap_id, pos, size);
> +ret = pci_add_capability2(pdev, cap_id, pos, size, );
>  break;
>  case PCI_CAP_ID_AF:
>  vfio_check_af_flr(vdev, pos);
> -ret = pci_add_capability(pdev, cap_id, pos, size);
> +ret = pci_add_capability2(pdev, cap_id, pos,