[Qemu-devel] Re: [PATCH 3/3] raw-posix: Re-open host CD-ROM after media change

[Juan Quintela <quintela@redhat.com>]

Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Thu, Mar 24, 2011 at 12:42 PM, Kevin Wolf <kwolf@redhat.com> wrote:
>> Am 23.03.2011 21:50, schrieb Stefan Hajnoczi:
>>> On Wed, Mar 23, 2011 at 8:27 PM, Juan Quintela <quintela@redhat.com> wrote:
>>>> Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> wrote:
>>>>> +
>>>>> +    if (s->fd == -1) {
>>>>> +        s->fd = qemu_open(bs->filename, s->open_flags, 0644);
>>>>
>>>> Everything else on that file uses plain "open" not "qemu_open".
>>>> diference is basically that qemu_open() adds flag O_CLOEXEC.
>>>>
>>>> I don't know if this one should be vanilla open or the other ones
>>>> qemu_open().
>>>>
>>>> What do you think?
>>>
>>> raw_open_common() uses qemu_open().  That's why I used it.
>>
>> And I think it's correct. There's no reason not to set O_CLOEXEC here.
>> Maybe some of the open() users need to be fixed.

I didn't doubt that.  What I tried to point is that there are three
opens for cdrom/floppy on that file.  It makes sense that all are the
same.  I guessed that proper fix was to change all others to
qemu_open(), but just wanted to point that it was inconsistent, and
should be done one or other way.

>>>>> +        if (s->fd < 0) {
>>>>> +            return 0;
>>>>> +        }
>>>>> +    }
>>>>> +
>>>>> +    ret = (ioctl(s->fd, CDROM_DRIVE_STATUS, CDSL_CURRENT) == CDS_DISC_OK);
>>>>
>>>> parens are not needed around ==.
>>>
>>> Yes, if you want I'll remove them.  I just did it for readability.
>>
>> I like them.
>
> I will have to #ifdef QUINTELA and #ifdef KWOLF :).  Seriously, I'll
> leave the braces unless anyone feels really strongly either way.  This
> passes checkpatch.pl BTW.

In

x = (a == b);

braces are useless.  But my preference is not 'strong' enough to try to
force it on other people.

It appears that other people feel strong about it, so let it be.

Later, Juan.