[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 6/6] block: Enable qemu_open/close to work wi
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH v5 6/6] block: Enable qemu_open/close to work with fd sets |
Date: |
Thu, 26 Jul 2012 11:07:07 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120605 Thunderbird/13.0 |
Am 26.07.2012 05:57, schrieb Corey Bryant:
> On 07/25/2012 03:43 PM, Eric Blake wrote:
>> On 07/23/2012 07:08 AM, Corey Bryant wrote:
>>> +int monitor_fdset_get_fd(Monitor *mon, int64_t fdset_id, int flags)
>>> +{
>>> + mon_fdset_t *mon_fdset;
>>> + mon_fdset_fd_t *mon_fdset_fd;
>>> + int mon_fd_flags;
>>> +
>>> + if (!mon) {
>>> + errno = ENOENT;
>>> + return -1;
>>> + }
>>> +
>>> + QLIST_FOREACH(mon_fdset, &mon->fdsets, next) {
>>> + if (mon_fdset->id != fdset_id) {
>>> + continue;
>>> + }
>>> + QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) {
>>> + if (mon_fdset_fd->removed) {
>>> + continue;
>>> + }
>>> +
>>> + mon_fd_flags = fcntl(mon_fdset_fd->fd, F_GETFL);
>>> + if (mon_fd_flags == -1) {
>>> + return -1;
>>
>> This says we fail on the first fcntl() failure, instead of trying other
>> fds in the set. Granted, an fcntl() failure is probably the sign of a
>> bigger bug (such as closing an fd at the wrong point in time), so I
>> guess trying to go on doesn't make much sense once we already know we
>> are hosed.
>>
>
> I think I'll stick with it the way it is. If fcntl() fails we might
> have a tainted fd set so I think we should fail.
The alternative would be s/return 1/continue/, right? I think either way
is acceptable.
>>> + }
>>> +
>>> + switch (flags & O_ACCMODE) {
>>> + case O_RDWR:
>>> + if ((mon_fd_flags & O_ACCMODE) == O_RDWR) {
>>> + return mon_fdset_fd->fd;
>>> + }
>>> + break;
>>> + case O_RDONLY:
>>> + if ((mon_fd_flags & O_ACCMODE) == O_RDONLY) {
>>> + return mon_fdset_fd->fd;
>>> + }
>>> + break;
>>
>> Do we want to allow the case where the caller asked for O_RDONLY, but
>> the set only has O_RDWR? After all, the caller is getting a compatible
>> subset of what the set offers.
>
> I don't see a problem with it.
I would require exact matches like you implemented, in order to prevent
damage if we ever had a bug that writes to a read-only file. I believe
it also makes the semantics clearer and the code simpler, while it
shouldn't make much of a difference for clients.
Kevin
- Re: [Qemu-devel] [PATCH v5 6/6] block: Enable qemu_open/close to work with fd sets, (continued)
- Re: [Qemu-devel] [PATCH v5 6/6] block: Enable qemu_open/close to work with fd sets, Kevin Wolf, 2012/07/24
- Re: [Qemu-devel] [PATCH v5 6/6] block: Enable qemu_open/close to work with fd sets, Corey Bryant, 2012/07/24
- Re: [Qemu-devel] [PATCH v5 6/6] block: Enable qemu_open/close to work with fd sets, Kevin Wolf, 2012/07/25
- Re: [Qemu-devel] [PATCH v5 6/6] block: Enable qemu_open/close to work with fd sets, Eric Blake, 2012/07/25
- Re: [Qemu-devel] [PATCH v5 6/6] block: Enable qemu_open/close to work with fd sets, Corey Bryant, 2012/07/25
- Re: [Qemu-devel] [PATCH v5 6/6] block: Enable qemu_open/close to work with fd sets, Eric Blake, 2012/07/26
- Re: [Qemu-devel] [PATCH v5 6/6] block: Enable qemu_open/close to work with fd sets, Kevin Wolf, 2012/07/26
- Re: [Qemu-devel] [PATCH v5 6/6] block: Enable qemu_open/close to work with fd sets, Corey Bryant, 2012/07/27
Re: [Qemu-devel] [PATCH v5 6/6] block: Enable qemu_open/close to work with fd sets, Eric Blake, 2012/07/25
[Qemu-devel] [PATCH v5 5/6] block: Convert close calls to qemu_close, Corey Bryant, 2012/07/23
[Qemu-devel] [PATCH v5 1/6] qemu-char: Add MSG_CMSG_CLOEXEC flag to recvmsg, Corey Bryant, 2012/07/23
[Qemu-devel] [PATCH v5 4/6] block: Convert open calls to qemu_open, Corey Bryant, 2012/07/23
[Qemu-devel] [PATCH v5 2/6] qapi: Introduce add-fd, remove-fd, query-fdsets, Corey Bryant, 2012/07/23