Re: [Qemu-devel] [PATCH 2/2] qemu-nbd: Improve error reporting

[Ryota Ozaki]

On Sat, Mar 20, 2010 at 2:26 PM, malc <address@hidden> wrote:
> On Sat, 20 Mar 2010, Ryota Ozaki wrote:
>
>> - use err(3) instead of errx(3) if errno is available
>>   to report why failed
>> - let fail prior to daemon(3) if opening a nbd file
>>   is likely to fail after daemonizing to avoid silent
>>   failure exit
>
> I'm under impression that you and the original author of this
> code is misunderstading how err[x] works, first argument is
> what will be used to exit the process and has little to do
> with POSIX/C error codes.

Oh, you're right. I'll revise and resend the patch.
(and I'll add missing Signed-off-by fields)

Thanks,
  ozaki-r

>
>> ---
>>  qemu-nbd.c |   16 +++++++++++-----
>>  1 files changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/qemu-nbd.c b/qemu-nbd.c
>> index b89c361..5f10ff0 100644
>> --- a/qemu-nbd.c
>> +++ b/qemu-nbd.c
>> @@ -316,7 +316,7 @@ int main(int argc, char **argv)
>>      if (disconnect) {
>>          fd = open(argv[optind], O_RDWR);
>>          if (fd == -1)
>> -            errx(errno, "Cannot open %s", argv[optind]);
>> +            err(errno, "Cannot open %s", argv[optind]);
>>
>>          nbd_disconnect(fd);
>>
>> @@ -333,23 +333,29 @@ int main(int argc, char **argv)
>>      if (bs == NULL)
>>          return 1;
>>
>> -    if (bdrv_open(bs, argv[optind], flags) < 0)
>> -        return 1;
>> +    if ((ret = bdrv_open(bs, argv[optind], flags)) < 0) {
>> +        errno = -ret;
>> +        err(errno, "Failed to bdrv_open '%s'", argv[optind]);
>> +    }
>>
>>      fd_size = bs->total_sectors * 512;
>>
>>      if (partition != -1 &&
>>          find_partition(bs, partition, &dev_offset, &fd_size))
>> -        errx(errno, "Could not find partition %d", partition);
>> +        err(errno, "Could not find partition %d", partition);
>>
>>      if (device) {
>>          pid_t pid;
>>          int sock;
>>
>> +        /* want to fail before daemonizing */
>> +        if (access(device, R_OK|W_OK) == -1)
>> +            err(errno, "Could not access '%s'", device);
>> +
>>          if (!verbose) {
>>              /* detach client and server */
>>              if (daemon(0, 0) == -1) {
>> -                errx(errno, "Failed to daemonize");
>> +                err(errno, "Failed to daemonize");
>>              }
>>          }
>>
>>
>
> --
> mailto:address@hidden
>