Re: [Qemu-devel] [PATCH v5 08/17] vfio: Pass an Error object to vfio_connect_container

[David Gibson]

On Mon, Oct 10, 2016 at 02:36:28PM +0200, Markus Armbruster wrote:
> Auger Eric  writes:
> 
> > Hi,
> >
> > On 10/10/2016 07:34, David Gibson wrote:
> >> On Fri, Oct 07, 2016 at 09:36:09AM +0200, Auger Eric wrote:
> >>> Hi,
> >>>
> >>> On 07/10/2016 09:01, Markus Armbruster wrote:
>  Eric Auger  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 
> > Reviewed-by: Markus Armbruster 
> >
> > ---
> >
> > 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
> [...]
> > @@ -1008,7 +1010,9 @@ static int vfio_connect_container(VFIOGroup 
> > *group, AddressSpace *as)
>container = g_malloc0(sizeof(*container));
>container->space = space;
>container->fd = fd;
>if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1_IOMMU) ||
>ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1v2_IOMMU)) {
> [...]
>} else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU) ||
>   ioctl(fd, VFIO_CHECK_EXTENSION, 
> VFIO_SPAPR_TCE_v2_IOMMU)) {
>struct vfio_iommu_spapr_tce_info info;
>bool v2 = !!ioctl(fd, VFIO_CHECK_EXTENSION, 
> VFIO_SPAPR_TCE_v2_IOMMU);
> 
>ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, );
>if (ret) {
>error_setg_errno(errp, errno, "failed to set group 
> container");
>ret = -errno;
>goto free_container_exit;
>}
>container->iommu_type =
>v2 ? VFIO_SPAPR_TCE_v2_IOMMU : VFIO_SPAPR_TCE_IOMMU;
>ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);
>if (ret) {
>error_setg_errno(errp, errno, "failed to set iommu for 
> container");
>ret = -errno;
>goto free_container_exit;
>}
> 
>/*
> * The host kernel code implementing VFIO_IOMMU_DISABLE is 
> called
> * when container fd is closed so we do not call it explicitly
> * in this file.
> */
>if (!v2) {
>ret = ioctl(fd, VFIO_IOMMU_ENABLE);
>if (ret) {
>error_setg_errno(errp, errno, "failed to enable 
> container");
>ret = -errno;
>goto free_container_exit;
>}
>} else {
>container->prereg_listener = vfio_prereg_listener;
> 
>memory_listener_register(>prereg_listener,
> >   _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 ...
> >>>
> >>> David, Alexey, could you confirm we should set the returned value to the
> >>> container->error below?
> >> 
> >> I think the right approach is to change container->error from an int
> >> to an Error *.  As now, we stash the first error from the listener in
> >> there.
> >> 
> >> realize() would check for a non-NULL error in the container after
> >> registering the listener, and if present, propagate it up to the
> >> caller.
> >> 
> >>>
> >>> Thanks
> >>>
> >>> Eric
> 

Re: [Qemu-devel] [PATCH v5 08/17] vfio: Pass an Error object to vfio_connect_container

[Markus Armbruster]

David Gibson  writes:

> On Mon, Oct 10, 2016 at 03:21:24PM +0200, Auger Eric wrote:
>> Hi Markus,
>> On 10/10/2016 14:36, Markus Armbruster wrote:
>> > Auger Eric  writes:
>> > 
>> >> Hi,
>> >>
>> >> On 10/10/2016 07:34, David Gibson wrote:
>> >>> On Fri, Oct 07, 2016 at 09:36:09AM +0200, Auger Eric wrote:
>>  Hi,
>> 
>>  On 07/10/2016 09:01, Markus Armbruster wrote:
>> > Eric Auger  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 
>> >> Reviewed-by: Markus Armbruster 
>> >>
>> >> ---
>> >>
>> >> 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
>> > [...]
>> >> @@ -1008,7 +1010,9 @@ static int vfio_connect_container(VFIOGroup 
>> >> *group, AddressSpace *as)
>> >container = g_malloc0(sizeof(*container));
>> >container->space = space;
>> >container->fd = fd;
>> >if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1_IOMMU) ||
>> >ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1v2_IOMMU)) {
>> > [...]
>> >} else if (ioctl(fd, VFIO_CHECK_EXTENSION, 
>> > VFIO_SPAPR_TCE_IOMMU) ||
>> >   ioctl(fd, VFIO_CHECK_EXTENSION, 
>> > VFIO_SPAPR_TCE_v2_IOMMU)) {
>> >struct vfio_iommu_spapr_tce_info info;
>> >bool v2 = !!ioctl(fd, VFIO_CHECK_EXTENSION, 
>> > VFIO_SPAPR_TCE_v2_IOMMU);
>> > 
>> >ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, );
>> >if (ret) {
>> >error_setg_errno(errp, errno, "failed to set group 
>> > container");
>> >ret = -errno;
>> >goto free_container_exit;
>> >}
>> >container->iommu_type =
>> >v2 ? VFIO_SPAPR_TCE_v2_IOMMU : VFIO_SPAPR_TCE_IOMMU;
>> >ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);
>> >if (ret) {
>> >error_setg_errno(errp, errno, "failed to set iommu for 
>> > container");
>> >ret = -errno;
>> >goto free_container_exit;
>> >}
>> > 
>> >/*
>> > * The host kernel code implementing VFIO_IOMMU_DISABLE is 
>> > called
>> > * when container fd is closed so we do not call it 
>> > explicitly
>> > * in this file.
>> > */
>> >if (!v2) {
>> >ret = ioctl(fd, VFIO_IOMMU_ENABLE);
>> >if (ret) {
>> >error_setg_errno(errp, errno, "failed to enable 
>> > container");
>> >ret = -errno;
>> >goto free_container_exit;
>> >}
>> >} else {
>> >container->prereg_listener = vfio_prereg_listener;
>> > 
>> >memory_listener_register(>prereg_listener,
>> >>   _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 ...
>> 
>>  David, Alexey, could you confirm we should set the 

Re: [Qemu-devel] [PATCH v5 08/17] vfio: Pass an Error object to vfio_connect_container

[David Gibson]

On Mon, Oct 10, 2016 at 03:21:24PM +0200, Auger Eric wrote:
> Hi Markus,
> On 10/10/2016 14:36, Markus Armbruster wrote:
> > Auger Eric  writes:
> > 
> >> Hi,
> >>
> >> On 10/10/2016 07:34, David Gibson wrote:
> >>> On Fri, Oct 07, 2016 at 09:36:09AM +0200, Auger Eric wrote:
>  Hi,
> 
>  On 07/10/2016 09:01, Markus Armbruster wrote:
> > Eric Auger  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 
> >> Reviewed-by: Markus Armbruster 
> >>
> >> ---
> >>
> >> 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
> > [...]
> >> @@ -1008,7 +1010,9 @@ static int vfio_connect_container(VFIOGroup 
> >> *group, AddressSpace *as)
> >container = g_malloc0(sizeof(*container));
> >container->space = space;
> >container->fd = fd;
> >if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1_IOMMU) ||
> >ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1v2_IOMMU)) {
> > [...]
> >} else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU) 
> > ||
> >   ioctl(fd, VFIO_CHECK_EXTENSION, 
> > VFIO_SPAPR_TCE_v2_IOMMU)) {
> >struct vfio_iommu_spapr_tce_info info;
> >bool v2 = !!ioctl(fd, VFIO_CHECK_EXTENSION, 
> > VFIO_SPAPR_TCE_v2_IOMMU);
> > 
> >ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, );
> >if (ret) {
> >error_setg_errno(errp, errno, "failed to set group 
> > container");
> >ret = -errno;
> >goto free_container_exit;
> >}
> >container->iommu_type =
> >v2 ? VFIO_SPAPR_TCE_v2_IOMMU : VFIO_SPAPR_TCE_IOMMU;
> >ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);
> >if (ret) {
> >error_setg_errno(errp, errno, "failed to set iommu for 
> > container");
> >ret = -errno;
> >goto free_container_exit;
> >}
> > 
> >/*
> > * The host kernel code implementing VFIO_IOMMU_DISABLE is 
> > called
> > * when container fd is closed so we do not call it 
> > explicitly
> > * in this file.
> > */
> >if (!v2) {
> >ret = ioctl(fd, VFIO_IOMMU_ENABLE);
> >if (ret) {
> >error_setg_errno(errp, errno, "failed to enable 
> > container");
> >ret = -errno;
> >goto free_container_exit;
> >}
> >} else {
> >container->prereg_listener = vfio_prereg_listener;
> > 
> >memory_listener_register(>prereg_listener,
> >>   _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 ...
> 
>  David, Alexey, could you confirm we should set the returned value to the
>  container->error below?
> >>>
> >>> I think the right approach is to change container->error from an int
> >>> to an Error *.  As now, we stash the first 

Re: [Qemu-devel] [PATCH v5 08/17] vfio: Pass an Error object to vfio_connect_container

[Auger Eric]

Hi Markus,
On 10/10/2016 14:36, Markus Armbruster wrote:
> Auger Eric  writes:
> 
>> Hi,
>>
>> On 10/10/2016 07:34, David Gibson wrote:
>>> On Fri, Oct 07, 2016 at 09:36:09AM +0200, Auger Eric wrote:
 Hi,

 On 07/10/2016 09:01, Markus Armbruster wrote:
> Eric Auger  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 
>> Reviewed-by: Markus Armbruster 
>>
>> ---
>>
>> 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
> [...]
>> @@ -1008,7 +1010,9 @@ static int vfio_connect_container(VFIOGroup 
>> *group, AddressSpace *as)
>container = g_malloc0(sizeof(*container));
>container->space = space;
>container->fd = fd;
>if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1_IOMMU) ||
>ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1v2_IOMMU)) {
> [...]
>} else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU) ||
>   ioctl(fd, VFIO_CHECK_EXTENSION, 
> VFIO_SPAPR_TCE_v2_IOMMU)) {
>struct vfio_iommu_spapr_tce_info info;
>bool v2 = !!ioctl(fd, VFIO_CHECK_EXTENSION, 
> VFIO_SPAPR_TCE_v2_IOMMU);
> 
>ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, );
>if (ret) {
>error_setg_errno(errp, errno, "failed to set group 
> container");
>ret = -errno;
>goto free_container_exit;
>}
>container->iommu_type =
>v2 ? VFIO_SPAPR_TCE_v2_IOMMU : VFIO_SPAPR_TCE_IOMMU;
>ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);
>if (ret) {
>error_setg_errno(errp, errno, "failed to set iommu for 
> container");
>ret = -errno;
>goto free_container_exit;
>}
> 
>/*
> * The host kernel code implementing VFIO_IOMMU_DISABLE is 
> called
> * when container fd is closed so we do not call it explicitly
> * in this file.
> */
>if (!v2) {
>ret = ioctl(fd, VFIO_IOMMU_ENABLE);
>if (ret) {
>error_setg_errno(errp, errno, "failed to enable 
> container");
>ret = -errno;
>goto free_container_exit;
>}
>} else {
>container->prereg_listener = vfio_prereg_listener;
> 
>memory_listener_register(>prereg_listener,
>>   _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 ...

 David, Alexey, could you confirm we should set the returned value to the
 container->error below?
>>>
>>> I think the right approach is to change container->error from an int
>>> to an Error *.  As now, we stash the first error from the listener in
>>> there.
>>>
>>> realize() would check for a non-NULL error in the container after
>>> registering the listener, and if present, propagate it up to the
>>> caller.
>>>

 Thanks

 Eric


>
>>  memory_listener_unregister(>prereg_listener);
>> -

Re: [Qemu-devel] [PATCH v5 08/17] vfio: Pass an Error object to vfio_connect_container

[Markus Armbruster]

Auger Eric  writes:

> Hi,
>
> On 10/10/2016 07:34, David Gibson wrote:
>> On Fri, Oct 07, 2016 at 09:36:09AM +0200, Auger Eric wrote:
>>> Hi,
>>>
>>> On 07/10/2016 09:01, Markus Armbruster wrote:
 Eric Auger  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 
> Reviewed-by: Markus Armbruster 
>
> ---
>
> 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
[...]
> @@ -1008,7 +1010,9 @@ static int vfio_connect_container(VFIOGroup *group, 
> AddressSpace *as)
   container = g_malloc0(sizeof(*container));
   container->space = space;
   container->fd = fd;
   if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1_IOMMU) ||
   ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1v2_IOMMU)) {
[...]
   } else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU) ||
  ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_v2_IOMMU)) 
{
   struct vfio_iommu_spapr_tce_info info;
   bool v2 = !!ioctl(fd, VFIO_CHECK_EXTENSION, 
VFIO_SPAPR_TCE_v2_IOMMU);

   ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, );
   if (ret) {
   error_setg_errno(errp, errno, "failed to set group 
container");
   ret = -errno;
   goto free_container_exit;
   }
   container->iommu_type =
   v2 ? VFIO_SPAPR_TCE_v2_IOMMU : VFIO_SPAPR_TCE_IOMMU;
   ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);
   if (ret) {
   error_setg_errno(errp, errno, "failed to set iommu for 
container");
   ret = -errno;
   goto free_container_exit;
   }

   /*
* The host kernel code implementing VFIO_IOMMU_DISABLE is called
* when container fd is closed so we do not call it explicitly
* in this file.
*/
   if (!v2) {
   ret = ioctl(fd, VFIO_IOMMU_ENABLE);
   if (ret) {
   error_setg_errno(errp, errno, "failed to enable 
container");
   ret = -errno;
   goto free_container_exit;
   }
   } else {
   container->prereg_listener = vfio_prereg_listener;

   memory_listener_register(>prereg_listener,
>   _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 ...
>>>
>>> David, Alexey, could you confirm we should set the returned value to the
>>> container->error below?
>> 
>> I think the right approach is to change container->error from an int
>> to an Error *.  As now, we stash the first error from the listener in
>> there.
>> 
>> realize() would check for a non-NULL error in the container after
>> registering the listener, and if present, propagate it up to the
>> caller.
>> 
>>>
>>> Thanks
>>>
>>> Eric
>>>
>>>

>  memory_listener_unregister(>prereg_listener);
> -error_report("vfio: RAM memory listener initialization 
> failed for container");
> +ret = container->error;
> Thank you for your answers. OK to change container->error from an int
> to an Error *.
>
> So 

Re: [Qemu-devel] [PATCH v5 08/17] vfio: Pass an Error object to vfio_connect_container

[Auger Eric]

Hi,

On 10/10/2016 07:34, David Gibson wrote:
> On Fri, Oct 07, 2016 at 09:36:09AM +0200, Auger Eric wrote:
>> Hi,
>>
>> On 07/10/2016 09:01, Markus Armbruster wrote:
>>> Eric Auger  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 
 Reviewed-by: Markus Armbruster 

 ---

 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, );
  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, );
  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) {
 -

Re: [Qemu-devel] [PATCH v5 08/17] vfio: Pass an Error object to vfio_connect_container

[David Gibson]

On Fri, Oct 07, 2016 at 09:36:09AM +0200, Auger Eric wrote:
> Hi,
> 
> On 07/10/2016 09:01, Markus Armbruster wrote:
> > Eric Auger  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 
> >> Reviewed-by: Markus Armbruster 
> >>
> >> ---
> >>
> >> 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, );
> >>  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, );
> >>  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");
> 

Re: [Qemu-devel] [PATCH v5 08/17] vfio: Pass an Error object to vfio_connect_container

[Alexey Kardashevskiy]

On 07/10/16 18:36, Auger Eric wrote:
> Hi,
> 
> On 07/10/2016 09:01, Markus Armbruster wrote:
>> Eric Auger  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 
>>> Reviewed-by: Markus Armbruster 
>>>
>>> ---
>>>
>>> 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, );
>>>  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, );
>>>  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 

Re: [Qemu-devel] [PATCH v5 08/17] vfio: Pass an Error object to vfio_connect_container

[Auger Eric]

Hi,

On 07/10/2016 09:01, Markus Armbruster wrote:
> Eric Auger  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 
>> Reviewed-by: Markus Armbruster 
>>
>> ---
>>
>> 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, );
>>  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, );
>>  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)
>>   

Re: [Qemu-devel] [PATCH v5 08/17] vfio: Pass an Error object to vfio_connect_container

[Markus Armbruster]

Eric Auger  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.

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 
> Reviewed-by: Markus Armbruster 
>
> ---
>
> 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, );
>  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, );
>  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)
>   _space_memory);
>  if (container->error) {

I tried to see where non-zero container->error comes from, but failed.
Can you help?

>  memory_listener_unregister(>prereg_listener);
> -error_report("vfio: