On Thu, Nov 29, 2018 at 02:52:18PM +0100, Michal Privoznik wrote:
> This file implements wrappers over XATTR getter/setter. It
> ensures the proper XATTR namespace is used.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> src/security/Makefile.inc.am | 2 +
> src/security/security_util.c | 226 +++++++++++++++++++++++++++++++++++
> src/security/security_util.h | 32 +++++
> 3 files changed, 260 insertions(+)
> create mode 100644 src/security/security_util.c
> create mode 100644 src/security/security_util.h
>
> +/**
> + * virSecurityGetRememberedLabel:
> + * @name: security driver name
> + * @path: file name
> + * @label: label
> + *
> + * For given @path and security driver (@name) fetch remembered
> + * @label. The caller must not restore label if an error is
> + * indicated or if @label is NULL upon return.
> + *
> + * Returns: 0 on success,
> + * -1 otherwise (with error reported)
> + */
> +int
> +virSecurityGetRememberedLabel(const char *name,
> + const char *path,
> + char **label)
> +{
> + char *ref_name = NULL;
> + char *attr_name = NULL;
> + char *value = NULL;
> + unsigned int refcount = 0;
> + int ret = -1;
> +
> + *label = NULL;
> +
> + if (!(ref_name = virSecurityGetRefCountAttrName(name)))
> + goto cleanup;
> +
> + if (virFileGetXAtrr(path, ref_name, &value) < 0) {
> + if (errno == ENOSYS || errno == ENODATA || errno == ENOTSUP) {
> + ret = 0;
> + } else {
> + virReportSystemError(errno,
> + _("Unable to get XATTR %s on %s"),
> + ref_name,
> + path);
> + }
> + goto cleanup;
> + }
> +
> + if (virStrToLong_ui(value, NULL, 10, &refcount) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("malformed refcount %s on %s"),
> + value, path);
> + goto cleanup;
> + }
> +
> + VIR_FREE(value);
> +
> + refcount--;
> +
> + if (refcount > 0) {
> + if (virAsprintf(&value, "%u", refcount) < 0)
> + goto cleanup;
> +
> + if (virFileSetXAtrr(path, ref_name, value) < 0)
> + goto cleanup;
> + } else {
> + if (virFileRemoveXAttr(path, ref_name) < 0)
> + goto cleanup;
> +
> + if (!(attr_name = virSecurityGetAttrName(name)))
> + goto cleanup;
> +
> + if (virFileGetXAtrr(path, attr_name, label) < 0)
> + goto cleanup;
I'm not understanding why we only fetch the label value in
this half of the conditional ? Shouldn't we be unconditionally
getting the label, regardless of the updated refcount value.
If not, the method description could have an explanation of
intended behaviour.
> +
> + if (virFileRemoveXAttr(path, attr_name) < 0)
> + goto cleanup;
> + }
> +
> + ret = 0;
> + cleanup:
> + VIR_FREE(value);
> + VIR_FREE(attr_name);
> + VIR_FREE(ref_name);
> + return ret;
> +}
> +
> +
> +int
> +virSecuritySetRememberedLabel(const char *name,
> + const char *path,
> + const char *label)
> +{
> + char *ref_name = NULL;
> + char *attr_name = NULL;
> + char *value = NULL;
> + unsigned int refcount = 0;
> + int ret = -1;
> +
> + if (!(ref_name = virSecurityGetRefCountAttrName(name)))
> + goto cleanup;
> +
> + if (virFileGetXAtrr(path, ref_name, &value) < 0) {
> + if (errno == ENOSYS || errno == ENOTSUP) {
> + ret = 0;
> + goto cleanup;
> + } else if (errno != ENODATA) {
> + virReportSystemError(errno,
> + _("Unable to get XATTR %s on %s"),
> + ref_name,
> + path);
> + goto cleanup;
> + }
> + }
> +
> + if (value &&
> + virStrToLong_ui(value, NULL, 10, &refcount) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("malformed refcount %s on %s"),
> + value, path);
> + goto cleanup;
> + }
> +
> + VIR_FREE(value);
> +
> + refcount++;
> +
> + if (refcount == 1) {
> + if (!(attr_name = virSecurityGetAttrName(name)))
> + goto cleanup;
> +
> + if (virFileSetXAtrr(path, attr_name, label) < 0)
> + goto cleanup;
> + }
Do we need to have a
} else {
.... check the currently remember label matches
what was passed into this method ?
}
if not, can you add API docs for this method explainng the
intended semantics when label is already remembered.
> +
> + if (virAsprintf(&value, "%u", refcount) < 0)
> + goto cleanup;
> +
> + if (virFileSetXAtrr(path, ref_name, value) < 0)
> + goto cleanup;
> +
> + ret = 0;
> + cleanup:
> + VIR_FREE(value);
> + VIR_FREE(attr_name);
> + VIR_FREE(ref_name);
> + return ret;
> +}
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
