[libvirt] [PATCH] storage: fix disregarding specified owner/group permissions

[Ryota Ozaki]

Hi Daniel,

On Wed, Mar 11, 2009 at 5:27 PM, Daniel Veillard <veillard at redhat.com> wrote:
> On Mon, Mar 09, 2009 at 09:16:45AM +0900, Ryota Ozaki wrote:
>> Hi,
>>
>> storage_conf.c always sets owner/group permissions as 0, even if non-0 values
>> are specified in XML. Because XPaths in DefParsePerms functions are wrong and
>> XPath functions using the XPaths fail, as a result the values obtained
>> by getpid()/getgid()
>> are used instead.
>>
>> This patch fixes this and also unifies duplicated functions,
>> virStoragePoolDefParsePerms
>> and virStorageVolDefParsePerms, as a common function virStorageDefParsePerms
>> to fix the bug prettily.
>>
>> One concern I have is the default value of mode permission in
>> virStorageDefParsePerms
>> is now 0700 but that in virStorageVolDefParsePerms was 0600. Is this
>> considerable
>> change?
>
>  Hi Ozaki,
>
> the patch looks fine, I like the refactoring idea using relative XPath
> queries to make the lookup code more reusable. For the default mode,
> I think the simplest is to add an extra argument 'defaultmode' to
> virStorageDefParsePerms.

I think that's fine. I will revise that way.

> However I think there is a problem with the patch I could not spot:
> when running the regression tests (make check in a tree checkout)
> I get 11 failing tests after applying the patch (and none before),
> I'm afraid somehow the new code crashes in some case but I could not
> find there (would need to run some of the tests under a debugger).
>
>  Would you mind looking at this ?

umm, I have nothing in mind, but anyway I will investigate that soon.


Thank you for reviewing and sorry for not doing tests before submitting.
  ozaki-r

> Daniel
>
> --
> Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
> daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
> http://veillard.com/ | virtualization library  http://libvirt.org/
>