| Message ID | 1474404352-28958-8-git-send-email-eric.auger@redhat.com |
|---|---|
| State | New |
| Headers | show |
Eric Auger <eric.auger@redhat.com> writes: > Pass an error object to prepare for migration to VFIO-PCI realize. > > Signed-off-by: Eric Auger <eric.auger@redhat.com> > > --- > > v2: creation > --- > hw/vfio/common.c | 20 +++++++++++--------- > hw/vfio/pci.c | 3 +-- > hw/vfio/platform.c | 11 ++++++++--- > include/hw/vfio/vfio-common.h | 2 +- > 4 files changed, 21 insertions(+), 15 deletions(-) > > diff --git a/hw/vfio/common.c b/hw/vfio/common.c > index b313e7c..ef9e4cd 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); > @@ -1115,7 +1116,7 @@ static void vfio_disconnect_container(VFIOGroup *group) > } > } > > -VFIOGroup *vfio_get_group(int groupid, AddressSpace *as) > +VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp) > { > VFIOGroup *group; > char path[32]; > @@ -1127,8 +1128,8 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace *as) > if (group->container->space->as == as) { > return group; > } else { > - error_report("vfio: group %d used in multiple address spaces", > - group->groupid); > + error_setg(errp, "group %d used in multiple address spaces", > + group->groupid); > return NULL; > } > } > @@ -1139,19 +1140,20 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace *as) > snprintf(path, sizeof(path), "/dev/vfio/%d", groupid); > group->fd = qemu_open(path, O_RDWR); > if (group->fd < 0) { > - error_report("vfio: error opening %s: %m", path); > + error_setg_errno(errp, errno, "error opening %s", path); > goto free_group_exit; > } > > if (ioctl(group->fd, VFIO_GROUP_GET_STATUS, &status)) { > - error_report("vfio: error getting group status: %m"); > + error_setg_errno(errp, errno, "error getting group %d status", groupid); > goto close_fd_exit; > } > > if (!(status.flags & VFIO_GROUP_FLAGS_VIABLE)) { > - error_report("vfio: error, group %d is not viable, please ensure " > - "all devices within the iommu_group are bound to their " > - "vfio bus driver.", groupid); > + error_setg(errp, "group %d is not viable", groupid); > + error_append_hint(errp, > + "Please ensure all devices within the iommu_group " > + "are bound to their vfio bus driver.\n"); > goto close_fd_exit; > } > > @@ -1159,7 +1161,7 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace *as) > QLIST_INIT(&group->device_list); > > if (vfio_connect_container(group, as)) { > - error_report("vfio: failed to setup container for group %d", groupid); > + error_setg(errp, "failed to setup container for group %d", groupid); > goto close_fd_exit; > } > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > index f9a4fe7..1173d4a 100644 > --- a/hw/vfio/pci.c > +++ b/hw/vfio/pci.c > @@ -2562,9 +2562,8 @@ static int vfio_initfn(PCIDevice *pdev) > > trace_vfio_initfn(vdev->vbasedev.name, groupid); > > - group = vfio_get_group(groupid, pci_device_iommu_address_space(pdev)); > + group = vfio_get_group(groupid, pci_device_iommu_address_space(pdev), &err); > if (!group) { > - error_setg(&err, "failed to get group %d", groupid); > ret = -ENOENT; > goto error; > } By now you can see the conversion running step by step like clockwork :) > diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c > index a559e7b..7bf525b 100644 > --- a/hw/vfio/platform.c > +++ b/hw/vfio/platform.c > @@ -552,6 +552,7 @@ static int vfio_base_device_init(VFIODevice *vbasedev) > ssize_t len; > struct stat st; > int groupid; > + Error *err = NULL; > int ret; > > /* @sysfsdev takes precedence over @host */ > @@ -592,10 +593,10 @@ static int vfio_base_device_init(VFIODevice *vbasedev) > > trace_vfio_platform_base_device_init(vbasedev->name, groupid); > > - group = vfio_get_group(groupid, &address_space_memory); > + group = vfio_get_group(groupid, &address_space_memory, &err); > if (!group) { > - error_report("vfio: failed to get group %d", groupid); > - return -ENOENT; > + ret = -ENOENT; > + goto error; > } > > QLIST_FOREACH(vbasedev_iter, &group->device_list, next) { > @@ -619,6 +620,10 @@ static int vfio_base_device_init(VFIODevice *vbasedev) > vfio_put_group(group); > } > > +error: > + if (err) { > + error_reportf_err(err, ERR_PREFIX, vbasedev->name); > + } > return ret; > } Hmm, vfio_base_device_init() is called by realize method vfio_platform_realize(). It should therefore error_propagate() instead of error_reportf_err()! > > diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h > index fd19880..4fb6fc3 100644 > --- a/include/hw/vfio/vfio-common.h > +++ b/include/hw/vfio/vfio-common.h > @@ -155,7 +155,7 @@ void vfio_region_mmaps_set_enabled(VFIORegion *region, bool enabled); > void vfio_region_exit(VFIORegion *region); > void vfio_region_finalize(VFIORegion *region); > void vfio_reset_handler(void *opaque); > -VFIOGroup *vfio_get_group(int groupid, AddressSpace *as); > +VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp); > void vfio_put_group(VFIOGroup *group); > int vfio_get_device(VFIOGroup *group, const char *name, > VFIODevice *vbasedev);
Hi Markus, On 22/09/2016 19:01, Markus Armbruster wrote: > Eric Auger <eric.auger@redhat.com> writes: > >> Pass an error object to prepare for migration to VFIO-PCI realize. >> >> Signed-off-by: Eric Auger <eric.auger@redhat.com> >> >> --- >> >> v2: creation >> --- >> hw/vfio/common.c | 20 +++++++++++--------- >> hw/vfio/pci.c | 3 +-- >> hw/vfio/platform.c | 11 ++++++++--- >> include/hw/vfio/vfio-common.h | 2 +- >> 4 files changed, 21 insertions(+), 15 deletions(-) >> >> diff --git a/hw/vfio/common.c b/hw/vfio/common.c >> index b313e7c..ef9e4cd 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); >> @@ -1115,7 +1116,7 @@ static void vfio_disconnect_container(VFIOGroup *group) >> } >> } >> >> -VFIOGroup *vfio_get_group(int groupid, AddressSpace *as) >> +VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp) >> { >> VFIOGroup *group; >> char path[32]; >> @@ -1127,8 +1128,8 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace *as) >> if (group->container->space->as == as) { >> return group; >> } else { >> - error_report("vfio: group %d used in multiple address spaces", >> - group->groupid); >> + error_setg(errp, "group %d used in multiple address spaces", >> + group->groupid); >> return NULL; >> } >> } >> @@ -1139,19 +1140,20 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace *as) >> snprintf(path, sizeof(path), "/dev/vfio/%d", groupid); >> group->fd = qemu_open(path, O_RDWR); >> if (group->fd < 0) { >> - error_report("vfio: error opening %s: %m", path); >> + error_setg_errno(errp, errno, "error opening %s", path); >> goto free_group_exit; >> } >> >> if (ioctl(group->fd, VFIO_GROUP_GET_STATUS, &status)) { >> - error_report("vfio: error getting group status: %m"); >> + error_setg_errno(errp, errno, "error getting group %d status", groupid); >> goto close_fd_exit; >> } >> >> if (!(status.flags & VFIO_GROUP_FLAGS_VIABLE)) { >> - error_report("vfio: error, group %d is not viable, please ensure " >> - "all devices within the iommu_group are bound to their " >> - "vfio bus driver.", groupid); >> + error_setg(errp, "group %d is not viable", groupid); >> + error_append_hint(errp, >> + "Please ensure all devices within the iommu_group " >> + "are bound to their vfio bus driver.\n"); >> goto close_fd_exit; >> } >> >> @@ -1159,7 +1161,7 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace *as) >> QLIST_INIT(&group->device_list); >> >> if (vfio_connect_container(group, as)) { >> - error_report("vfio: failed to setup container for group %d", groupid); >> + error_setg(errp, "failed to setup container for group %d", groupid); >> goto close_fd_exit; >> } >> >> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c >> index f9a4fe7..1173d4a 100644 >> --- a/hw/vfio/pci.c >> +++ b/hw/vfio/pci.c >> @@ -2562,9 +2562,8 @@ static int vfio_initfn(PCIDevice *pdev) >> >> trace_vfio_initfn(vdev->vbasedev.name, groupid); >> >> - group = vfio_get_group(groupid, pci_device_iommu_address_space(pdev)); >> + group = vfio_get_group(groupid, pci_device_iommu_address_space(pdev), &err); >> if (!group) { >> - error_setg(&err, "failed to get group %d", groupid); >> ret = -ENOENT; >> goto error; >> } > > By now you can see the conversion running step by step like clockwork :) hopefully! > >> diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c >> index a559e7b..7bf525b 100644 >> --- a/hw/vfio/platform.c >> +++ b/hw/vfio/platform.c >> @@ -552,6 +552,7 @@ static int vfio_base_device_init(VFIODevice *vbasedev) >> ssize_t len; >> struct stat st; >> int groupid; >> + Error *err = NULL; >> int ret; >> >> /* @sysfsdev takes precedence over @host */ >> @@ -592,10 +593,10 @@ static int vfio_base_device_init(VFIODevice *vbasedev) >> >> trace_vfio_platform_base_device_init(vbasedev->name, groupid); >> >> - group = vfio_get_group(groupid, &address_space_memory); >> + group = vfio_get_group(groupid, &address_space_memory, &err); >> if (!group) { >> - error_report("vfio: failed to get group %d", groupid); >> - return -ENOENT; >> + ret = -ENOENT; >> + goto error; >> } >> >> QLIST_FOREACH(vbasedev_iter, &group->device_list, next) { >> @@ -619,6 +620,10 @@ static int vfio_base_device_init(VFIODevice *vbasedev) >> vfio_put_group(group); >> } >> >> +error: >> + if (err) { >> + error_reportf_err(err, ERR_PREFIX, vbasedev->name); >> + } >> return ret; >> } > > Hmm, vfio_base_device_init() is called by realize method > vfio_platform_realize(). It should therefore error_propagate() instead > of error_reportf_err()! I added separate patches to properly handle error propagation in vfio_base_device_init up to vfio_platform_realize since error reporting is wrong in the vfio_platform device although realize is in place. Thanks Eric > >> >> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h >> index fd19880..4fb6fc3 100644 >> --- a/include/hw/vfio/vfio-common.h >> +++ b/include/hw/vfio/vfio-common.h >> @@ -155,7 +155,7 @@ void vfio_region_mmaps_set_enabled(VFIORegion *region, bool enabled); >> void vfio_region_exit(VFIORegion *region); >> void vfio_region_finalize(VFIORegion *region); >> void vfio_reset_handler(void *opaque); >> -VFIOGroup *vfio_get_group(int groupid, AddressSpace *as); >> +VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp); >> void vfio_put_group(VFIOGroup *group); >> int vfio_get_device(VFIOGroup *group, const char *name, >> VFIODevice *vbasedev); >
diff --git a/hw/vfio/common.c b/hw/vfio/common.c index b313e7c..ef9e4cd 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); @@ -1115,7 +1116,7 @@ static void vfio_disconnect_container(VFIOGroup *group) } } -VFIOGroup *vfio_get_group(int groupid, AddressSpace *as) +VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp) { VFIOGroup *group; char path[32]; @@ -1127,8 +1128,8 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace *as) if (group->container->space->as == as) { return group; } else { - error_report("vfio: group %d used in multiple address spaces", - group->groupid); + error_setg(errp, "group %d used in multiple address spaces", + group->groupid); return NULL; } } @@ -1139,19 +1140,20 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace *as) snprintf(path, sizeof(path), "/dev/vfio/%d", groupid); group->fd = qemu_open(path, O_RDWR); if (group->fd < 0) { - error_report("vfio: error opening %s: %m", path); + error_setg_errno(errp, errno, "error opening %s", path); goto free_group_exit; } if (ioctl(group->fd, VFIO_GROUP_GET_STATUS, &status)) { - error_report("vfio: error getting group status: %m"); + error_setg_errno(errp, errno, "error getting group %d status", groupid); goto close_fd_exit; } if (!(status.flags & VFIO_GROUP_FLAGS_VIABLE)) { - error_report("vfio: error, group %d is not viable, please ensure " - "all devices within the iommu_group are bound to their " - "vfio bus driver.", groupid); + error_setg(errp, "group %d is not viable", groupid); + error_append_hint(errp, + "Please ensure all devices within the iommu_group " + "are bound to their vfio bus driver.\n"); goto close_fd_exit; } @@ -1159,7 +1161,7 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace *as) QLIST_INIT(&group->device_list); if (vfio_connect_container(group, as)) { - error_report("vfio: failed to setup container for group %d", groupid); + error_setg(errp, "failed to setup container for group %d", groupid); goto close_fd_exit; } diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index f9a4fe7..1173d4a 100644 --- a/hw/vfio/pci.c +++ b/hw/vfio/pci.c @@ -2562,9 +2562,8 @@ static int vfio_initfn(PCIDevice *pdev) trace_vfio_initfn(vdev->vbasedev.name, groupid); - group = vfio_get_group(groupid, pci_device_iommu_address_space(pdev)); + group = vfio_get_group(groupid, pci_device_iommu_address_space(pdev), &err); if (!group) { - error_setg(&err, "failed to get group %d", groupid); ret = -ENOENT; goto error; } diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c index a559e7b..7bf525b 100644 --- a/hw/vfio/platform.c +++ b/hw/vfio/platform.c @@ -552,6 +552,7 @@ static int vfio_base_device_init(VFIODevice *vbasedev) ssize_t len; struct stat st; int groupid; + Error *err = NULL; int ret; /* @sysfsdev takes precedence over @host */ @@ -592,10 +593,10 @@ static int vfio_base_device_init(VFIODevice *vbasedev) trace_vfio_platform_base_device_init(vbasedev->name, groupid); - group = vfio_get_group(groupid, &address_space_memory); + group = vfio_get_group(groupid, &address_space_memory, &err); if (!group) { - error_report("vfio: failed to get group %d", groupid); - return -ENOENT; + ret = -ENOENT; + goto error; } QLIST_FOREACH(vbasedev_iter, &group->device_list, next) { @@ -619,6 +620,10 @@ static int vfio_base_device_init(VFIODevice *vbasedev) vfio_put_group(group); } +error: + if (err) { + error_reportf_err(err, ERR_PREFIX, vbasedev->name); + } return ret; } diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h index fd19880..4fb6fc3 100644 --- a/include/hw/vfio/vfio-common.h +++ b/include/hw/vfio/vfio-common.h @@ -155,7 +155,7 @@ void vfio_region_mmaps_set_enabled(VFIORegion *region, bool enabled); void vfio_region_exit(VFIORegion *region); void vfio_region_finalize(VFIORegion *region); void vfio_reset_handler(void *opaque); -VFIOGroup *vfio_get_group(int groupid, AddressSpace *as); +VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp); void vfio_put_group(VFIOGroup *group); int vfio_get_device(VFIOGroup *group, const char *name, VFIODevice *vbasedev);
Pass an error object to prepare for migration to VFIO-PCI realize. Signed-off-by: Eric Auger <eric.auger@redhat.com> --- v2: creation --- hw/vfio/common.c | 20 +++++++++++--------- hw/vfio/pci.c | 3 +-- hw/vfio/platform.c | 11 ++++++++--- include/hw/vfio/vfio-common.h | 2 +- 4 files changed, 21 insertions(+), 15 deletions(-)