[libvirt] [PATCH] qemu: Don't chown files on NFS share if dynamic_ownership is off

[Michal Privoznik]

On 07.07.2011 20:12, Laine Stump wrote:
> On 07/07/2011 12:00 PM, Michal Privoznik wrote:
>> On 07.07.2011 17:52, Eric Blake wrote:
>>> On 07/07/2011 09:33 AM, Michal Privoznik wrote:
>>>> When dynamic ownership is disabled we don't want to chown any files,
>>>> not just local.
>>> Is there more details on a scenario where this was causing an issue?
>>> Either a BZ number or a set of steps to reproduce the problem.
>>>
>>>> ---
>>>>   src/qemu/qemu_driver.c |    5 ++---
>>>>   1 files changed, 2 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>>>> index 52b7dfd..968865f 100644
>>>> --- a/src/qemu/qemu_driver.c
>>>> +++ b/src/qemu/qemu_driver.c
>>>> @@ -2163,11 +2163,10 @@ 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
>>>> +        /* If the path is regular file which exists
>>>>            * already and dynamic_ownership is off, we don't
>>>>            * want to change it's ownership, just open it as-is */
>>>> -        if (is_reg&&  !driver->dynamicOwnership&&
>>>> -            virStorageFileIsSharedFS(path) == 0) {
>>>> +        if (is_reg&&  !driver->dynamicOwnership) {
>>> The code change looks fine, but without a pointer to a reproducer case
>>> proving that it is a bug fix, I'm not sure if this would have unintended
>>> consequences.
>>>
>> Sure, https://bugzilla.redhat.com/show_bug.cgi?id=716478
> 
> 
> And I can't think of any reason why it *should* check for local (if
> anything, we should do *less* changing of ownership on remote
> filesystems, not more). Oh, and this is fairly recent code, so there
> won't be anybody relying on the old behavior. So ACK.

Thanks, pushed.

Michal