Re: [libvirt] [PATCHv3] security: Don't ignore errors when parsing DAC security labels

On 09/19/2012 02:42 PM, Peter Krempa wrote: > The DAC security driver silently ignored errors when parsing the DAC > label and used default values instead. > > With a domain containing the following label definition: > > <seclabel type='static' model='dac' relabel='yes'> > <label>sdfklsdjlfjklsdjkl</label> > </seclabel> > > the domain would start normaly but the disk images would be still owned > by root and no error was displayed. > > This patch changes the behavior if the parsing of the label fails (note > that a not present label is not a failure and in this case the default > label should be used) the error isn't masked but is raised that causes > the domain start to fail with a descriptive error message: > > virsh # start tr > error: Failed to start domain tr > error: internal error invalid argument: failed to parse DAC label > 'sdfklsdjlfjklsdjkl' for domain 'tr' > > I also changed the error code to "invalid argument" from "internal > error" and tweaked the various error messages to contain correct and > useful information. > --- > Diff to v2: > - Fixed all error reporting paths to contain useful messages > - Tweaked error messages to contain more information > - Fixed printing of seclabel->label instead of seclabel->imagelabel in the imagelabel parsing func. > > src/security/security_dac.c | 95 +++++++++++++++++++++++++++++---------------- > 1 file changed, 61 insertions(+), 34 deletions(-) >

> > Code seems fine, though, so ACK with unifying those messages. I suggest either "seclabel" and "imagelabel" OR 'security label" and "security imagelabel", whatever you think suits the error and XML better, but one for all errors with the same label.