Re: [Qemu-devel] [PATCH v4 11/12] spapr vfio: enable for spapr

[Alex Williamson <alex.williamson@redhat.com>]

On Tue, 2013-09-10 at 19:00 +1000, Alexey Kardashevskiy wrote:
> On 09/06/2013 05:05 AM, Alex Williamson wrote:
> > On Fri, 2013-08-30 at 20:15 +1000, Alexey Kardashevskiy wrote:
> >> This turns the sPAPR support on and enables VFIO container use
> >> in the kernel.
> >>
> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> >> ---
> >> Changes:
> >> v4:
> >> * fixed format string to use %m which is a glibc extension:
> >> "Print output of strerror(errno). No argument is required."
> >> ---
> >>  hw/misc/vfio.c | 30 ++++++++++++++++++++++++++++++
> >>  1 file changed, 30 insertions(+)
> >>
> >> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
> >> index 4210471..882da70 100644
> >> --- a/hw/misc/vfio.c
> >> +++ b/hw/misc/vfio.c
> >> @@ -2815,6 +2815,36 @@ static int vfio_connect_container(VFIOGroup *group, VFIOAddressSpace *space)
> >>  
> >>          memory_listener_register(&container->iommu_data.listener,
> >>                                   container->space->as);
> >> +    } else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU)) {
> >> +        ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &fd);
> >> +        if (ret) {
> >> +            error_report("vfio: failed to set group container: %m");
> >> +            g_free(container);
> >> +            close(fd);
> >> +            return -errno;
> >> +        }
> >> +
> >> +        ret = ioctl(fd, VFIO_SET_IOMMU, VFIO_SPAPR_TCE_IOMMU);
> >> +        if (ret) {
> >> +            error_report("vfio: failed to set iommu for container: %m");
> >> +            g_free(container);
> >> +            close(fd);
> >> +            return -errno;
> >> +        }
> >> +
> >> +        ret = ioctl(fd, VFIO_IOMMU_ENABLE);
> >> +        if (ret) {
> >> +            error_report("vfio: failed to enable container: %m");
> >> +            g_free(container);
> >> +            close(fd);
> >> +            return -errno;
> > 
> > These (and the copies that already exist in this function) are screaming
> > for a goto.
> 
> 
> Heh. So. There should be 2 patches then - one adding gotos to the existing
> code and another one adding new functionality-with-gotos-already.
> I can do that, is it what you suggest?

Yes please.

> What about the rest of the series? Next time I will split "[Qemu-devel]
> [PATCH v4 05/12] spapr_pci: convert init to realize" but the rest will be
> still the same. I have understanding that Alex Graf is expecting you to
> review the whole thing (ack/sob? not sure how this all works) before he
> pulls it into his tree.

Oh, I've been picking out the vfio ones to review.  Ok, on the next
revision I'll review them all, but please still split it into series for
vfio and series for ppc.  Thanks,

Alex