| Message ID | 1374411399-17535-1-git-send-email-lilei@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
In the logic of ram_save_iterate(), it checks the ret of qemu_file_rate_limit(f) by ret < 0 if there has been an error. But now it will never return negative value because qemu_file_rate_limit() return 1 if qemu_file_get_error(). Also the original implementation of qemu_file_rate_limit() as function buffered_rate_limit() did return negative for this situation. On 07/21/2013 08:56 PM, Lei Li wrote: > 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 defination of the meaning of the return values > as well. > > > Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com> > --- > savevm.c | 14 ++++++++++++-- > 1 files changed, 12 insertions(+), 2 deletions(-) > > diff --git a/savevm.c b/savevm.c > index e0491e7..f406790 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;
PING? any comments? On 07/24/2013 11:48 PM, Lei Li wrote: > In the logic of ram_save_iterate(), it checks the ret of > qemu_file_rate_limit(f) > by ret < 0 if there has been an error. But now it will never return > negative value > because qemu_file_rate_limit() return 1 if qemu_file_get_error(). > > Also the original implementation of qemu_file_rate_limit() as function > buffered_rate_limit() did return negative for this situation. > > > On 07/21/2013 08:56 PM, Lei Li wrote: >> 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 defination of the meaning of the return >> values >> as well. >> >> >> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com> >> --- >> savevm.c | 14 ++++++++++++-- >> 1 files changed, 12 insertions(+), 2 deletions(-) >> >> diff --git a/savevm.c b/savevm.c >> index e0491e7..f406790 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; > >
diff --git a/savevm.c b/savevm.c index e0491e7..f406790 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;
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 defination of the meaning of the return values as well. Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com> --- savevm.c | 14 ++++++++++++-- 1 files changed, 12 insertions(+), 2 deletions(-)