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

Re: [libvirt] [PATCH] qemu: Don't change ownership of file when appending to it



On 24.05.2011 17:17, Eric Blake wrote:
> On 05/24/2011 09:12 AM, Daniel Veillard wrote:
>> On Tue, May 24, 2011 at 02:54:28PM +0200, Michal Privoznik wrote:
>>> Saving domain to previously created file changes also its ownership.
>>> This is certainly not what users want if some conditions are met:
>>> it is a regular, local file and dynamic_ownership is off.
>>> ---
>>>  src/qemu/qemu_driver.c |   12 +++++++++++-
>>>  1 files changed, 11 insertions(+), 1 deletions(-)
>>>
> 
>>> @@ -2013,6 +2015,14 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom,
>>>          is_reg = true;
>>>      } else {
>>>          is_reg = !!S_ISREG(sb.st_mode);
>>> +        /* If the path is regular local file which exists
>>> +         * already and dynamic_ownership is off, we don't
>>> +         * want to change it's ownership, just append the data */
>>> +        if (is_reg && !driver->dynamicOwnership &&
>>> +            virStorageFileIsSharedFS(path) == 0) {
>>> +            uid=sb.st_uid;
>>> +            gid=sb.st_gid;
> 
> The comment is misleading - we aren't using O_APPEND, but O_TRUNC (that
> is, we are keeping the same inode and file, but rewriting its entire
> contents, rather than appending to existing contents).
> 
> How about:
> 
> s/just append the data/just open it as-is/
> 
>>   The explaination sounds fine, and patch seems to implement just this,
>>
>>     ACK,
>>
>> but maybe give a 24 hours grace period for others to review it too, as
>> I'm not 100% sure :-)
> 
> You've also got my ACK with the comment tweak.
> 
Fixed & pushed. Thanks.


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