Re: [libvirt] [PATCH v1 04/23] security_dac: Introduce remember/recall stubs

On Mon, Oct 12, 2015 at 12:25:49 +0200, Michal Privoznik wrote: > These stubs will be worked in later. They merely lay out the > structure of the feature. > > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > --- > src/security/security_dac.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 45 insertions(+) > > diff --git a/src/security/security_dac.c b/src/security/security_dac.c > index 9b079e0..9b53332 100644 > --- a/src/security/security_dac.c > +++ b/src/security/security_dac.c > @@ -184,6 +184,51 @@ virSecurityDACGetImageIds(virSecurityLabelDefPtr seclabel, > return 0; > } > > +/** > + * virSecurityDACRememberLabel: > + * @priv: driver's private data > + * @path: path to the file > + * @uid: user owning the @path > + * @gid: group owning the @path > + * > + * Remember the owner of @path (represented by @uid:@gid). > + * > + * Returns: 0 on success, -1 on failure > + */ > +static int > +ATTRIBUTE_UNUSED I'm not a fan of unused static functions. Since the patch also doesn't contain any explanation in the commit message I'd suggest you merge it to the patch that calls the functions.

> +virSecurityDACRememberLabel(virSecurityDACDataPtr priv ATTRIBUTE_UNUSED, > + const char *path ATTRIBUTE_UNUSED, I'm afraid you will eventually need a more complex data type than just a string. We are already doing DAC ownership manipulation on gluster volumes via the chown callback so we probably want to do the same thing there too at least for disk images.