* [Qemu-devel] [PATCH 1/3] qemu-nbd: Fix return value handling of bdrv_open
@ 2010-03-20 6:23 Ryota Ozaki
2010-03-20 6:23 ` [Qemu-devel] [PATCH 2/3] qemu-nbd: Fix invalid usage of the first argument of errx Ryota Ozaki
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Ryota Ozaki @ 2010-03-20 6:23 UTC (permalink / raw)
To: qemu-devel
bdrv_open may return -errno so we have to check
if the return value is '< 0', not '== -1'.
Signed-off-by: Ryota Ozaki <ozaki.ryota@gmail.com>
---
qemu-nbd.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index a393583..b89c361 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -333,7 +333,7 @@ int main(int argc, char **argv)
if (bs == NULL)
return 1;
- if (bdrv_open(bs, argv[optind], flags) == -1)
+ if (bdrv_open(bs, argv[optind], flags) < 0)
return 1;
fd_size = bs->total_sectors * 512;
--
1.6.5.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH 2/3] qemu-nbd: Fix invalid usage of the first argument of errx
2010-03-20 6:23 [Qemu-devel] [PATCH 1/3] qemu-nbd: Fix return value handling of bdrv_open Ryota Ozaki
@ 2010-03-20 6:23 ` Ryota Ozaki
2010-03-27 13:03 ` Aurelien Jarno
2010-03-20 6:23 ` [Qemu-devel] [PATCH 3/3] qemu-nbd: Improve error reporting Ryota Ozaki
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Ryota Ozaki @ 2010-03-20 6:23 UTC (permalink / raw)
To: qemu-devel
errx takes the exit status of a process as the first
argument. Passing errno to it is wrong. Instead the
patch lets errx take EXIT_FAILURE.
Signed-off-by: Ryota Ozaki <ozaki.ryota@gmail.com>
---
qemu-nbd.c | 34 +++++++++++++++++-----------------
1 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index b89c361..6d854d3 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -114,7 +114,7 @@ static int find_partition(BlockDriverState *bs, int partition,
int ext_partnum = 4;
if (bdrv_read(bs, 0, data, 1))
- errx(EINVAL, "error while reading");
+ errx(EXIT_FAILURE, "error while reading");
if (data[510] != 0x55 || data[511] != 0xaa) {
errno = -EINVAL;
@@ -133,7 +133,7 @@ static int find_partition(BlockDriverState *bs, int partition,
int j;
if (bdrv_read(bs, mbr[i].start_sector_abs, data1, 1))
- errx(EINVAL, "error while reading");
+ errx(EXIT_FAILURE, "error while reading");
for (j = 0; j < 4; j++) {
read_partition(&data1[446 + 16 * j], &ext[j]);
@@ -240,20 +240,20 @@ int main(int argc, char **argv)
case 'p':
li = strtol(optarg, &end, 0);
if (*end) {
- errx(EINVAL, "Invalid port `%s'", optarg);
+ errx(EXIT_FAILURE, "Invalid port `%s'", optarg);
}
if (li < 1 || li > 65535) {
- errx(EINVAL, "Port out of range `%s'", optarg);
+ errx(EXIT_FAILURE, "Port out of range `%s'", optarg);
}
port = (uint16_t)li;
break;
case 'o':
dev_offset = strtoll (optarg, &end, 0);
if (*end) {
- errx(EINVAL, "Invalid offset `%s'", optarg);
+ errx(EXIT_FAILURE, "Invalid offset `%s'", optarg);
}
if (dev_offset < 0) {
- errx(EINVAL, "Offset must be positive `%s'", optarg);
+ errx(EXIT_FAILURE, "Offset must be positive `%s'", optarg);
}
break;
case 'r':
@@ -263,14 +263,14 @@ int main(int argc, char **argv)
case 'P':
partition = strtol(optarg, &end, 0);
if (*end)
- errx(EINVAL, "Invalid partition `%s'", optarg);
+ errx(EXIT_FAILURE, "Invalid partition `%s'", optarg);
if (partition < 1 || partition > 8)
- errx(EINVAL, "Invalid partition %d", partition);
+ errx(EXIT_FAILURE, "Invalid partition %d", partition);
break;
case 'k':
socket = optarg;
if (socket[0] != '/')
- errx(EINVAL, "socket path must be absolute\n");
+ errx(EXIT_FAILURE, "socket path must be absolute\n");
break;
case 'd':
disconnect = true;
@@ -281,10 +281,10 @@ int main(int argc, char **argv)
case 'e':
shared = strtol(optarg, &end, 0);
if (*end) {
- errx(EINVAL, "Invalid shared device number '%s'", optarg);
+ errx(EXIT_FAILURE, "Invalid shared device number '%s'", optarg);
}
if (shared < 1) {
- errx(EINVAL, "Shared device number must be greater than 0\n");
+ errx(EXIT_FAILURE, "Shared device number must be greater than 0\n");
}
break;
case 't':
@@ -302,13 +302,13 @@ int main(int argc, char **argv)
exit(0);
break;
case '?':
- errx(EINVAL, "Try `%s --help' for more information.",
+ errx(EXIT_FAILURE, "Try `%s --help' for more information.",
argv[0]);
}
}
if ((argc - optind) != 1) {
- errx(EINVAL, "Invalid number of argument.\n"
+ errx(EXIT_FAILURE, "Invalid number of argument.\n"
"Try `%s --help' for more information.",
argv[0]);
}
@@ -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]);
+ errx(EXIT_FAILURE, "Cannot open %s", argv[optind]);
nbd_disconnect(fd);
@@ -340,7 +340,7 @@ int main(int argc, char **argv)
if (partition != -1 &&
find_partition(bs, partition, &dev_offset, &fd_size))
- errx(errno, "Could not find partition %d", partition);
+ errx(EXIT_FAILURE, "Could not find partition %d", partition);
if (device) {
pid_t pid;
@@ -349,7 +349,7 @@ int main(int argc, char **argv)
if (!verbose) {
/* detach client and server */
if (daemon(0, 0) == -1) {
- errx(errno, "Failed to daemonize");
+ errx(EXIT_FAILURE, "Failed to daemonize");
}
}
@@ -429,7 +429,7 @@ int main(int argc, char **argv)
data = qemu_memalign(512, NBD_BUFFER_SIZE);
if (data == NULL)
- errx(ENOMEM, "Cannot allocate data buffer");
+ errx(EXIT_FAILURE, "Cannot allocate data buffer");
do {
--
1.6.5.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH 3/3] qemu-nbd: Improve error reporting
2010-03-20 6:23 [Qemu-devel] [PATCH 1/3] qemu-nbd: Fix return value handling of bdrv_open Ryota Ozaki
2010-03-20 6:23 ` [Qemu-devel] [PATCH 2/3] qemu-nbd: Fix invalid usage of the first argument of errx Ryota Ozaki
@ 2010-03-20 6:23 ` Ryota Ozaki
2010-03-27 13:02 ` Aurelien Jarno
2010-03-20 7:01 ` [Qemu-devel] [PATCH 1/3] qemu-nbd: Fix return value handling of bdrv_open Markus Armbruster
2010-03-27 13:03 ` Aurelien Jarno
3 siblings, 1 reply; 9+ messages in thread
From: Ryota Ozaki @ 2010-03-20 6:23 UTC (permalink / raw)
To: qemu-devel
- 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
Signed-off-by: Ryota Ozaki <ozaki.ryota@gmail.com>
---
qemu-nbd.c | 16 +++++++++++-----
1 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 6d854d3..8fb8cc3 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(EXIT_FAILURE, "Cannot open %s", argv[optind]);
+ err(EXIT_FAILURE, "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(EXIT_FAILURE, "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(EXIT_FAILURE, "Could not find partition %d", partition);
+ err(EXIT_FAILURE, "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(EXIT_FAILURE, "Could not access '%s'", device);
+
if (!verbose) {
/* detach client and server */
if (daemon(0, 0) == -1) {
- errx(EXIT_FAILURE, "Failed to daemonize");
+ err(EXIT_FAILURE, "Failed to daemonize");
}
}
--
1.6.5.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] qemu-nbd: Fix return value handling of bdrv_open
2010-03-20 6:23 [Qemu-devel] [PATCH 1/3] qemu-nbd: Fix return value handling of bdrv_open Ryota Ozaki
2010-03-20 6:23 ` [Qemu-devel] [PATCH 2/3] qemu-nbd: Fix invalid usage of the first argument of errx Ryota Ozaki
2010-03-20 6:23 ` [Qemu-devel] [PATCH 3/3] qemu-nbd: Improve error reporting Ryota Ozaki
@ 2010-03-20 7:01 ` Markus Armbruster
2010-03-20 7:05 ` Ryota Ozaki
2010-03-27 13:03 ` Aurelien Jarno
3 siblings, 1 reply; 9+ messages in thread
From: Markus Armbruster @ 2010-03-20 7:01 UTC (permalink / raw)
To: Ryota Ozaki; +Cc: qemu-devel
Ryota Ozaki <ozaki.ryota@gmail.com> writes:
> bdrv_open may return -errno so we have to check
> if the return value is '< 0', not '== -1'.
>
> Signed-off-by: Ryota Ozaki <ozaki.ryota@gmail.com>
> ---
> qemu-nbd.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index a393583..b89c361 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -333,7 +333,7 @@ int main(int argc, char **argv)
> if (bs == NULL)
> return 1;
>
> - if (bdrv_open(bs, argv[optind], flags) == -1)
> + if (bdrv_open(bs, argv[optind], flags) < 0)
> return 1;
>
> fd_size = bs->total_sectors * 512;
Same bug in qemu-io.c. Could you fix that as well?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] qemu-nbd: Fix return value handling of bdrv_open
2010-03-20 7:01 ` [Qemu-devel] [PATCH 1/3] qemu-nbd: Fix return value handling of bdrv_open Markus Armbruster
@ 2010-03-20 7:05 ` Ryota Ozaki
0 siblings, 0 replies; 9+ messages in thread
From: Ryota Ozaki @ 2010-03-20 7:05 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel
On Sat, Mar 20, 2010 at 4:01 PM, Markus Armbruster <armbru@redhat.com> wrote:
> Ryota Ozaki <ozaki.ryota@gmail.com> writes:
>
>> bdrv_open may return -errno so we have to check
>> if the return value is '< 0', not '== -1'.
>>
>> Signed-off-by: Ryota Ozaki <ozaki.ryota@gmail.com>
>> ---
>> qemu-nbd.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/qemu-nbd.c b/qemu-nbd.c
>> index a393583..b89c361 100644
>> --- a/qemu-nbd.c
>> +++ b/qemu-nbd.c
>> @@ -333,7 +333,7 @@ int main(int argc, char **argv)
>> if (bs == NULL)
>> return 1;
>>
>> - if (bdrv_open(bs, argv[optind], flags) == -1)
>> + if (bdrv_open(bs, argv[optind], flags) < 0)
>> return 1;
>>
>> fd_size = bs->total_sectors * 512;
>
> Same bug in qemu-io.c. Could you fix that as well?
>
OK. I will.
Thanks,
ozaki-r
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] qemu-nbd: Improve error reporting
2010-03-20 6:23 ` [Qemu-devel] [PATCH 3/3] qemu-nbd: Improve error reporting Ryota Ozaki
@ 2010-03-27 13:02 ` Aurelien Jarno
2010-03-28 12:26 ` Ryota Ozaki
0 siblings, 1 reply; 9+ messages in thread
From: Aurelien Jarno @ 2010-03-27 13:02 UTC (permalink / raw)
To: Ryota Ozaki; +Cc: qemu-devel
On Sat, Mar 20, 2010 at 03:23:24PM +0900, 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
>
> Signed-off-by: Ryota Ozaki <ozaki.ryota@gmail.com>
> ---
> qemu-nbd.c | 16 +++++++++++-----
> 1 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 6d854d3..8fb8cc3 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(EXIT_FAILURE, "Cannot open %s", argv[optind]);
> + err(EXIT_FAILURE, "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(EXIT_FAILURE, "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(EXIT_FAILURE, "Could not find partition %d", partition);
> + err(EXIT_FAILURE, "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(EXIT_FAILURE, "Could not access '%s'", device);
> +
First of all you need to put this line between curly braces. Secondly,
qemu-nbd as a read-only option to export a block device as read-only.
The test has to be improved to also take care of that.
> if (!verbose) {
> /* detach client and server */
> if (daemon(0, 0) == -1) {
> - errx(EXIT_FAILURE, "Failed to daemonize");
> + err(EXIT_FAILURE, "Failed to daemonize");
> }
> }
>
> --
> 1.6.5.2
>
>
>
>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] qemu-nbd: Fix return value handling of bdrv_open
2010-03-20 6:23 [Qemu-devel] [PATCH 1/3] qemu-nbd: Fix return value handling of bdrv_open Ryota Ozaki
` (2 preceding siblings ...)
2010-03-20 7:01 ` [Qemu-devel] [PATCH 1/3] qemu-nbd: Fix return value handling of bdrv_open Markus Armbruster
@ 2010-03-27 13:03 ` Aurelien Jarno
3 siblings, 0 replies; 9+ messages in thread
From: Aurelien Jarno @ 2010-03-27 13:03 UTC (permalink / raw)
To: Ryota Ozaki; +Cc: qemu-devel
On Sat, Mar 20, 2010 at 03:23:22PM +0900, Ryota Ozaki wrote:
> bdrv_open may return -errno so we have to check
> if the return value is '< 0', not '== -1'.
Thanks, applied.
> Signed-off-by: Ryota Ozaki <ozaki.ryota@gmail.com>
> ---
> qemu-nbd.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index a393583..b89c361 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -333,7 +333,7 @@ int main(int argc, char **argv)
> if (bs == NULL)
> return 1;
>
> - if (bdrv_open(bs, argv[optind], flags) == -1)
> + if (bdrv_open(bs, argv[optind], flags) < 0)
> return 1;
>
> fd_size = bs->total_sectors * 512;
> --
> 1.6.5.2
>
>
>
>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] qemu-nbd: Fix invalid usage of the first argument of errx
2010-03-20 6:23 ` [Qemu-devel] [PATCH 2/3] qemu-nbd: Fix invalid usage of the first argument of errx Ryota Ozaki
@ 2010-03-27 13:03 ` Aurelien Jarno
0 siblings, 0 replies; 9+ messages in thread
From: Aurelien Jarno @ 2010-03-27 13:03 UTC (permalink / raw)
To: Ryota Ozaki; +Cc: qemu-devel
On Sat, Mar 20, 2010 at 03:23:23PM +0900, Ryota Ozaki wrote:
> errx takes the exit status of a process as the first
> argument. Passing errno to it is wrong. Instead the
> patch lets errx take EXIT_FAILURE.
Thanks, applied.
> Signed-off-by: Ryota Ozaki <ozaki.ryota@gmail.com>
> ---
> qemu-nbd.c | 34 +++++++++++++++++-----------------
> 1 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index b89c361..6d854d3 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -114,7 +114,7 @@ static int find_partition(BlockDriverState *bs, int partition,
> int ext_partnum = 4;
>
> if (bdrv_read(bs, 0, data, 1))
> - errx(EINVAL, "error while reading");
> + errx(EXIT_FAILURE, "error while reading");
>
> if (data[510] != 0x55 || data[511] != 0xaa) {
> errno = -EINVAL;
> @@ -133,7 +133,7 @@ static int find_partition(BlockDriverState *bs, int partition,
> int j;
>
> if (bdrv_read(bs, mbr[i].start_sector_abs, data1, 1))
> - errx(EINVAL, "error while reading");
> + errx(EXIT_FAILURE, "error while reading");
>
> for (j = 0; j < 4; j++) {
> read_partition(&data1[446 + 16 * j], &ext[j]);
> @@ -240,20 +240,20 @@ int main(int argc, char **argv)
> case 'p':
> li = strtol(optarg, &end, 0);
> if (*end) {
> - errx(EINVAL, "Invalid port `%s'", optarg);
> + errx(EXIT_FAILURE, "Invalid port `%s'", optarg);
> }
> if (li < 1 || li > 65535) {
> - errx(EINVAL, "Port out of range `%s'", optarg);
> + errx(EXIT_FAILURE, "Port out of range `%s'", optarg);
> }
> port = (uint16_t)li;
> break;
> case 'o':
> dev_offset = strtoll (optarg, &end, 0);
> if (*end) {
> - errx(EINVAL, "Invalid offset `%s'", optarg);
> + errx(EXIT_FAILURE, "Invalid offset `%s'", optarg);
> }
> if (dev_offset < 0) {
> - errx(EINVAL, "Offset must be positive `%s'", optarg);
> + errx(EXIT_FAILURE, "Offset must be positive `%s'", optarg);
> }
> break;
> case 'r':
> @@ -263,14 +263,14 @@ int main(int argc, char **argv)
> case 'P':
> partition = strtol(optarg, &end, 0);
> if (*end)
> - errx(EINVAL, "Invalid partition `%s'", optarg);
> + errx(EXIT_FAILURE, "Invalid partition `%s'", optarg);
> if (partition < 1 || partition > 8)
> - errx(EINVAL, "Invalid partition %d", partition);
> + errx(EXIT_FAILURE, "Invalid partition %d", partition);
> break;
> case 'k':
> socket = optarg;
> if (socket[0] != '/')
> - errx(EINVAL, "socket path must be absolute\n");
> + errx(EXIT_FAILURE, "socket path must be absolute\n");
> break;
> case 'd':
> disconnect = true;
> @@ -281,10 +281,10 @@ int main(int argc, char **argv)
> case 'e':
> shared = strtol(optarg, &end, 0);
> if (*end) {
> - errx(EINVAL, "Invalid shared device number '%s'", optarg);
> + errx(EXIT_FAILURE, "Invalid shared device number '%s'", optarg);
> }
> if (shared < 1) {
> - errx(EINVAL, "Shared device number must be greater than 0\n");
> + errx(EXIT_FAILURE, "Shared device number must be greater than 0\n");
> }
> break;
> case 't':
> @@ -302,13 +302,13 @@ int main(int argc, char **argv)
> exit(0);
> break;
> case '?':
> - errx(EINVAL, "Try `%s --help' for more information.",
> + errx(EXIT_FAILURE, "Try `%s --help' for more information.",
> argv[0]);
> }
> }
>
> if ((argc - optind) != 1) {
> - errx(EINVAL, "Invalid number of argument.\n"
> + errx(EXIT_FAILURE, "Invalid number of argument.\n"
> "Try `%s --help' for more information.",
> argv[0]);
> }
> @@ -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]);
> + errx(EXIT_FAILURE, "Cannot open %s", argv[optind]);
>
> nbd_disconnect(fd);
>
> @@ -340,7 +340,7 @@ int main(int argc, char **argv)
>
> if (partition != -1 &&
> find_partition(bs, partition, &dev_offset, &fd_size))
> - errx(errno, "Could not find partition %d", partition);
> + errx(EXIT_FAILURE, "Could not find partition %d", partition);
>
> if (device) {
> pid_t pid;
> @@ -349,7 +349,7 @@ int main(int argc, char **argv)
> if (!verbose) {
> /* detach client and server */
> if (daemon(0, 0) == -1) {
> - errx(errno, "Failed to daemonize");
> + errx(EXIT_FAILURE, "Failed to daemonize");
> }
> }
>
> @@ -429,7 +429,7 @@ int main(int argc, char **argv)
>
> data = qemu_memalign(512, NBD_BUFFER_SIZE);
> if (data == NULL)
> - errx(ENOMEM, "Cannot allocate data buffer");
> + errx(EXIT_FAILURE, "Cannot allocate data buffer");
>
> do {
>
> --
> 1.6.5.2
>
>
>
>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] qemu-nbd: Improve error reporting
2010-03-27 13:02 ` Aurelien Jarno
@ 2010-03-28 12:26 ` Ryota Ozaki
0 siblings, 0 replies; 9+ messages in thread
From: Ryota Ozaki @ 2010-03-28 12:26 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: qemu-devel
On Sat, Mar 27, 2010 at 10:02 PM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> On Sat, Mar 20, 2010 at 03:23:24PM +0900, 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
>>
>> Signed-off-by: Ryota Ozaki <ozaki.ryota@gmail.com>
>> ---
>> qemu-nbd.c | 16 +++++++++++-----
>> 1 files changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/qemu-nbd.c b/qemu-nbd.c
>> index 6d854d3..8fb8cc3 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(EXIT_FAILURE, "Cannot open %s", argv[optind]);
>> + err(EXIT_FAILURE, "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(EXIT_FAILURE, "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(EXIT_FAILURE, "Could not find partition %d", partition);
>> + err(EXIT_FAILURE, "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(EXIT_FAILURE, "Could not access '%s'", device);
>> +
>
> First of all you need to put this line between curly braces. Secondly,
Oh, sorry.
> qemu-nbd as a read-only option to export a block device as read-only.
> The test has to be improved to also take care of that.
I guess the option is intended to be only for disk image, because
open(2) for nbd device file doesn't care about the option.
Nonetheless, extending the option to operations to nbd device file
makes sense, because the option allows to open nbd device
file without write permission.
So I'll correct it as well as fixing the problems you pointed out.
Thanks,
ozaki-r
>
>> if (!verbose) {
>> /* detach client and server */
>> if (daemon(0, 0) == -1) {
>> - errx(EXIT_FAILURE, "Failed to daemonize");
>> + err(EXIT_FAILURE, "Failed to daemonize");
>> }
>> }
>>
>> --
>> 1.6.5.2
>>
>>
>>
>>
>
> --
> Aurelien Jarno GPG: 1024D/F1BCDB73
> aurelien@aurel32.net http://www.aurel32.net
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-03-28 12:27 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-20 6:23 [Qemu-devel] [PATCH 1/3] qemu-nbd: Fix return value handling of bdrv_open Ryota Ozaki
2010-03-20 6:23 ` [Qemu-devel] [PATCH 2/3] qemu-nbd: Fix invalid usage of the first argument of errx Ryota Ozaki
2010-03-27 13:03 ` Aurelien Jarno
2010-03-20 6:23 ` [Qemu-devel] [PATCH 3/3] qemu-nbd: Improve error reporting Ryota Ozaki
2010-03-27 13:02 ` Aurelien Jarno
2010-03-28 12:26 ` Ryota Ozaki
2010-03-20 7:01 ` [Qemu-devel] [PATCH 1/3] qemu-nbd: Fix return value handling of bdrv_open Markus Armbruster
2010-03-20 7:05 ` Ryota Ozaki
2010-03-27 13:03 ` Aurelien Jarno
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).