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

Pavel Hrdina phrdina at redhat.com
Fri Jan 31 09:26:37 UTC 2020


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 at redhat.com>
> ---
>  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
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200131/64a62ce3/attachment-0001.sig>


More information about the libvir-list mailing list