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

[Daniel Veillard]

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.
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 ?

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/