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

[Eric Blake]

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.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110524/34cdb1de/attachment-0001.sig>