Re: [Qemu-devel] [PATCH 04/18] savevm: set right return value for qemu_file_rate_limit

[Lei Li]

On 08/21/2013 06:42 PM, Paolo Bonzini wrote:
> Il 21/08/2013 09:18, Lei Li ha scritto:
> > Commit 1964a397063967acc5ce71a2a24ed26e74824ee1 refactors rate
> > limiting to QEMUFile, but set the return value for qemu_file_rate_limit
> > to 1 in the case of qemu_file_get_error. It is wrong and should be negative
> > compared to the original function buffered_rate_limit and the current logic
> > in ram_save_iterate. As qemu_file_rate_limit is called manually to determine
> > if it has to exit, add the definition of the meaning of the return values
> > as well.
> When there is an error it returns "time to stop" so that ultimately we
> get to the migration_thread function and check qemu_file_get_error there.
>
> Why doesn't this work for you?

Hi Paolo,

Say, In ram_save_iterate(), the current logic is:

ret = qemu_file_rate_limit();
while(ret == 0) {
    save RAM blocks until no more to send.
}
if (ret < 0) {
    return ret;
}
...

And in savevm layer, qemu_savevm_state_iterate() set an error if the return
value of ram_save_iterate < 0.

Obviously the return value of qemu_file_rate_limit() should have an negative
value for there has been an error. Otherwise we need to modify the logic above.

> Paolo
>
> > Signed-off-by: Lei Li <address@hidden>
> > ---
> >   savevm.c |   14 ++++++++++++--
> >   1 files changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/savevm.c b/savevm.c
> > index 68552a7..6362275 100644
> > --- a/savevm.c
> > +++ b/savevm.c
> > @@ -904,10 +904,20 @@ int64_t qemu_ftell(QEMUFile *f)
> >       return f->pos;
> >   }
> >   
> > +/*
> > + * The meaning of the return values is:
> > + *   0: We can continue sending
> > + *   1: Time to stop
> > + *   negative: There has been an error
> > + */
> > +
> >   int qemu_file_rate_limit(QEMUFile *f)
> >   {
> > -    if (qemu_file_get_error(f)) {
> > -        return 1;
> > +    int ret;
> > +
> > +    ret = qemu_file_get_error(f);
> > +    if (ret) {
> > +        return ret;
> >       }
> >       if (f->xfer_limit > 0 && f->bytes_xfer > f->xfer_limit) {
> >           return 1;


--
Lei