[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 08/17] vfio: Pass an Error object to vfio_con
From: |
Alexey Kardashevskiy |
Subject: |
Re: [Qemu-devel] [PATCH v5 08/17] vfio: Pass an Error object to vfio_connect_container |
Date: |
Mon, 10 Oct 2016 13:02:13 +1100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 |
On 07/10/16 18:36, Auger Eric wrote:
> Hi,
>
> On 07/10/2016 09:01, Markus Armbruster wrote:
>> Eric Auger <address@hidden> writes:
>>
>>> The error is currently simply reported in vfio_get_group. Don't
>>> bother too much with the prefix which will be handled at upper level,
>>> later on.
>>>
>>> Also return an error value in case container->error is not 0 and
>>> the container is teared down.
>>
>> "torn down", I think.
>
> Sure. I had a wrong feeling when writing this ...
>>
>> Is this a bug fix? See also below.
>>
>>> On vfio_spapr_remove_window failure, we also report an error whereas
>>> it was silent before.
>>>
>>> Signed-off-by: Eric Auger <address@hidden>
>>> Reviewed-by: Markus Armbruster <address@hidden>
>>>
>>> ---
>>>
>>> v4 -> v5:
>>> - set ret to container->error
>>> - mention error report on vfio_spapr_remove_window failure in the commit
>>> message
>>> ---
>>> hw/vfio/common.c | 40 +++++++++++++++++++++++++---------------
>>> 1 file changed, 25 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>>> index 29188a1..85a7759 100644
>>> --- a/hw/vfio/common.c
>>> +++ b/hw/vfio/common.c
>>> @@ -34,6 +34,7 @@
>>> #include "qemu/range.h"
>>> #include "sysemu/kvm.h"
>>> #include "trace.h"
>>> +#include "qapi/error.h"
>>>
>>> struct vfio_group_head vfio_group_list =
>>> QLIST_HEAD_INITIALIZER(vfio_group_list);
>>> @@ -900,7 +901,8 @@ static void vfio_put_address_space(VFIOAddressSpace
>>> *space)
>>> }
>>> }
>>>
>>> -static int vfio_connect_container(VFIOGroup *group, AddressSpace *as)
>>> +static int vfio_connect_container(VFIOGroup *group, AddressSpace *as,
>>> + Error **errp)
>>> {
>>> VFIOContainer *container;
>>> int ret, fd;
>>> @@ -918,15 +920,15 @@ static int vfio_connect_container(VFIOGroup *group,
>>> AddressSpace *as)
>>>
>>> fd = qemu_open("/dev/vfio/vfio", O_RDWR);
>>> if (fd < 0) {
>>> - error_report("vfio: failed to open /dev/vfio/vfio: %m");
>>> + error_setg_errno(errp, errno, "failed to open /dev/vfio/vfio");
>>> ret = -errno;
>>> goto put_space_exit;
>>> }
>>>
>>> ret = ioctl(fd, VFIO_GET_API_VERSION);
>>> if (ret != VFIO_API_VERSION) {
>>> - error_report("vfio: supported vfio version: %d, "
>>> - "reported version: %d", VFIO_API_VERSION, ret);
>>> + error_setg(errp, "supported vfio version: %d, "
>>> + "reported version: %d", VFIO_API_VERSION, ret);
>>> ret = -EINVAL;
>>> goto close_fd_exit;
>>> }
>>> @@ -941,7 +943,7 @@ static int vfio_connect_container(VFIOGroup *group,
>>> AddressSpace *as)
>>>
>>> ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &fd);
>>> if (ret) {
>>> - error_report("vfio: failed to set group container: %m");
>>> + error_setg_errno(errp, errno, "failed to set group container");
>>> ret = -errno;
>>> goto free_container_exit;
>>> }
>>> @@ -949,7 +951,7 @@ static int vfio_connect_container(VFIOGroup *group,
>>> AddressSpace *as)
>>> container->iommu_type = v2 ? VFIO_TYPE1v2_IOMMU : VFIO_TYPE1_IOMMU;
>>> ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);
>>> if (ret) {
>>> - error_report("vfio: failed to set iommu for container: %m");
>>> + error_setg_errno(errp, errno, "failed to set iommu for
>>> container");
>>> ret = -errno;
>>> goto free_container_exit;
>>> }
>>> @@ -976,7 +978,7 @@ static int vfio_connect_container(VFIOGroup *group,
>>> AddressSpace *as)
>>>
>>> ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &fd);
>>> if (ret) {
>>> - error_report("vfio: failed to set group container: %m");
>>> + error_setg_errno(errp, errno, "failed to set group container");
>>> ret = -errno;
>>> goto free_container_exit;
>>> }
>>> @@ -984,7 +986,7 @@ static int vfio_connect_container(VFIOGroup *group,
>>> AddressSpace *as)
>>> v2 ? VFIO_SPAPR_TCE_v2_IOMMU : VFIO_SPAPR_TCE_IOMMU;
>>> ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);
>>> if (ret) {
>>> - error_report("vfio: failed to set iommu for container: %m");
>>> + error_setg_errno(errp, errno, "failed to set iommu for
>>> container");
>>> ret = -errno;
>>> goto free_container_exit;
>>> }
>>> @@ -997,7 +999,7 @@ static int vfio_connect_container(VFIOGroup *group,
>>> AddressSpace *as)
>>> if (!v2) {
>>> ret = ioctl(fd, VFIO_IOMMU_ENABLE);
>>> if (ret) {
>>> - error_report("vfio: failed to enable container: %m");
>>> + error_setg_errno(errp, errno, "failed to enable
>>> container");
>>> ret = -errno;
>>> goto free_container_exit;
>>> }
>>> @@ -1008,7 +1010,9 @@ static int vfio_connect_container(VFIOGroup *group,
>>> AddressSpace *as)
>>> &address_space_memory);
>>> if (container->error) {
>>
>> I tried to see where non-zero container->error comes from, but failed.
>> Can you help?
>
> Added Alexey in CC
>
> It is set in vfio_prereg_listener_region_add (spapr.c)
> There is a comment there saying:
> /*
> * On the initfn path, store the first error in the container so we
> * can gracefully fail. Runtime, there's not much we can do other
> * than throw a hardware error.
> */
> 1) by the way I should also s/initfn/realize now.
> 2) by gracefully fail I understand the error should be properly
> cascaded. Also when looking at the other vfio_memory_listener
> registration below, ret is set to container->error.
> 3) I could use error_setg_errno ...
Use on what Error* exactly? It is a listener, no return codes from there...
> David, Alexey, could you confirm we should set the returned value to the
> container->error below?
The hw/vfio/common.c's ret is -errno but the hw/vfio/spapr.c's ret is
ioctl()'s ret which is not a huge difference (as that specific ioctl() does
not return successful non-zero codes) but yes, I should have used -errno in
hw/vfio/spapr.c, not just ret.
>
> Thanks
>
> Eric
>
>
>>
>>> memory_listener_unregister(&container->prereg_listener);
>>> - error_report("vfio: RAM memory listener initialization
>>> failed for container");
>>> + ret = container->error;
>>> + error_setg(errp,
>>> + "RAM memory listener initialization failed for
>>> container");
>>> goto free_container_exit;
>>> }
>>> }
>>> @@ -1016,7 +1020,8 @@ static int vfio_connect_container(VFIOGroup *group,
>>> AddressSpace *as)
>>> info.argsz = sizeof(info);
>>> ret = ioctl(fd, VFIO_IOMMU_SPAPR_TCE_GET_INFO, &info);
>>> if (ret) {
>>> - error_report("vfio: VFIO_IOMMU_SPAPR_TCE_GET_INFO failed: %m");
>>> + error_setg_errno(errp, errno,
>>> + "VFIO_IOMMU_SPAPR_TCE_GET_INFO failed");
>>> ret = -errno;
>>> if (v2) {
>>> memory_listener_unregister(&container->prereg_listener);
>>> @@ -1033,6 +1038,8 @@ static int vfio_connect_container(VFIOGroup *group,
>>> AddressSpace *as)
>>> */
>>> ret = vfio_spapr_remove_window(container,
>>> info.dma32_window_start);
>>> if (ret) {
>>> + error_setg_errno(errp, -ret,
>>> + "failed to remove existing window");
>>> goto free_container_exit;
>>> }
>>> } else {
>>> @@ -1043,7 +1050,7 @@ static int vfio_connect_container(VFIOGroup *group,
>>> AddressSpace *as)
>>> 0x1000);
>>> }
>>> } else {
>>> - error_report("vfio: No available IOMMU models");
>>> + error_setg(errp, "No available IOMMU models");
>>> ret = -EINVAL;
>>> goto free_container_exit;
>>> }
>>> @@ -1054,7 +1061,8 @@ static int vfio_connect_container(VFIOGroup *group,
>>> AddressSpace *as)
>>>
>>> if (container->error) {
>>> ret = container->error;
>>> - error_report("vfio: memory listener initialization failed for
>>> container");
>>> + error_setg_errno(errp, -ret,
>>> + "memory listener initialization failed for
>>> container");
>>> goto listener_release_exit;
>>> }
>>>
>>> @@ -1120,6 +1128,7 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace
>>> *as)
>>> VFIOGroup *group;
>>> char path[32];
>>> struct vfio_group_status status = { .argsz = sizeof(status) };
>>> + Error *err = NULL;
>>>
>>> QLIST_FOREACH(group, &vfio_group_list, next) {
>>> if (group->groupid == groupid) {
>>> @@ -1158,8 +1167,9 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace
>>> *as)
>>> group->groupid = groupid;
>>> QLIST_INIT(&group->device_list);
>>>
>>> - if (vfio_connect_container(group, as)) {
>>> - error_report("vfio: failed to setup container for group %d",
>>> groupid);
>>> + if (vfio_connect_container(group, as, &err)) {
>>> + error_reportf_err(err, "vfio: failed to setup container for group
>>> %d",
>>> + groupid);
>>> goto close_fd_exit;
>>> }
>>
--
Alexey
- [Qemu-devel] [PATCH v5 00/17] Convert VFIO-PCI to realize, Eric Auger, 2016/10/06
- [Qemu-devel] [PATCH v5 01/17] vfio/pci: Use local error object in vfio_initfn, Eric Auger, 2016/10/06
- [Qemu-devel] [PATCH v5 02/17] vfio/pci: Pass an error object to vfio_populate_vga, Eric Auger, 2016/10/06
- [Qemu-devel] [PATCH v5 04/17] vfio/pci: Pass an error object to vfio_msix_early_setup, Eric Auger, 2016/10/06
- [Qemu-devel] [PATCH v5 03/17] vfio/pci: Pass an error object to vfio_populate_device, Eric Auger, 2016/10/06
- [Qemu-devel] [PATCH v5 05/17] vfio/pci: Pass an error object to vfio_intx_enable, Eric Auger, 2016/10/06
- [Qemu-devel] [PATCH v5 07/17] vfio/pci: Pass an error object to vfio_pci_igd_opregion_init, Eric Auger, 2016/10/06
- [Qemu-devel] [PATCH v5 08/17] vfio: Pass an Error object to vfio_connect_container, Eric Auger, 2016/10/06
- Re: [Qemu-devel] [PATCH v5 08/17] vfio: Pass an Error object to vfio_connect_container, Markus Armbruster, 2016/10/07
- Re: [Qemu-devel] [PATCH v5 08/17] vfio: Pass an Error object to vfio_connect_container, Auger Eric, 2016/10/07
- Re: [Qemu-devel] [PATCH v5 08/17] vfio: Pass an Error object to vfio_connect_container,
Alexey Kardashevskiy <=
- Re: [Qemu-devel] [PATCH v5 08/17] vfio: Pass an Error object to vfio_connect_container, David Gibson, 2016/10/10
- Re: [Qemu-devel] [PATCH v5 08/17] vfio: Pass an Error object to vfio_connect_container, Auger Eric, 2016/10/10
- Re: [Qemu-devel] [PATCH v5 08/17] vfio: Pass an Error object to vfio_connect_container, Markus Armbruster, 2016/10/10
- Re: [Qemu-devel] [PATCH v5 08/17] vfio: Pass an Error object to vfio_connect_container, Auger Eric, 2016/10/10
- Re: [Qemu-devel] [PATCH v5 08/17] vfio: Pass an Error object to vfio_connect_container, David Gibson, 2016/10/10
- Re: [Qemu-devel] [PATCH v5 08/17] vfio: Pass an Error object to vfio_connect_container, Markus Armbruster, 2016/10/10
- Re: [Qemu-devel] [PATCH v5 08/17] vfio: Pass an Error object to vfio_connect_container, David Gibson, 2016/10/10
[Qemu-devel] [PATCH v5 06/17] vfio/pci: Pass an error object to vfio_add_capabilities, Eric Auger, 2016/10/06
[Qemu-devel] [PATCH v5 10/17] vfio: Pass an error object to vfio_get_device, Eric Auger, 2016/10/06
[Qemu-devel] [PATCH v5 09/17] vfio: Pass an error object to vfio_get_group, Eric Auger, 2016/10/06