Messages in this thread Patch in this message | | | Subject | Re: [PATCH v2 01/10] KVM: arm/arm64: vgic-its: fix return value for restore | From | Andre Przywara <> | Date | Fri, 6 Oct 2017 15:37:15 +0100 |
| |
Hi,
On 27/09/17 14:28, Eric Auger wrote: > From: wanghaibin <wanghaibin.wang@huawei.com> > > This patch fix the migrate restore tables failure. > > The same scene, at the destination, the restore tables > interface traversal guest memory, and check the dte/ite > is valid or not. If all dtes/ites are invalid, we will do > try next one, and the last it will take the 1 return value, > but currently, it be treated as error. That's not correct. > > This patch try to fix this problem. > > Signed-off-by: wanghaibin <wanghaibin.wang@huawei.com>
Looks right to me. But I wonder if we actually should go over the file and unify the return value semantics or at least document them. It's a bit puzzling to have functions which return negative errors and 0 *or 1* on success, and then functions which go with the traditional C convention. That would help explaining the second hunk.
Also this return value handling is a bit weird in cases, like in handle_l1_dte():
if (ret <= 0) return ret; return 1;
which looks like a glorified "return ret;" in that case to me.
But actually this is just nitpicking and the actual patch seems correct.
Cheers, Andre.
> --- > > need to CC stable > > v1 -> v2: > - if (ret > 0) ret = 0 > --- > virt/kvm/arm/vgic/vgic-its.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c > index f51c1e1..fbbc97b 100644 > --- a/virt/kvm/arm/vgic/vgic-its.c > +++ b/virt/kvm/arm/vgic/vgic-its.c > @@ -2018,7 +2018,7 @@ static int vgic_its_restore_dte(struct vgic_its *its, u32 id, > return PTR_ERR(dev); > > ret = vgic_its_restore_itt(its, dev); > - if (ret) { > + if (ret < 0) { > vgic_its_free_device(its->dev->kvm, dev); > return ret; > } > @@ -2141,7 +2141,7 @@ static int vgic_its_restore_device_tables(struct vgic_its *its) > } > > if (ret > 0) > - ret = -EINVAL; > + ret = 0; > > return ret; > } >
| |