* [Qemu-devel] [PATCH resend 1/3] savevm: add comments for qemu_file_get_error()
@ 2013-08-23 13:30 Lei Li
2013-08-23 13:30 ` [Qemu-devel] [PATCH resend 2/3] savevm: fix wrong error set by ram_control_load_hook() Lei Li
2013-08-23 13:30 ` [Qemu-devel] [PATCH resend 3/3] arch_init: right return for ram_save_iterate Lei Li
0 siblings, 2 replies; 7+ messages in thread
From: Lei Li @ 2013-08-23 13:30 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, mrhines, anthony, Lei Li
Add comments for qemu_file_get_error(), as its return value
is not very clear.
Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
savevm.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/savevm.c b/savevm.c
index 03fc4d9..95a11f9 100644
--- a/savevm.c
+++ b/savevm.c
@@ -566,6 +566,13 @@ QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops)
return f;
}
+/*
+ * Get last error for stream f
+ *
+ * Return negative error value if there has been an error on previous
+ * operations, return 0 if no error happened.
+ *
+ */
int qemu_file_get_error(QEMUFile *f)
{
return f->last_error;
--
1.7.7.6
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH resend 2/3] savevm: fix wrong error set by ram_control_load_hook()
2013-08-23 13:30 [Qemu-devel] [PATCH resend 1/3] savevm: add comments for qemu_file_get_error() Lei Li
@ 2013-08-23 13:30 ` Lei Li
2013-08-23 14:18 ` Michael R. Hines
2013-08-23 14:18 ` Michael R. Hines
2013-08-23 13:30 ` [Qemu-devel] [PATCH resend 3/3] arch_init: right return for ram_save_iterate Lei Li
1 sibling, 2 replies; 7+ messages in thread
From: Lei Li @ 2013-08-23 13:30 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, mrhines, anthony, Lei Li
It should set negative error value if there has been an error.
Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
savevm.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/savevm.c b/savevm.c
index 95a11f9..a0be109 100644
--- a/savevm.c
+++ b/savevm.c
@@ -649,7 +649,7 @@ void ram_control_after_iterate(QEMUFile *f, uint64_t flags)
void ram_control_load_hook(QEMUFile *f, uint64_t flags)
{
- int ret = 0;
+ int ret = -EINVAL;
if (f->ops->hook_ram_load) {
ret = f->ops->hook_ram_load(f, f->opaque, flags);
--
1.7.7.6
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH resend 3/3] arch_init: right return for ram_save_iterate
2013-08-23 13:30 [Qemu-devel] [PATCH resend 1/3] savevm: add comments for qemu_file_get_error() Lei Li
2013-08-23 13:30 ` [Qemu-devel] [PATCH resend 2/3] savevm: fix wrong error set by ram_control_load_hook() Lei Li
@ 2013-08-23 13:30 ` Lei Li
2013-08-26 8:27 ` Paolo Bonzini
1 sibling, 1 reply; 7+ messages in thread
From: Lei Li @ 2013-08-23 13:30 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, mrhines, anthony, Lei Li
Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
arch_init.c | 7 +------
1 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 94d45e1..a34437c 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -709,16 +709,11 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
*/
ram_control_after_iterate(f, RAM_CONTROL_ROUND);
- if (ret < 0) {
- bytes_transferred += total_sent;
- return ret;
- }
-
qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
total_sent += 8;
bytes_transferred += total_sent;
- return total_sent;
+ return qemu_file_get_error(f);
}
static int ram_save_complete(QEMUFile *f, void *opaque)
--
1.7.7.6
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH resend 2/3] savevm: fix wrong error set by ram_control_load_hook()
2013-08-23 13:30 ` [Qemu-devel] [PATCH resend 2/3] savevm: fix wrong error set by ram_control_load_hook() Lei Li
@ 2013-08-23 14:18 ` Michael R. Hines
2013-08-23 14:18 ` Michael R. Hines
1 sibling, 0 replies; 7+ messages in thread
From: Michael R. Hines @ 2013-08-23 14:18 UTC (permalink / raw)
To: Lei Li; +Cc: pbonzini, qemu-devel, anthony
On 08/23/2013 09:30 AM, Lei Li wrote:
> It should set negative error value if there has been an error.
>
> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> ---
> savevm.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/savevm.c b/savevm.c
> index 95a11f9..a0be109 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -649,7 +649,7 @@ void ram_control_after_iterate(QEMUFile *f, uint64_t flags)
>
> void ram_control_load_hook(QEMUFile *f, uint64_t flags)
> {
> - int ret = 0;
> + int ret = -EINVAL;
>
> if (f->ops->hook_ram_load) {
> ret = f->ops->hook_ram_load(f, f->opaque, flags);
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH resend 2/3] savevm: fix wrong error set by ram_control_load_hook()
2013-08-23 13:30 ` [Qemu-devel] [PATCH resend 2/3] savevm: fix wrong error set by ram_control_load_hook() Lei Li
2013-08-23 14:18 ` Michael R. Hines
@ 2013-08-23 14:18 ` Michael R. Hines
1 sibling, 0 replies; 7+ messages in thread
From: Michael R. Hines @ 2013-08-23 14:18 UTC (permalink / raw)
To: Lei Li; +Cc: pbonzini, qemu-devel, anthony
On 08/23/2013 09:30 AM, Lei Li wrote:
> It should set negative error value if there has been an error.
>
> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> ---
> savevm.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/savevm.c b/savevm.c
> index 95a11f9..a0be109 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -649,7 +649,7 @@ void ram_control_after_iterate(QEMUFile *f, uint64_t flags)
>
> void ram_control_load_hook(QEMUFile *f, uint64_t flags)
> {
> - int ret = 0;
> + int ret = -EINVAL;
>
> if (f->ops->hook_ram_load) {
> ret = f->ops->hook_ram_load(f, f->opaque, flags);
Reviewed-By: Michael R. Hines <mrhines@us.ibm.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH resend 3/3] arch_init: right return for ram_save_iterate
2013-08-23 13:30 ` [Qemu-devel] [PATCH resend 3/3] arch_init: right return for ram_save_iterate Lei Li
@ 2013-08-26 8:27 ` Paolo Bonzini
2013-08-26 9:22 ` Lei Li
0 siblings, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2013-08-26 8:27 UTC (permalink / raw)
To: Lei Li; +Cc: mrhines, qemu-devel, anthony
Il 23/08/2013 15:30, Lei Li ha scritto:
> - if (ret < 0) {
> - bytes_transferred += total_sent;
> - return ret;
> - }
> -
> qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
> total_sent += 8;
> bytes_transferred += total_sent;
>
> - return total_sent;
> + return qemu_file_get_error(f);
No, this will never make ram_save_iterate (and thus
qemu_savevm_state_iterate) return a positive, non-zero value. Thus:
ret = qemu_file_get_error(f);
if (ret < 0) {
return ret;
}
return total_sent;
If you look at the code, you can see that it never returns zero.
Probably it should do something like
bytes_transferred += total_sent;
/* Do not count these 8 bytes into total_sent, so that we can
* return 0 if no page had been dirtied.
*/
qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
bytes_transferred += 8;
and then proceed as above with "ret = qemu_file_get_error(f)".
Paolo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH resend 3/3] arch_init: right return for ram_save_iterate
2013-08-26 8:27 ` Paolo Bonzini
@ 2013-08-26 9:22 ` Lei Li
0 siblings, 0 replies; 7+ messages in thread
From: Lei Li @ 2013-08-26 9:22 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: anthony, qemu-devel, mrhines
On 08/26/2013 04:27 PM, Paolo Bonzini wrote:
> Il 23/08/2013 15:30, Lei Li ha scritto:
>> - if (ret < 0) {
>> - bytes_transferred += total_sent;
>> - return ret;
>> - }
>> -
>> qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
>> total_sent += 8;
>> bytes_transferred += total_sent;
>>
>> - return total_sent;
>> + return qemu_file_get_error(f);
> No, this will never make ram_save_iterate (and thus
> qemu_savevm_state_iterate) return a positive, non-zero value. Thus:
>
> ret = qemu_file_get_error(f);
> if (ret < 0) {
> return ret;
> }
> return total_sent;
>
> If you look at the code, you can see that it never returns zero.
> Probably it should do something like
>
> bytes_transferred += total_sent;
>
> /* Do not count these 8 bytes into total_sent, so that we can
> * return 0 if no page had been dirtied.
> */
> qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
> bytes_transferred += 8;
>
> and then proceed as above with "ret = qemu_file_get_error(f)".
Yes, you are right.
>
> Paolo
>
--
Lei
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-08-26 9:23 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-23 13:30 [Qemu-devel] [PATCH resend 1/3] savevm: add comments for qemu_file_get_error() Lei Li
2013-08-23 13:30 ` [Qemu-devel] [PATCH resend 2/3] savevm: fix wrong error set by ram_control_load_hook() Lei Li
2013-08-23 14:18 ` Michael R. Hines
2013-08-23 14:18 ` Michael R. Hines
2013-08-23 13:30 ` [Qemu-devel] [PATCH resend 3/3] arch_init: right return for ram_save_iterate Lei Li
2013-08-26 8:27 ` Paolo Bonzini
2013-08-26 9:22 ` Lei Li
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.