[Top][All Lists]

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

Re: [Qemu-devel] [PATCH v3 3/5] osdep: Enable qemu_open to dup pre-opene

From: Corey Bryant
Subject: Re: [Qemu-devel] [PATCH v3 3/5] osdep: Enable qemu_open to dup pre-opened fd
Date: Fri, 15 Jun 2012 16:49:12 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120605 Thunderbird/13.0

On 06/15/2012 04:00 PM, Eric Blake wrote:
On 06/15/2012 01:19 PM, Corey Bryant wrote:

There are some flags that I don't think we'll be able to change.  For
example: O_RDONLY, O_WRONLY, O_RDWR.  I assume libvirt would open all
files O_RDWR.

I think we need to check all of them and fail qemu_open() if they don't
match. Those that qemu can change, should be just changed, of course.

Ok.  I remember a scenario where QEMU opens a file read-only (perhaps to
check headers and determine the file format) before re-opening it
read-write.  Perhaps this is only when format= isn't specified with
-drive.  I'm thinking we may need to change flags to read-write where
they used to be read-only, in some circumstances.

In those situations, libvirt would pass fd with O_RDWR, and qemu_open()
would be fine requesting O_RDONLY the first time (subset is okay), and
O_RDWR the second time.  Where you have to error out is where libvirt
passes O_RDONLY but qemu wants O_RDWR, and so forth.

I'll plan on going with this approach.

In which scenario would any client break if we set FD_CLOEXEC? I don't
think compatibility means we can't fix any bugs.

I don't know if it breaks any client.  Maybe it's not a compatibility
error.  It dopes change behavior down the line though.  If you think


it's ok to set FD_CLOEXEC for getfd too, then I'm happy to do it.

The only case that a client might break is if there were a way to pass
an fd into qemu and then intentionally see that fd in a child process of
qemu.  But in the case of 'migrate fd:nnn', you aren't spawning a child
process, and even in the case of 'migrate exec:command' (which libvirt
no longer uses if fd:nnn works), I don't see how the client could have
ever intentionally tried to use 'getfd' in advance to pass an extra fd
for use inside the 'exec:command' child.  Besides, before 'pass-fd' was
around, how would the management app triggering the 'exec:command' even
know what fd number would accidentally be inherited into the
exec:command child?  I think it is pretty much a straight bug-fix for
'getfd' to always set FD_CLOEXEC, and preferably set it atomically via

Alright, I'll go ahead and make this update in the next version of the patch series.

Thanks for all the input!


reply via email to

[Prev in Thread] Current Thread [Next in Thread]