Re: [Qemu-devel] [PATCH -V3 3/4] target-ppc: Check for error on address translation in memsave command

[Luiz Capitulino]

On Mon, 26 Aug 2013 14:22:25 +0200
Andreas Färber <address@hidden> wrote:

> Am 26.08.2013 14:20, schrieb Aneesh Kumar K.V:
> > Andreas Färber <address@hidden> writes:
> > 
> >> Am 25.08.2013 20:32, schrieb Alexander Graf:
> >>>
> >>> On 23.08.2013, at 06:20, Aneesh Kumar K.V wrote:
> >>>
> >>>> From: "Aneesh Kumar K.V" <address@hidden>
> >>>>
> >>>> When we translate the virtual address to physical check for error.
> >>>>
> >>>> Signed-off-by: Aneesh Kumar K.V <address@hidden>
> >>>
> >>> I think this change is sane, but I'd really prefer to see an ack from (or 
> >>> get this applied by) Luiz.
> >>>
> >>>
> >>> Alex
> >>>
> >>>> ---
> >>>> cpus.c | 5 ++++-
> >>>> 1 file changed, 4 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/cpus.c b/cpus.c
> >>>> index 0f65e76..658366d 100644
> >>>> --- a/cpus.c
> >>>> +++ b/cpus.c
> >>>> @@ -1309,7 +1309,10 @@ void qmp_memsave(int64_t addr, int64_t size, 
> >>>> const char *filename,
> >>>>       l = sizeof(buf);
> >>>>       if (l > size)
> >>>>           l = size;
> >>>> -        cpu_memory_rw_debug(cpu, addr, buf, l, 0);
> >>>> +        if (cpu_memory_rw_debug(cpu, addr, buf, l, 0) != 0) {
> >>>> +            error_set(errp, QERR_INVALID_PARAMETER, "addr");
> >>
> >> I've been repeatedly told error_set() should no longer be used, in favor
> >> of error_setg(). :)
> >>
> > 
> > Updated to 
> > 
> > commit 23aa279e11f54808dd9f0f87b3c85e6303d00d9c
> > Author: Aneesh Kumar K.V <address@hidden>
> > Date:   Tue Aug 20 16:14:23 2013 +0530
> > 
> >     target-ppc: Check for error on address translation in memsave command
> >     
> >     When we translate the virtual address to physical check for error.
> >     
> >     Signed-off-by: Aneesh Kumar K.V <address@hidden>
> > 
> > diff --git a/cpus.c b/cpus.c
> > index 0f65e76..51c38a0 100644
> > --- a/cpus.c
> > +++ b/cpus.c
> > @@ -1309,7 +1309,10 @@ void qmp_memsave(int64_t addr, int64_t size, const 
> > char *filename,
> >          l = sizeof(buf);
> >          if (l > size)
> >              l = size;
> > -        cpu_memory_rw_debug(cpu, addr, buf, l, 0);
> > +        if (cpu_memory_rw_debug(cpu, addr, buf, l, 0) != 0) {
> > +            error_setg(errp, "Invalid addr 0x%016" PRIx64 "specified\n", 
> > addr);
> 
> Next trap is no \n please. ;)

Otherwise looks good.