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

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





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


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