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

[Supriya Kannery <supriyak@linux.vnet.ibm.com>]

On 10/10/2011 11:58 PM, Kevin Wolf wrote:
> Am 09.08.2011 11:22, schrieb supriya kannery:
>> 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.
>
> Are you still going to prepare an RFC patch implementing
> bdrv_reopen_prepare/commit/abort using a BDRVReopenState? Or has it even
> been posted and I just missed it?
>
> Kevin

Pls review the patchset posted at
http://lists.gnu.org/archive/html/qemu-devel/2011-10/msg01014.html

Working on further to similarly use BDRVReopenState for raw-win32 images.

thanks, Supriya