error: Eliminate QERR_IO_ERROR

[PATCH 2/6] dump/win_dump: Improve error messages on write error

Posted by Markus Armbruster 1 year, 9 months ago

create_win_dump() and write_run report qemu_write_full() failure to
their callers as

    An IO error has occurred

The errno set by qemu_write_full() is lost.

Improve this to

    win-dump: failed to write header: <description of errno>

and

    win-dump: failed to save memory: <description of errno>

This matches how dump.c reports similar errors.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 dump/win_dump.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/dump/win_dump.c b/dump/win_dump.c
index b7bfaff379..0e4fe692ce 100644
--- a/dump/win_dump.c
+++ b/dump/win_dump.c
@@ -12,7 +12,6 @@
 #include "sysemu/dump.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
-#include "qapi/qmp/qerror.h"
 #include "exec/cpu-defs.h"
 #include "hw/core/cpu.h"
 #include "qemu/win_dump_defs.h"
@@ -52,6 +51,7 @@ static size_t write_run(uint64_t base_page, uint64_t page_count,
     uint64_t addr = base_page << TARGET_PAGE_BITS;
     uint64_t size = page_count << TARGET_PAGE_BITS;
     uint64_t len, l;
+    int eno;
     size_t total = 0;
 
     while (size) {
@@ -65,9 +65,10 @@ static size_t write_run(uint64_t base_page, uint64_t page_count,
         }
 
         l = qemu_write_full(fd, buf, len);
+        eno = errno;
         cpu_physical_memory_unmap(buf, addr, false, len);
         if (l != len) {
-            error_setg(errp, QERR_IO_ERROR);
+            error_setg_errno(errp, eno, "win-dump: failed to save memory");
             return 0;
         }
 
@@ -459,7 +460,7 @@ void create_win_dump(DumpState *s, Error **errp)
 
     s->written_size = qemu_write_full(s->fd, h, hdr_size);
     if (s->written_size != hdr_size) {
-        error_setg(errp, QERR_IO_ERROR);
+        error_setg_errno(errp, errno, "win-dump: failed to write header");
         goto out_restore;
     }
 
-- 
2.45.0

Re: [PATCH 2/6] dump/win_dump: Improve error messages on write error

Posted by Philippe Mathieu-Daudé 1 year, 9 months ago

On 13/5/24 16:16, Markus Armbruster wrote:
> create_win_dump() and write_run report qemu_write_full() failure to
> their callers as
> 
>      An IO error has occurred
> 
> The errno set by qemu_write_full() is lost.
> 
> Improve this to
> 
>      win-dump: failed to write header: <description of errno>
> 
> and
> 
>      win-dump: failed to save memory: <description of errno>
> 
> This matches how dump.c reports similar errors.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   dump/win_dump.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/dump/win_dump.c b/dump/win_dump.c
> index b7bfaff379..0e4fe692ce 100644
> --- a/dump/win_dump.c
> +++ b/dump/win_dump.c
> @@ -12,7 +12,6 @@
>   #include "sysemu/dump.h"
>   #include "qapi/error.h"
>   #include "qemu/error-report.h"
> -#include "qapi/qmp/qerror.h"
>   #include "exec/cpu-defs.h"
>   #include "hw/core/cpu.h"
>   #include "qemu/win_dump_defs.h"
> @@ -52,6 +51,7 @@ static size_t write_run(uint64_t base_page, uint64_t page_count,
>       uint64_t addr = base_page << TARGET_PAGE_BITS;
>       uint64_t size = page_count << TARGET_PAGE_BITS;
>       uint64_t len, l;
> +    int eno;
>       size_t total = 0;
>   
>       while (size) {
> @@ -65,9 +65,10 @@ static size_t write_run(uint64_t base_page, uint64_t page_count,
>           }
>   
>           l = qemu_write_full(fd, buf, len);
> +        eno = errno;

Hmm this show the qemu_write_full() API isn't ideal.
Maybe we could pass &l as argument and return errno.
There are only 20 calls.

>           cpu_physical_memory_unmap(buf, addr, false, len);
>           if (l != len) {
> -            error_setg(errp, QERR_IO_ERROR);
> +            error_setg_errno(errp, eno, "win-dump: failed to save memory");
>               return 0;
>           }
>   
> @@ -459,7 +460,7 @@ void create_win_dump(DumpState *s, Error **errp)
>   
>       s->written_size = qemu_write_full(s->fd, h, hdr_size);
>       if (s->written_size != hdr_size) {
> -        error_setg(errp, QERR_IO_ERROR);
> +        error_setg_errno(errp, errno, "win-dump: failed to write header");
>           goto out_restore;
>       }
>

Re: [PATCH 2/6] dump/win_dump: Improve error messages on write error

Posted by Markus Armbruster 1 year, 9 months ago

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> On 13/5/24 16:16, Markus Armbruster wrote:
>> create_win_dump() and write_run report qemu_write_full() failure to
>> their callers as
>>      An IO error has occurred
>> The errno set by qemu_write_full() is lost.
>> Improve this to
>>      win-dump: failed to write header: <description of errno>
>> and
>>      win-dump: failed to save memory: <description of errno>
>> This matches how dump.c reports similar errors.
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   dump/win_dump.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>> diff --git a/dump/win_dump.c b/dump/win_dump.c
>> index b7bfaff379..0e4fe692ce 100644
>> --- a/dump/win_dump.c
>> +++ b/dump/win_dump.c
>> @@ -12,7 +12,6 @@
>>   #include "sysemu/dump.h"
>>   #include "qapi/error.h"
>>   #include "qemu/error-report.h"
>> -#include "qapi/qmp/qerror.h"
>>   #include "exec/cpu-defs.h"
>>   #include "hw/core/cpu.h"
>>   #include "qemu/win_dump_defs.h"
>> @@ -52,6 +51,7 @@ static size_t write_run(uint64_t base_page, uint64_t page_count,
>>       uint64_t addr = base_page << TARGET_PAGE_BITS;
>>       uint64_t size = page_count << TARGET_PAGE_BITS;
>>       uint64_t len, l;
>> +    int eno;
>>       size_t total = 0;
>>         while (size) {
>> @@ -65,9 +65,10 @@ static size_t write_run(uint64_t base_page, uint64_t page_count,
>>           }
>>             l = qemu_write_full(fd, buf, len);
>> +        eno = errno;
>
> Hmm this show the qemu_write_full() API isn't ideal.
> Maybe we could pass &l as argument and return errno.
> There are only 20 calls.

qemu_write_full() is a drop-in replacement for write().

>>           cpu_physical_memory_unmap(buf, addr, false, len);
>>           if (l != len) {
>> -            error_setg(errp, QERR_IO_ERROR);
>> +            error_setg_errno(errp, eno, "win-dump: failed to save memory");
>>               return 0;
>>           }
>>   @@ -459,7 +460,7 @@ void create_win_dump(DumpState *s, Error **errp)
>>         s->written_size = qemu_write_full(s->fd, h, hdr_size);
>>       if (s->written_size != hdr_size) {
>> -        error_setg(errp, QERR_IO_ERROR);
>> +        error_setg_errno(errp, errno, "win-dump: failed to write header");
>>           goto out_restore;
>>       }
>>

Re: [PATCH 2/6] dump/win_dump: Improve error messages on write error

Posted by Philippe Mathieu-Daudé 1 year, 9 months ago

On 13/5/24 16:48, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> On 13/5/24 16:16, Markus Armbruster wrote:
>>> create_win_dump() and write_run report qemu_write_full() failure to
>>> their callers as
>>>       An IO error has occurred
>>> The errno set by qemu_write_full() is lost.
>>> Improve this to
>>>       win-dump: failed to write header: <description of errno>
>>> and
>>>       win-dump: failed to save memory: <description of errno>
>>> This matches how dump.c reports similar errors.
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>> ---
>>>    dump/win_dump.c | 7 ++++---
>>>    1 file changed, 4 insertions(+), 3 deletions(-)
>>> diff --git a/dump/win_dump.c b/dump/win_dump.c
>>> index b7bfaff379..0e4fe692ce 100644
>>> --- a/dump/win_dump.c
>>> +++ b/dump/win_dump.c
>>> @@ -12,7 +12,6 @@
>>>    #include "sysemu/dump.h"
>>>    #include "qapi/error.h"
>>>    #include "qemu/error-report.h"
>>> -#include "qapi/qmp/qerror.h"
>>>    #include "exec/cpu-defs.h"
>>>    #include "hw/core/cpu.h"
>>>    #include "qemu/win_dump_defs.h"
>>> @@ -52,6 +51,7 @@ static size_t write_run(uint64_t base_page, uint64_t page_count,
>>>        uint64_t addr = base_page << TARGET_PAGE_BITS;
>>>        uint64_t size = page_count << TARGET_PAGE_BITS;
>>>        uint64_t len, l;
>>> +    int eno;
>>>        size_t total = 0;
>>>          while (size) {
>>> @@ -65,9 +65,10 @@ static size_t write_run(uint64_t base_page, uint64_t page_count,
>>>            }
>>>              l = qemu_write_full(fd, buf, len);
>>> +        eno = errno;
>>
>> Hmm this show the qemu_write_full() API isn't ideal.
>> Maybe we could pass &l as argument and return errno.
>> There are only 20 calls.
> 
> qemu_write_full() is a drop-in replacement for write().

Fine.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>