On 10/08/12 19:22, Eric Blake wrote:
On 10/08/2012 07:52 AM, Marcelo Cerri wrote:
On Mon, Oct 08, 2012 at 02:11:26PM +0200, Peter Krempa wrote:
On 10/06/12 04:52, Marcelo Cerri wrote:
The functions virGetUserID and virGetGroupID are now able to parse
user/group names and IDs in a similar way to coreutils' chown. So, user
and group parsing in security_dac can be simplified.
---
src/security/security_dac.c | 40 ++++++++++------------------------------
1 file changed, 10 insertions(+), 30 deletions(-)
+ if (virGetGroupID(group, &thegid) < 0) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("Invalid group \"%s\" in DAC label \"%s\""),
+ group, label);
Hm, this will mask the errors from virGetGroupID that might be
useful. Should we remove this message in favor of the underlying
messages or have this that has more relevant information where to
find the issue but maybe mask the reason?
I thought about that when I was testing these changes and it seemed more
useful for me when the message points to where the problem is, in this
case the DAC label.
But you are right, it will mask the reason why a DAC label couldn't be
parsed.
Any opinions?
I think it will be pretty obvious from the virGetGroupID() error message
that the failure came from inability to parse a group id string, even if
we don't pinpoint which DAC label contained that string. I think it's
simpler to just reuse the already-present error than to attempt nesting
the messages.
I agree, the nesting doesn't look good. (Too bad we don't have native
support for error message stacks.) I think it could be slightly more
helpful to state that the DAC label contains the problem but not at the
cost of masking the real error from beneath.
Peter
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
