diff mbox

use different fd variables for device and socket, unbreak qemu-nbd -c

Message ID 20120114114459.493513DE@gandalf.tls.msk.ru
State New
Headers show

Commit Message

Michael Tokarev Jan. 14, 2012, 11:41 a.m. UTC
commit a61c67828dea7c64edaf226cadb45b4ffcc1d411
Author: Paolo Bonzini <pbonzini@redhat.com>
Date:   Mon Sep 12 17:28:11 2011 +0200

    qemu-nbd: use common main loop

    Using a single main loop for sockets will help yielding from the socket
    coroutine back to the main loop, and later reentering it.

changed code to use local variable "fd" in qemu-nbd.c:main()
in two places: for /dev/nbd device and for control socket.
The result is that qemu-nbd -c $device does not work anymore.

Use two variables - devfs and sockfd - for the two purposes,
instead of one fd.

Signed-Off-By: Michael Tokarev <mjt@tls.msk.ru>
---
 qemu-nbd.c |   26 +++++++++++++-------------
 1 files changed, 13 insertions(+), 13 deletions(-)

Comments

Paolo Bonzini Jan. 15, 2012, 10:45 a.m. UTC | #1
On 01/14/2012 12:41 PM, Michael Tokarev wrote:
> commit a61c67828dea7c64edaf226cadb45b4ffcc1d411
> Author: Paolo Bonzini<pbonzini@redhat.com>
> Date:   Mon Sep 12 17:28:11 2011 +0200
>
>      qemu-nbd: use common main loop
>
>      Using a single main loop for sockets will help yielding from the socket
>      coroutine back to the main loop, and later reentering it.
>
> changed code to use local variable "fd" in qemu-nbd.c:main()
> in two places: for /dev/nbd device and for control socket.
> The result is that qemu-nbd -c $device does not work anymore.
>
> Use two variables - devfs and sockfd - for the two purposes,
> instead of one fd.
>
> Signed-Off-By: Michael Tokarev<mjt@tls.msk.ru>
> ---
>   qemu-nbd.c |   26 +++++++++++++-------------
>   1 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index eb61c33..e76c782 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -301,7 +301,7 @@ int main(int argc, char **argv)
>       int flags = BDRV_O_RDWR;
>       int partition = -1;
>       int ret;
> -    int fd;
> +    int sockfd, devfd;
>       int persistent = 0;
>       pthread_t client_thread;
>
> @@ -401,13 +401,13 @@ int main(int argc, char **argv)
>       }
>
>       if (disconnect) {
> -        fd = open(argv[optind], O_RDWR);
> -        if (fd == -1)
> +        sockfd = open(argv[optind], O_RDWR);
> +        if (sockfd == -1)
>               err(EXIT_FAILURE, "Cannot open %s", argv[optind]);
>
> -        nbd_disconnect(fd);
> +        nbd_disconnect(sockfd);
>
> -        close(fd);
> +        close(sockfd);
>
>           printf("%s disconnected\n", argv[optind]);
>

This should be devfd.

> @@ -470,8 +470,8 @@ int main(int argc, char **argv)
>           /* Open before spawning new threads.  In the future, we may
>            * drop privileges after opening.
>            */
> -        fd = open(device, O_RDWR);
> -        if (fd == -1) {
> +        devfd = open(device, O_RDWR);
> +        if (devfd == -1) {
>               err(EXIT_FAILURE, "Failed to open %s", device);
>           }
>
> @@ -501,19 +501,19 @@ int main(int argc, char **argv)
>       exp = nbd_export_new(bs, dev_offset, fd_size, nbdflags);
>
>       if (sockpath) {
> -        fd = unix_socket_incoming(sockpath);
> +        sockfd = unix_socket_incoming(sockpath);
>       } else {
> -        fd = tcp_socket_incoming(bindto, port);
> +        sockfd = tcp_socket_incoming(bindto, port);
>       }
>
> -    if (fd == -1) {
> +    if (sockfd == -1) {
>           return 1;
>       }
>
>       if (device) {
>           int ret;
>
> -        ret = pthread_create(&client_thread, NULL, nbd_client_thread,&fd);
> +        ret = pthread_create(&client_thread, NULL, nbd_client_thread,&devfd);
>           if (ret != 0) {
>               errx(EXIT_FAILURE, "Failed to create client thread: %s",
>                    strerror(ret));
> @@ -524,8 +524,8 @@ int main(int argc, char **argv)
>       }
>
>       qemu_init_main_loop();
> -    qemu_set_fd_handler2(fd, nbd_can_accept, nbd_accept, NULL,
> -                         (void *)(uintptr_t)fd);
> +    qemu_set_fd_handler2(sockfd, nbd_can_accept, nbd_accept, NULL,
> +                         (void *)(uintptr_t)sockfd);
>
>       do {
>           main_loop_wait(false);

Otherwise looks good, I'll fix up and send for inclusion.

Paolo
Michael Tokarev Jan. 15, 2012, 1:01 p.m. UTC | #2
On 15.01.2012 14:45, Paolo Bonzini wrote:
> On 01/14/2012 12:41 PM, Michael Tokarev wrote:
>> commit a61c67828dea7c64edaf226cadb45b4ffcc1d411
>> Author: Paolo Bonzini<pbonzini@redhat.com>
>> Date:   Mon Sep 12 17:28:11 2011 +0200
>>
>>      qemu-nbd: use common main loop
>>
>>      Using a single main loop for sockets will help yielding from the socket
>>      coroutine back to the main loop, and later reentering it.
>>
>> changed code to use local variable "fd" in qemu-nbd.c:main()
>> in two places: for /dev/nbd device and for control socket.
>> The result is that qemu-nbd -c $device does not work anymore.
>>
>> Use two variables - devfs and sockfd - for the two purposes,
>> instead of one fd.
>>
>> Signed-Off-By: Michael Tokarev<mjt@tls.msk.ru>
>> ---
>>   qemu-nbd.c |   26 +++++++++++++-------------
>>   1 files changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/qemu-nbd.c b/qemu-nbd.c
>> index eb61c33..e76c782 100644
>> --- a/qemu-nbd.c
>> +++ b/qemu-nbd.c
>> @@ -301,7 +301,7 @@ int main(int argc, char **argv)
>>       int flags = BDRV_O_RDWR;
>>       int partition = -1;
>>       int ret;
>> -    int fd;
>> +    int sockfd, devfd;
>>       int persistent = 0;
>>       pthread_t client_thread;
>>
>> @@ -401,13 +401,13 @@ int main(int argc, char **argv)
>>       }
>>
>>       if (disconnect) {
>> -        fd = open(argv[optind], O_RDWR);
>> -        if (fd == -1)
>> +        sockfd = open(argv[optind], O_RDWR);
>> +        if (sockfd == -1)
>>               err(EXIT_FAILURE, "Cannot open %s", argv[optind]);
>>
>> -        nbd_disconnect(fd);
>> +        nbd_disconnect(sockfd);
>>
>> -        close(fd);
>> +        close(sockfd);
>>
>>           printf("%s disconnected\n", argv[optind]);
> 
> This should be devfd.

Yes indeed.  In that case make it nbdfd not devfd - there are other
variables prefixed "nbd" already.

[]
> Otherwise looks good, I'll fix up and send for inclusion.

Thanks!

FWIW, it is not -stable material, since 1.0 isn't broken yet ;)

/mjt
diff mbox

Patch

diff --git a/qemu-nbd.c b/qemu-nbd.c
index eb61c33..e76c782 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -301,7 +301,7 @@  int main(int argc, char **argv)
     int flags = BDRV_O_RDWR;
     int partition = -1;
     int ret;
-    int fd;
+    int sockfd, devfd;
     int persistent = 0;
     pthread_t client_thread;
 
@@ -401,13 +401,13 @@  int main(int argc, char **argv)
     }
 
     if (disconnect) {
-        fd = open(argv[optind], O_RDWR);
-        if (fd == -1)
+        sockfd = open(argv[optind], O_RDWR);
+        if (sockfd == -1)
             err(EXIT_FAILURE, "Cannot open %s", argv[optind]);
 
-        nbd_disconnect(fd);
+        nbd_disconnect(sockfd);
 
-        close(fd);
+        close(sockfd);
 
         printf("%s disconnected\n", argv[optind]);
 
@@ -470,8 +470,8 @@  int main(int argc, char **argv)
         /* Open before spawning new threads.  In the future, we may
          * drop privileges after opening.
          */
-        fd = open(device, O_RDWR);
-        if (fd == -1) {
+        devfd = open(device, O_RDWR);
+        if (devfd == -1) {
             err(EXIT_FAILURE, "Failed to open %s", device);
         }
 
@@ -501,19 +501,19 @@  int main(int argc, char **argv)
     exp = nbd_export_new(bs, dev_offset, fd_size, nbdflags);
 
     if (sockpath) {
-        fd = unix_socket_incoming(sockpath);
+        sockfd = unix_socket_incoming(sockpath);
     } else {
-        fd = tcp_socket_incoming(bindto, port);
+        sockfd = tcp_socket_incoming(bindto, port);
     }
 
-    if (fd == -1) {
+    if (sockfd == -1) {
         return 1;
     }
 
     if (device) {
         int ret;
 
-        ret = pthread_create(&client_thread, NULL, nbd_client_thread, &fd);
+        ret = pthread_create(&client_thread, NULL, nbd_client_thread, &devfd);
         if (ret != 0) {
             errx(EXIT_FAILURE, "Failed to create client thread: %s",
                  strerror(ret));
@@ -524,8 +524,8 @@  int main(int argc, char **argv)
     }
 
     qemu_init_main_loop();
-    qemu_set_fd_handler2(fd, nbd_can_accept, nbd_accept, NULL,
-                         (void *)(uintptr_t)fd);
+    qemu_set_fd_handler2(sockfd, nbd_can_accept, nbd_accept, NULL,
+                         (void *)(uintptr_t)sockfd);
 
     do {
         main_loop_wait(false);