Am 06.09.2012 17:34, schrieb Corey Bryant:
On 09/06/2012 05:23 AM, Kevin Wolf wrote:
Am 05.09.2012 18:43, schrieb Jeff Cody:
+ }
+
+ int fcntl_flags = O_APPEND | O_ASYNC | O_NONBLOCK;
+#ifdef O_NOATIME
+ fcntl_flags |= O_NOATIME;
+#endif
+ if ((raw_s->open_flags & ~fcntl_flags) == (s->open_flags & ~fcntl_flags)) {
+ /* dup the original fd */
+ /* TODO: use qemu fcntl wrapper */
+ raw_s->fd = fcntl(s->fd, F_DUPFD_CLOEXEC, 0);
+ if (raw_s->fd == -1) {
+ ret = -1;
+ goto error;
+ }
+ ret = fcntl_setfl(raw_s->fd, raw_s->open_flags);
+ } else {
+ raw_s->fd = qemu_open(state->bs->filename, raw_s->open_flags, 0644);
+ if (raw_s->fd == -1) {
+ ret = -1;
+ }
Ignoring this part for now, with qemu_dup_flags() it's going to look a
bit different. In particular, I'm hoping that we don't get a second
fcntl_flags enumeration here, but can just fall back to qemu_open()
whenever qemu_dup_flags() fails.
That will require modification to qemu_dup_flags()... I believe
qemu_dup_flags() silently filters out fcntl incompatible flags.
Maybe it would be best to create a small helper function in osdep.c, that
fetches the fcntl_flags. Then qemu_dup_flags() and this function would
use the same helper to fetch fcntl_flags. The results of that would
determine if we call qemu_dup_flags() or qemu_open().
Although, I do think it makes sense to always try qemu_open() if
qemu_dup_flags() fails for some reason.
I'm curious why you can't always call qemu_open().
I believe the original reason was that qemu_open() is more likely to
fail, for example if the image file has been renamed/moved/deleted since
the first open. You could still use fcntl() on an existing file
descriptor, but reopening would fail.
Some things to consider so that fd passing doesn't break when a reopen
occurs. Mainly all the concerns revolve around how fd passing keeps
track of references to fd sets (note: adding and removing fd set
references is all done in qemu_open and qemu_close).
* When reopening, qemu_open needs to be called before qemu_close. This
will prevent the reference list for an fdset from becoming empty. If
qemu_close is called before qemu_open, the reference list can become
empty, and the fdset could be cleaned up before the qemu_open. Then
qemu_open would fail.
Will automatically be right when we properly implement transactional
semantics.
* qemu_open/qemu_close need to be used rather than open/close so that
the references for fd passing are properly accounted for.
Congratulations, you've just discovered a bug in Jeff's patches. It was
a good idea to CC you. ;-)
* I don't think you want to call qemu_dup_flags directly since it
doesn't update the reference list for fd passing. Only qemu_open and
qemu_close update the reference list.
That's a good point, too. So probably a small wrapper that just updates
the reference list in addition?