[PATCH v2] libxl: Don't ignore error when we fail to give access to ioport/irq/iomem

[PATCH v2] libxl: Don't ignore error when we fail to give access to ioport/irq/iomem

[Julien Grall]

If we fail to give the access, the domain will unlikely work correctly.
So we should bail out at the first error.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>

---
This patch is candidate for a backport for at least Xen 4.4 and Xen 4.5.

    Changes in v2:
        - Also bail out when libxl fails to give access to iomem
---
 tools/libxl/libxl_create.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 1198225..6f87d1c 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1167,6 +1167,7 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
                  "failed give dom%d access to ioports %"PRIx32"-%"PRIx32,
                  domid, io->first, io->first + io->number - 1);
             ret = ERROR_FAIL;
+            goto error_out;
         }
     }
 
@@ -1182,6 +1183,7 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
         if (ret < 0) {
             LOGE(ERROR, "failed give dom%d access to irq %d", domid, irq);
             ret = ERROR_FAIL;
+            goto error_out;
         }
     }
 
@@ -1198,7 +1200,7 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
                  "failed give dom%d access to iomem range %"PRIx64"-%"PRIx64,
                  domid, io->start, io->start + io->number - 1);
             ret = ERROR_FAIL;
-            continue;
+            goto error_out;
         }
         ret = xc_domain_memory_mapping(CTX->xch, domid,
                                        io->gfn, io->start,
@@ -1209,6 +1211,7 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
                  " to guest address %"PRIx64,
                  domid, io->start, io->start + io->number - 1, io->gfn);
             ret = ERROR_FAIL;
+            goto error_out;
         }
     }
 
-- 
2.1.4

Re: [PATCH v2] libxl: Don't ignore error when we fail to give access to ioport/irq/iomem

[Wei Liu]

On Fri, Jan 09, 2015 at 03:56:45PM +0000, Julien Grall wrote:
> If we fail to give the access, the domain will unlikely work correctly.
> So we should bail out at the first error.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

Re: [PATCH v2] libxl: Don't ignore error when we fail to give access to ioport/irq/iomem

[Ian Campbell]

On Fri, 2015-01-09 at 16:06 +0000, Wei Liu wrote:
> On Fri, Jan 09, 2015 at 03:56:45PM +0000, Julien Grall wrote:
> > If we fail to give the access, the domain will unlikely work correctly.
> > So we should bail out at the first error.
> > 
> > Signed-off-by: Julien Grall <julien.grall@linaro.org>
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > Cc: Ian Campbell <ian.campbell@citrix.com>
> 
> Acked-by: Wei Liu <wei.liu2@citrix.com>

Applied, thanks.

Should this be backported?

Re: [PATCH v2] libxl: Don't ignore error when we fail to give access to ioport/irq/iomem

[Julien Grall]

Hi Ian,

On 12/01/15 16:35, Ian Campbell wrote:
> On Fri, 2015-01-09 at 16:06 +0000, Wei Liu wrote:
>> On Fri, Jan 09, 2015 at 03:56:45PM +0000, Julien Grall wrote:
>>> If we fail to give the access, the domain will unlikely work correctly.
>>> So we should bail out at the first error.
>>>
>>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>>> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>>> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>> Cc: Ian Campbell <ian.campbell@citrix.com>
>>
>> Acked-by: Wei Liu <wei.liu2@citrix.com>
> 
> Applied, thanks.
> 
> Should this be backported?

Yes. But it may not apply directly on Xen 4.4.

Regards,

-- 
Julien Grall

Re: [PATCH v2] libxl: Don't ignore error when we fail to give access to ioport/irq/iomem

[Ian Jackson]

Julien Grall writes ("Re: [PATCH v2] libxl: Don't ignore error when we fail to give access to ioport/irq/iomem"):
> On 12/01/15 16:35, Ian Campbell wrote:
> > Should this be backported?
> 
> Yes. But it may not apply directly on Xen 4.4.

I have made a note of this.  I will apply it to 4.5 after the release
freeze is over and 4.5 is handed over to stable tree maintenance.

As you suspected, it doesn't apply cleanly to 4.4.  I would welcome a
backport.  If no-one else backports it I will do it myself.

Thanks,
Ian.

Re: [PATCH v2] libxl: Don't ignore error when we fail to give access to ioport/irq/iomem

[Julien Grall]

Hi Ian,

On 12/01/15 16:59, Ian Jackson wrote:
> Julien Grall writes ("Re: [PATCH v2] libxl: Don't ignore error when we fail to give access to ioport/irq/iomem"):
>> On 12/01/15 16:35, Ian Campbell wrote:
>>> Should this be backported?
>>
>> Yes. But it may not apply directly on Xen 4.4.
> 
> I have made a note of this.  I will apply it to 4.5 after the release
> freeze is over and 4.5 is handed over to stable tree maintenance.
> 
> As you suspected, it doesn't apply cleanly to 4.4.  I would welcome a
> backport.  If no-one else backports it I will do it myself.

I will give a look to provide a backport for Xen 4.4.

Would it useful to have a backport for Xen 4.3?

Regards,

-- 
Julien Grall

Re: [PATCH v2] libxl: Don't ignore error when we fail to give access to ioport/irq/iomem

[Ian Jackson]

Julien Grall writes ("Re: [PATCH v2] libxl: Don't ignore error when we fail to give access to ioport/irq/iomem"):
> On 12/01/15 16:59, Ian Jackson wrote:
> > As you suspected, it doesn't apply cleanly to 4.4.  I would welcome a
> > backport.  If no-one else backports it I will do it myself.
> 
> I will give a look to provide a backport for Xen 4.4.

Thanks.

> Would it useful to have a backport for Xen 4.3?

Yes.  With luck, though, the 4.4 patch can be cherry-picked onto 4.3.

Ian.

Re: [PATCH v2] libxl: Don't ignore error when we fail to give access to ioport/irq/iomem

[Wei Liu]

On Mon, Jan 12, 2015 at 04:35:19PM +0000, Ian Campbell wrote:
> On Fri, 2015-01-09 at 16:06 +0000, Wei Liu wrote:
> > On Fri, Jan 09, 2015 at 03:56:45PM +0000, Julien Grall wrote:
> > > If we fail to give the access, the domain will unlikely work correctly.
> > > So we should bail out at the first error.
> > > 
> > > Signed-off-by: Julien Grall <julien.grall@linaro.org>
> > > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > Cc: Ian Campbell <ian.campbell@citrix.com>
> > 
> > Acked-by: Wei Liu <wei.liu2@citrix.com>
> 
> Applied, thanks.
> 
> Should this be backported?
> 

Yes. This should be backported.

Wei.