On Wed, Jun 27, 2012 at 11:16:32AM +0200, Kevin Wolf wrote:
Am 27.06.2012 00:54, schrieb Eric Blake:
It seems like libvirt would be in a better position to understand when a file is no longer in use, and then it can call close_fd. No? Of course the the only fd that needs to be closed is the originally passed fd. The dup'd fd's are closed by QEMU.
The more we discuss this, the more I'm convinced that commands like 'block-commit' that will reopen a file to change from O_RDONLY to O_RDWR will need to have an optional argument that says the name of the file to reopen. That is, I've seen three proposals:
Thanks for the summary. In fact, after having read this, I start thinking that we're oversimplifying things because we're only thinking about one use case, block-commit.
There are more: Live snapshot doesn't only put a new image on top, it must also make the old top-level image read-only. This isn't bad per se, but it shows that QMP commands can easily become bloated if you decide to change every command.
The really bad case that nobody thought of is that, when block-commit has finished, we need to switch back to read-only. This is an event that is not triggered by a certain monitor command, but that comes from inside qemu. I'm almost sure that we'll see more of this as we add more asynchronous commands.
This only works if we can pass the new file descriptor in advance. It would work nicely if you go with pass-fd and actually maintain a list of file descriptors for each /dev/fd/N, along with the different flags the file descriptors are meant for. I can't see how it would work with the temporary /dev/fdlist/N or the fd:name approach because they both imply that the original file descriptors are closed by the time that the QMP command returns.
What I don't understand though is that when you're "reopening" FDs, with the pass-fd command approach, you're merely dup'ing the original FDs that was passed in from the client. Why can't you alternatively just dup the FD you already have. I don't see why we need to keep the original FD around forever. If the QMP command handler nees the temporary /dev/fdlist/N file to exist for longer than the duration of the command, it can simply dup() it to get a permanent copy of its own. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|