Re: [libvirt PATCH v2 38/56] tools: convert to use g_poll instead of poll

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jan 28, 2020 at 01:11:19PM +0000, Daniel P. Berrangé wrote:
> g_poll is portable to Windows platforms.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> ---
>  tools/virsh-domain.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 1a48695b4e..04ba44d4f2 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -23,7 +23,6 @@
>  #include "virsh-util.h"
>  
>  #include <fcntl.h>
> -#include <poll.h>
>  #include <signal.h>
>  #include <sys/time.h>
>  
> @@ -4299,15 +4298,15 @@ virshWatchJob(vshControl *ctl,
>      struct sigaction old_sig_action;
>      sigset_t sigmask, oldsigmask;
>  #endif /* !WIN32 */
> -    struct pollfd pollfd[2] = {{.fd = pipe_fd, .events = POLLIN, .revents = 0},
> -                               {.fd = STDIN_FILENO, .events = POLLIN, .revents = 0}};
> +    GPollFD pollfd[2] = {{.fd = pipe_fd, .events = G_IO_IN, .revents = 0},
> +                         {.fd = STDIN_FILENO, .events = G_IO_IN, .revents = 0}};

This doesn't look right.  Based on the GLib documentation in case of
unix the .fd should be file descriptor but in case of WIN32 it should
be HANDLE.

Looking at the GNULIB implementation it was converting FD into HANDLE
internally in the poll() function so everything was safe but in case
of g_poll() they except the .fd already be converted to HANDLE.

Our implementation of virPipeQuiet calls _poll() for WIN32 but based on
the reference [1] it looks like it returns FD.

I guess that we should create a wrapper around g_poll to covert FD into
HANDLE on WIN32 to be on a safe side.

Pavel

>      unsigned long long start_us, curr_us;
>      virDomainJobInfo jobinfo;
>      int ret = -1;
>      char retchar;
>      bool functionReturn = false;
>      bool jobStarted = false;
> -    nfds_t npollfd = 2;
> +    int npollfd = 2;
>  
>  #ifndef WIN32
>      sigemptyset(&sigmask);
> @@ -4326,16 +4325,16 @@ virshWatchJob(vshControl *ctl,
>  
>      start_us = g_get_real_time();
>      while (1) {
> -        ret = poll((struct pollfd *)&pollfd, npollfd, 500);
> +        ret = g_poll(pollfd, npollfd, 500);
>          if (ret > 0) {
> -            if (pollfd[1].revents & POLLIN &&
> +            if (pollfd[1].revents & G_IO_IN &&
>                  saferead(STDIN_FILENO, &retchar, sizeof(retchar)) > 0) {
>                  if (vshTTYIsInterruptCharacter(ctl, retchar))
>                      virDomainAbortJob(dom);
>                  continue;
>              }
>  
> -            if (pollfd[0].revents & POLLIN &&
> +            if (pollfd[0].revents & G_IO_IN &&
>                  saferead(pipe_fd, &retchar, sizeof(retchar)) > 0 &&
>                  retchar == '0') {
>                  if (verbose) {
> -- 
> 2.24.1
> 

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux