Re: [Qemu-devel] [PATCH v3 1/1] dump.c: allow fd_write_vmcore to return errno on failure

[Murilo Opsfelder Araujo <muriloo@linux.vnet.ibm.com>]

On 02/12/2018 03:31 PM, Eric Blake wrote:
> On 02/12/2018 08:46 AM, Murilo Opsfelder Araujo wrote:
>> On 02/12/2018 12:25 PM, Daniel Henrique Barboza wrote:
>>> From: Yasmin Beatriz <yasmins@linux.vnet.ibm.com>
>>>
>>> fd_write_vmcore can fail to execute for a lot of reasons that can be
>>> retrieved by errno, but it only returns -1. This makes difficult for
>>> the caller to know what happened and only a generic error message is
>>> propagated back to the user. This is an example using dump-guest-memory:
>>>
> 
>>> +++ b/dump.c
>>> @@ -107,7 +107,7 @@ static int fd_write_vmcore(const void *buf,
>>> size_t size, void *opaque)
>>>
>>>       written_size = qemu_write_full(s->fd, buf, size);
>>>       if (written_size != size) {
>>> -        return -1;
>>> +        return -errno;
>>>       }
>>>
>>>       return 0;
>>> @@ -140,7 +140,7 @@ static void write_elf64_header(DumpState *s,
>>> Error **errp)
>>>
>>>       ret = fd_write_vmcore(&elf_header, sizeof(elf_header), s);
>>>       if (ret < 0) {
>>> -        error_setg(errp, "dump: failed to write elf header");
>>> +        error_setg_errno(errp, -ret, "dump: failed to write elf
>>> header");
>>
>> Do we need -ret passed to error_setg_errno()? fd_write_vmcore() returns
>> negative errno in case of error.
> 
> Yes, this usage is correct.  error_setg_errno() takes a positive errno
> value (using strerror, which only decodes positive values into useful
> strings); but we typically return negative errno values (as was
> correctly done in fd_write_vmcore), so the extra layer of negation here
> is needed.
> 

For some reason I assumed "non-zero" in the error_setg_errno()
description as negative.

Thanks Daniel and Eric.