Re: [Qemu-devel] [RFC PATCH 3/4] block: Enable QEMU to retrieve passed fd before attempting open

[Corey Bryant <coreyb@linux.vnet.ibm.com>]

On 05/21/2012 05:50 PM, Eric Blake wrote:
> On 05/21/2012 02:19 PM, Corey Bryant wrote:
>> With this patch, when QEMU needs to "open" a file, it will first
>> check to see if a matching filename/fd pair were passed via the
>> -filefd command line option or the getfd_file monitor command.
>> If a match is found, QEMU will use the passed fd and will not
>> attempt to open the file.  Otherwise, if a match is not found,
>> QEMU will attempt to open the file on it's own.
>>
>> Signed-off-by: Corey Bryant<coreyb@linux.vnet.ibm.com>
>
>> +int file_open(const char *filename, int flags, mode_t mode)
>> +{
>> +    int fd;
>> +
>> +#ifdef _WIN32
>> +    return qemu_open(filename, flags, mode);
>> +#else
>
> Would it be any easier to write:
>
> #ifndef _WIN32
>    qemu_get_fd_file() stuff
> #endif
>    return qemu_open()
>
> so that you aren't repeating the return line?
>

Yes that's easier.  Thanks for the suggestion!

>
>> +    fd = qemu_get_fd_file(filename, false);
>> +    if (fd != -1) {
>> +        return dup(fd);
>
> Why are you dup'ing the fd?  That just sounds like a way to leak fds.
> Remember, the existing 'getfd' monitor command doesn't dup things - it
> either gets consumed by a successful use of the named fd, or it remains
> open on failure and the user can close it by calling 'closefd'.

The way it works now is that the fd/filename pairs that are passed in 
(either through -filefd or getfd_file) will persist on the option and 
monitor structures.  In other words, when we find a match for a filename 
on the monitor structure, we don't delete it from the struct.  We leave 
it there in case we need to open the file again.

So we dup the fd and let QEMU use the new fd as if it opened it itself. 
  This allows QEMU to close the fd on its own, and if it needs to 
re-open the fd, it can dup it again.

>
> Or, if you are intentionally allowing the user to reuse the fd for more
> than one qemu open instance, you need to document this point.

Ok, yes.  I'll document this.

>
> What happens if qemu wants O_WRONLY or O_RDWR access, but the user
> passed in an fd with only O_RDONLY access?

This is an area of concern for me.  And it's an area where Anthony's 
call-back approach was much simpler because it passed the open flags 
directly from QEMU to libvirt.

I don't think these flags can be set through fcntl(F_SETFL).  So I think 
they have to be set on the open() by the managing application.  The 
problem is that, today, QEMU will open a single file several different 
times on initialization alone (reading cow headers and what not), and 
the open flags vary on these open calls.  The difference with the new 
approach in this patch series is that the fd from a single open call is 
re-used for each of the "opens".

-- 
Regards,
Corey