Re: [Qemu-devel] Safely reopening image files by stashing fds

[supriya kannery <supriyak@in.ibm.com>]

Kevin Wolf wrote:
> Am 08.08.2011 09:02, schrieb Supriya Kannery:
>   
>> On 08/05/2011 09:19 PM, Anthony Liguori wrote:
>>     
>>> On 08/05/2011 10:43 AM, Kevin Wolf wrote:
>>>       
>>>> Am 05.08.2011 17:24, schrieb Stefan Hajnoczi:
>>>>         
>>>>> On Fri, Aug 5, 2011 at 3:28 PM, Christoph Hellwig<hch@lst.de> wrote:
>>>>>           
>>>>>> On Fri, Aug 05, 2011 at 02:12:48PM +0100, Daniel P. Berrange wrote:
>>>>>>             
>>>>>>>> Because you cannot change O_DIRECT on an open fd :(. This is why
>>>>>>>> we're going through this pain.
>>>>>>>>                 
>>>>>>> Hmm, I remember hearing that before, but looking at the current
>>>>>>> fcntl()
>>>>>>> manpage, it claims you *can* change O_DIRECT using SET_FL. Perhaps
>>>>>>> this
>>>>>>> is a newish feature, but it'd be nicer to use it if possible ?
>>>>>>>               
>>>>>> It's been there since day 1 of O_DIRECT support.
>>>>>>             
>>>>> Sorry, my bad. So for Linux we could just use fcntl for
>>>>> block_set_hostcache and not bother with reopening. However, we will
>>>>> need to reopen should we wish to support changing O_DSYNC.
>>>>>           
>>>> We do wish to support that.
>>>>
>>>> Anthony thinks that allowing the guest to toggle WCE is a prerequisite
>>>> for making cache=writeback the default. And this is something that I
>>>> definitely want to do for 1.0.
>>>>         
>>> Indeed.
>>>
>>>       
>> We discussed the following so far...
>> 1. How to safely reopen image files
>> 2. Dynamic hostcache change
>> 3. Support for dynamic change of O_DSYNC
>>
>> Since 2 is independent of 1, shall I go ahead implementing
>> hostcache change using fcntl.
>>
>> Implementation for safely reopening image files using "BDRVReopenState"
>> can be done separately as a pre-requisite before implementing 3
>>     
>
> Doing it separately means that we would introduce yet another callback
> that is used just to change O_DIRECT. In the end we want it to use
> bdrv_reopen(), too, so I'm not sure if there is a need for a temporary
> solution.
>
>   
Could you please explain "In the end we want it to use bdrv_reopen" at 
bit more.
When fcntl() can change O_DIRECT on  open fd , is there a  need to 
"re-open"
the image file?

Considering the current way of having separate high level commands for
changing block parameters (block_set_hostcache, and may be block_set_flush
in furture), these dynamic requests will be sequential. So wouldn't it 
be better to
avoid re-opening of image if possible for individual flag change request 
that comes in?
> Actually, once we know what we really want (I haven't seen many comments
> on the BDRVReopenState suggestion yet), it should be pretty easy to
> implement.
>
> Kevin
>   
Will work on to get an RFC patch with this proposed BDRVReopenState to 
get more
inputs.