diff mbox

[11/18] nbd: ask and print error information from qemu-sockets

Message ID 1349275025-5093-12-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini Oct. 3, 2012, 2:36 p.m. UTC 
Before:

    $ qemu-system-x86_64 nbd:localhost:12345
    inet_connect_opts: connect(ipv4,yakj.usersys.redhat.com,127.0.0.1,12345): Connection refused
    qemu-system-x86_64: could not open disk image nbd:localhost:12345: Connection refused

After:

    $ x86_64-softmmu/qemu-system-x86_64 nbd:localhost:12345
    qemu-system-x86_64: Failed to connect to socket: Connection refused
    qemu-system-x86_64: could not open disk image nbd:localhost:12345: Connection refused

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 nbd.c | 39 +++++++++++++++++++++++++++++++--------
 1 file modificato, 31 inserzioni(+), 8 rimozioni(-)

Comments

Luiz Capitulino Oct. 4, 2012, 8:08 p.m. UTC | #1 
On Wed,  3 Oct 2012 16:36:58 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Before:
> 
>     $ qemu-system-x86_64 nbd:localhost:12345
>     inet_connect_opts: connect(ipv4,yakj.usersys.redhat.com,127.0.0.1,12345): Connection refused
>     qemu-system-x86_64: could not open disk image nbd:localhost:12345: Connection refused
> 
> After:
> 
>     $ x86_64-softmmu/qemu-system-x86_64 nbd:localhost:12345
>     qemu-system-x86_64: Failed to connect to socket: Connection refused
>     qemu-system-x86_64: could not open disk image nbd:localhost:12345: Connection refused
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  nbd.c | 39 +++++++++++++++++++++++++++++++--------
>  1 file modificato, 31 inserzioni(+), 8 rimozioni(-)
> 
> diff --git a/nbd.c b/nbd.c
> index f61a288..cec5a94 100644
> --- a/nbd.c
> +++ b/nbd.c
> @@ -208,7 +208,14 @@ int tcp_socket_outgoing(const char *address, uint16_t port)
>  
>  int tcp_socket_outgoing_spec(const char *address_and_port)
>  {
> -    return inet_connect(address_and_port, NULL);
> +    Error *local_err = NULL;
> +    int fd = inet_connect(address_and_port, &local_err);
> +
> +    if (local_err != NULL) {
> +        qerror_report_err(local_err);
> +        error_free(local_err);

Can't you propagate errp instead of using qerror_report_err()? This function
should only be used when the caller expects QError semantics.

> +    }
> +    return fd;
>  }
>  
>  int tcp_socket_incoming(const char *address, uint16_t port)
> @@ -220,22 +227,38 @@ int tcp_socket_incoming(const char *address, uint16_t port)
>  
>  int tcp_socket_incoming_spec(const char *address_and_port)
>  {
> -    char *ostr  = NULL;
> -    int olen = 0;
> -    return inet_listen(address_and_port, ostr, olen, SOCK_STREAM, 0, NULL);
> +    Error *local_err = NULL;
> +    int fd = inet_listen(address_and_port, NULL, 0, SOCK_STREAM, 0, &local_err);
> +
> +    if (local_err != NULL) {
> +        qerror_report_err(local_err);
> +        error_free(local_err);
> +    }
> +    return fd;
>  }
>  
>  int unix_socket_incoming(const char *path)
>  {
> -    char *ostr = NULL;
> -    int olen = 0;
> +    Error *local_err = NULL;
> +    int fd = unix_listen(path, NULL, 0, &local_err);
>  
> -    return unix_listen(path, ostr, olen, NULL);
> +    if (local_err != NULL) {
> +        qerror_report_err(local_err);
> +        error_free(local_err);
> +    }
> +    return fd;
>  }
>  
>  int unix_socket_outgoing(const char *path)
>  {
> -    return unix_connect(path, NULL);
> +    Error *local_err = NULL;
> +    int fd = unix_connect(path, &local_err);
> +
> +    if (local_err != NULL) {
> +        qerror_report_err(local_err);
> +        error_free(local_err);
> +    }
> +    return fd;
>  }
>  
>  /* Basic flow for negotiation
Paolo Bonzini Oct. 5, 2012, 6:27 a.m. UTC | #2 
Il 04/10/2012 22:08, Luiz Capitulino ha scritto:
> On Wed,  3 Oct 2012 16:36:58 +0200
> Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
>> Before:
>>
>>     $ qemu-system-x86_64 nbd:localhost:12345
>>     inet_connect_opts: connect(ipv4,yakj.usersys.redhat.com,127.0.0.1,12345): Connection refused
>>     qemu-system-x86_64: could not open disk image nbd:localhost:12345: Connection refused
>>
>> After:
>>
>>     $ x86_64-softmmu/qemu-system-x86_64 nbd:localhost:12345
>>     qemu-system-x86_64: Failed to connect to socket: Connection refused
>>     qemu-system-x86_64: could not open disk image nbd:localhost:12345: Connection refused
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  nbd.c | 39 +++++++++++++++++++++++++++++++--------
>>  1 file modificato, 31 inserzioni(+), 8 rimozioni(-)
>>
>> diff --git a/nbd.c b/nbd.c
>> index f61a288..cec5a94 100644
>> --- a/nbd.c
>> +++ b/nbd.c
>> @@ -208,7 +208,14 @@ int tcp_socket_outgoing(const char *address, uint16_t port)
>>  
>>  int tcp_socket_outgoing_spec(const char *address_and_port)
>>  {
>> -    return inet_connect(address_and_port, NULL);
>> +    Error *local_err = NULL;
>> +    int fd = inet_connect(address_and_port, &local_err);
>> +
>> +    if (local_err != NULL) {
>> +        qerror_report_err(local_err);
>> +        error_free(local_err);
> 
> Can't you propagate errp instead of using qerror_report_err()? This function
> should only be used when the caller expects QError semantics.

No, I cannot.  These functions are used only for a) qemu-nbd, for which
qerror_report_err() is ok; b) the NBD driver's bdrv_open, which is not
able to propagate errors yet.

When error propagation is added to bdrv_open they can just disappear,
replaced by direct calls to functions in qemu-sockets.c.

Paolo

>> +    }
>> +    return fd;
>>  }
>>  
>>  int tcp_socket_incoming(const char *address, uint16_t port)
>> @@ -220,22 +227,38 @@ int tcp_socket_incoming(const char *address, uint16_t port)
>>  
>>  int tcp_socket_incoming_spec(const char *address_and_port)
>>  {
>> -    char *ostr  = NULL;
>> -    int olen = 0;
>> -    return inet_listen(address_and_port, ostr, olen, SOCK_STREAM, 0, NULL);
>> +    Error *local_err = NULL;
>> +    int fd = inet_listen(address_and_port, NULL, 0, SOCK_STREAM, 0, &local_err);
>> +
>> +    if (local_err != NULL) {
>> +        qerror_report_err(local_err);
>> +        error_free(local_err);
>> +    }
>> +    return fd;
>>  }
>>  
>>  int unix_socket_incoming(const char *path)
>>  {
>> -    char *ostr = NULL;
>> -    int olen = 0;
>> +    Error *local_err = NULL;
>> +    int fd = unix_listen(path, NULL, 0, &local_err);
>>  
>> -    return unix_listen(path, ostr, olen, NULL);
>> +    if (local_err != NULL) {
>> +        qerror_report_err(local_err);
>> +        error_free(local_err);
>> +    }
>> +    return fd;
>>  }
>>  
>>  int unix_socket_outgoing(const char *path)
>>  {
>> -    return unix_connect(path, NULL);
>> +    Error *local_err = NULL;
>> +    int fd = unix_connect(path, &local_err);
>> +
>> +    if (local_err != NULL) {
>> +        qerror_report_err(local_err);
>> +        error_free(local_err);
>> +    }
>> +    return fd;
>>  }
>>  
>>  /* Basic flow for negotiation
> 
> 
>
Luiz Capitulino Oct. 5, 2012, 12:42 p.m. UTC | #3 
On Fri, 05 Oct 2012 08:27:25 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 04/10/2012 22:08, Luiz Capitulino ha scritto:
> > On Wed,  3 Oct 2012 16:36:58 +0200
> > Paolo Bonzini <pbonzini@redhat.com> wrote:
> > 
> >> Before:
> >>
> >>     $ qemu-system-x86_64 nbd:localhost:12345
> >>     inet_connect_opts: connect(ipv4,yakj.usersys.redhat.com,127.0.0.1,12345): Connection refused
> >>     qemu-system-x86_64: could not open disk image nbd:localhost:12345: Connection refused
> >>
> >> After:
> >>
> >>     $ x86_64-softmmu/qemu-system-x86_64 nbd:localhost:12345
> >>     qemu-system-x86_64: Failed to connect to socket: Connection refused
> >>     qemu-system-x86_64: could not open disk image nbd:localhost:12345: Connection refused
> >>
> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> >> ---
> >>  nbd.c | 39 +++++++++++++++++++++++++++++++--------
> >>  1 file modificato, 31 inserzioni(+), 8 rimozioni(-)
> >>
> >> diff --git a/nbd.c b/nbd.c
> >> index f61a288..cec5a94 100644
> >> --- a/nbd.c
> >> +++ b/nbd.c
> >> @@ -208,7 +208,14 @@ int tcp_socket_outgoing(const char *address, uint16_t port)
> >>  
> >>  int tcp_socket_outgoing_spec(const char *address_and_port)
> >>  {
> >> -    return inet_connect(address_and_port, NULL);
> >> +    Error *local_err = NULL;
> >> +    int fd = inet_connect(address_and_port, &local_err);
> >> +
> >> +    if (local_err != NULL) {
> >> +        qerror_report_err(local_err);
> >> +        error_free(local_err);
> > 
> > Can't you propagate errp instead of using qerror_report_err()? This function
> > should only be used when the caller expects QError semantics.
> 
> No, I cannot.  These functions are used only for a) qemu-nbd, for which
> qerror_report_err() is ok; b) the NBD driver's bdrv_open, which is not
> able to propagate errors yet.
> 
> When error propagation is added to bdrv_open they can just disappear,
> replaced by direct calls to functions in qemu-sockets.c.

Ok. I've started working on adding error propagation to bdrv_open(), should
have a series soon.
diff mbox

Patch

diff --git a/nbd.c b/nbd.c
index f61a288..cec5a94 100644
--- a/nbd.c
+++ b/nbd.c
@@ -208,7 +208,14 @@  int tcp_socket_outgoing(const char *address, uint16_t port)
 
 int tcp_socket_outgoing_spec(const char *address_and_port)
 {
-    return inet_connect(address_and_port, NULL);
+    Error *local_err = NULL;
+    int fd = inet_connect(address_and_port, &local_err);
+
+    if (local_err != NULL) {
+        qerror_report_err(local_err);
+        error_free(local_err);
+    }
+    return fd;
 }
 
 int tcp_socket_incoming(const char *address, uint16_t port)
@@ -220,22 +227,38 @@  int tcp_socket_incoming(const char *address, uint16_t port)
 
 int tcp_socket_incoming_spec(const char *address_and_port)
 {
-    char *ostr  = NULL;
-    int olen = 0;
-    return inet_listen(address_and_port, ostr, olen, SOCK_STREAM, 0, NULL);
+    Error *local_err = NULL;
+    int fd = inet_listen(address_and_port, NULL, 0, SOCK_STREAM, 0, &local_err);
+
+    if (local_err != NULL) {
+        qerror_report_err(local_err);
+        error_free(local_err);
+    }
+    return fd;
 }
 
 int unix_socket_incoming(const char *path)
 {
-    char *ostr = NULL;
-    int olen = 0;
+    Error *local_err = NULL;
+    int fd = unix_listen(path, NULL, 0, &local_err);
 
-    return unix_listen(path, ostr, olen, NULL);
+    if (local_err != NULL) {
+        qerror_report_err(local_err);
+        error_free(local_err);
+    }
+    return fd;
 }
 
 int unix_socket_outgoing(const char *path)
 {
-    return unix_connect(path, NULL);
+    Error *local_err = NULL;
+    int fd = unix_connect(path, &local_err);
+
+    if (local_err != NULL) {
+        qerror_report_err(local_err);
+        error_free(local_err);
+    }
+    return fd;
 }
 
 /* Basic flow for negotiation