Re: [PATCH v4 32/34] monitor: fdset: Match against O_DIRECT

[Markus Armbruster]

Fabiano Rosas <faro...@suse.de> writes:

> We're about to enable the use of O_DIRECT in the migration code and
> due to the alignment restrictions imposed by filesystems we need to
> make sure the flag is only used when doing aligned IO.
>
> The migration will do parallel IO to different regions of a file, so
> we need to use more than one file descriptor. Those cannot be obtained
> by duplicating (dup()) since duplicated file descriptors share the
> file status flags, including O_DIRECT. If one migration channel does
> unaligned IO while another sets O_DIRECT to do aligned IO, the
> filesystem would fail the unaligned operation.
>
> The add-fd QMP command along with the fdset code are specifically
> designed to allow the user to pass a set of file descriptors with
> different access flags into QEMU to be later fetched by code that
> needs to alternate between those flags when doing IO.
>
> Extend the fdset matching function to behave the same with the
> O_DIRECT flag.
>
> Signed-off-by: Fabiano Rosas <faro...@suse.de>
> ---
>  monitor/fds.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/monitor/fds.c b/monitor/fds.c
> index 9a28e4b72b..42bf3eb982 100644
> --- a/monitor/fds.c
> +++ b/monitor/fds.c
> @@ -413,6 +413,12 @@ static bool monitor_fdset_flags_match(int flags, int 
> fd_flags)
   static bool monitor_fdset_flags_match(int flags, int fd_flags)
   {
       bool match = false;
   
>      if ((flags & O_ACCMODE) == (fd_flags & O_ACCMODE)) {
>          match = true;
> +
> +#ifdef O_DIRECT
> +        if ((flags & O_DIRECT) != (fd_flags & O_DIRECT)) {
> +            match = false;
> +        }
> +#endif
>      }
>  
>      return match;
   }

I'd prefer something like

   static bool monitor_fdset_flags_match(int flags, int fd_flags)
   {
   #ifdef O_DIRECT
       if ((flags & O_DIRECT) != (fd_flags & O_DIRECT)) {
           return false;
       }
   #endif

       if ((flags & O_ACCMODE) != (fd_flags & O_ACCMODE)) {
           return false;

       }

       return true;
   }