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.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list