Re: [PATCH 0/9] migration/mapped-ram: Add direct-io support

2024-05-02 Thread Fabiano Rosas
Peter Xu  writes:

> On Fri, Apr 26, 2024 at 11:20:33AM -0300, Fabiano Rosas wrote:
>> If the user is not passing in a file name which QEMU can open at will,
>> we must then require that the user pass the two file descriptors with
>> the flags already properly set. We'll use the already existing fdset +
>> QMP add-fd infrastructure for this.
>
> Yes I remember such requirement that one extra fd is needed for direct-io,
> however today when I looked closer at the man page it looks like F_SETFL
> works with O_DIRECT too?
>
>F_SETFL (int)
>   Set the file status flags to the value specified by arg.
>   File access mode (O_RDONLY, O_WRONLY, O_RDWR) and file
>   creation flags (i.e., O_CREAT, O_EXCL, O_NOCTTY, O_TRUNC) in
>   arg are ignored.  On Linux, this command can change only the
>   O_APPEND, O_ASYNC, O_DIRECT, O_NOATIME, and O_NONBLOCK flags.
>   It is not possible to change the O_DSYNC and O_SYNC flags;
>   see BUGS, below.
>
> 8<
> $ cat fcntl.c
> #define _GNU_SOURCE
> #include 
> #include 
> #include 
> #include 
>
> int main(void)
> {
> int fd, newfd, ret, flags;
>
> fd = open("test.txt", O_RDWR | O_CREAT, 0660);
> assert(fd != -1);
>
> flags = fcntl(fd, F_GETFL);
> printf("old fd flags: 0x%x\n", flags);
>
> newfd = dup(fd);
> assert(newfd != -1);
>
> flags = fcntl(newfd, F_GETFL);
> printf("new fd flags: 0x%x\n", flags);
>
> flags |= O_DIRECT;
> ret = fcntl(newfd, F_SETFL, flags);
>
> flags = fcntl(fd, F_GETFL);
> printf("updated new flags: 0x%x\n", flags);
> 
> return 0;
> }
> $ make fcntl
> cc fcntl.c   -o fcntl
> $ ./fcntl 
> old fd flags: 0x8002
> new fd flags: 0x8002
> updated new flags: 0xc002
> 8<
>
> Perhaps I missed something important?

The dup()'ed file descriptor shares file status flags with the original
fd. Your code example proves just that. In the last two blocks you're
doing F_SETFL on the 'newfd' and then seeing the change take effect on
'fd'. That's what we don't want to happen.



Re: [PATCH 0/9] migration/mapped-ram: Add direct-io support

2024-05-02 Thread Peter Xu
On Fri, Apr 26, 2024 at 11:20:33AM -0300, Fabiano Rosas wrote:
> If the user is not passing in a file name which QEMU can open at will,
> we must then require that the user pass the two file descriptors with
> the flags already properly set. We'll use the already existing fdset +
> QMP add-fd infrastructure for this.

Yes I remember such requirement that one extra fd is needed for direct-io,
however today when I looked closer at the man page it looks like F_SETFL
works with O_DIRECT too?

   F_SETFL (int)
  Set the file status flags to the value specified by arg.
  File access mode (O_RDONLY, O_WRONLY, O_RDWR) and file
  creation flags (i.e., O_CREAT, O_EXCL, O_NOCTTY, O_TRUNC) in
  arg are ignored.  On Linux, this command can change only the
  O_APPEND, O_ASYNC, O_DIRECT, O_NOATIME, and O_NONBLOCK flags.
  It is not possible to change the O_DSYNC and O_SYNC flags;
  see BUGS, below.

8<
$ cat fcntl.c
#define _GNU_SOURCE
#include 
#include 
#include 
#include 

int main(void)
{
int fd, newfd, ret, flags;

fd = open("test.txt", O_RDWR | O_CREAT, 0660);
assert(fd != -1);

flags = fcntl(fd, F_GETFL);
printf("old fd flags: 0x%x\n", flags);

newfd = dup(fd);
assert(newfd != -1);

flags = fcntl(newfd, F_GETFL);
printf("new fd flags: 0x%x\n", flags);

flags |= O_DIRECT;
ret = fcntl(newfd, F_SETFL, flags);

flags = fcntl(fd, F_GETFL);
printf("updated new flags: 0x%x\n", flags);

return 0;
}
$ make fcntl
cc fcntl.c   -o fcntl
$ ./fcntl 
old fd flags: 0x8002
new fd flags: 0x8002
updated new flags: 0xc002
8<

Perhaps I missed something important?

-- 
Peter Xu