[libvirt] [PATCH v2 5/5] Update the remote API
Michal Privoznik
mprivozn at redhat.com
Thu Jul 19 12:45:23 UTC 2012
re: $SUBJ
well, you are introducing whole new API, so it is not just a remote :)
And maybe it's worth splitting into separate commits as advised here:
http://libvirt.org/api_extension.html
But from time to time we accept new API completely introduced,
documented and implemented in one patch. So I am cointinuing
On 18.07.2012 03:28, Marcelo Cerri wrote:
> Thist patch updates libvirt's API to allow applications to inspect the
> full list of security labels of a domain.
> ---
> daemon/remote.c | 62 ++++++++++++++++++++++++++++++++++++
> include/libvirt/libvirt.h.in | 2 +
> python/generator.py | 1 +
> src/driver.h | 4 ++
> src/libvirt.c | 47 +++++++++++++++++++++++++++
> src/libvirt_public.syms | 7 ++++
> src/qemu/qemu_driver.c | 71 ++++++++++++++++++++++++++++++++++++++++++
> src/remote/remote_driver.c | 46 +++++++++++++++++++++++++++
> src/remote/remote_protocol.x | 18 +++++++++-
> src/remote_protocol-structs | 1 +
> 10 files changed, 257 insertions(+), 2 deletions(-)
>
> diff --git a/daemon/remote.c b/daemon/remote.c
> index 9334221..aef6cc1 100644
> --- a/daemon/remote.c
> +++ b/daemon/remote.c
> @@ -1424,6 +1424,68 @@ cleanup:
> }
>
> static int
> +remoteDispatchDomainGetSecurityLabelList(virNetServerPtr server ATTRIBUTE_UNUSED,
> + virNetServerClientPtr client ATTRIBUTE_UNUSED,
> + virNetMessagePtr msg ATTRIBUTE_UNUSED,
> + virNetMessageErrorPtr rerr,
> + remote_domain_get_security_label_list_args *args,
> + remote_domain_get_security_label_list_ret *ret)
> +{
> + virDomainPtr dom = NULL;
> + virSecurityLabelPtr seclabels = NULL;
> + int i, len, rv = -1;
> + struct daemonClientPrivate *priv =
> + virNetServerClientGetPrivateData(client);
> +
> + if (!priv->conn) {
> + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
> + goto cleanup;
> + }
> +
> + if (!(dom = get_nonnull_domain(priv->conn, args->dom)))
> + goto cleanup;
> +
> + if ((len = virDomainGetSecurityLabelList(dom, &seclabels)) < 0) {
> + ret->ret = len;
> + ret->labels.labels_len = 0;
> + ret->labels.labels_val = NULL;
> + goto done;
> + }
> +
> + if (VIR_ALLOC_N(ret->labels.labels_val, len) < 0) {
> + virReportOOMError();
> + goto cleanup;
> + }
> +
> + for (i = 0; i < len; i++) {
> + size_t label_len = strlen(seclabels[i].label) + 1;
> + remote_domain_get_security_label_ret *cur = &ret->labels.labels_val[i];
> + if (VIR_ALLOC_N(cur->label.label_val, label_len) < 0) {
> + virReportOOMError();
> + goto cleanup;
> + }
> + if (virStrcpy(cur->label.label_val, seclabels[i].label, label_len) == NULL) {
> + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to copy security label"));
> + goto cleanup;
> + }
> + cur->label.label_len = label_len;
> + cur->enforcing = seclabels[i].enforcing;
> + }
> + ret->labels.labels_len = ret->ret = len;
> +
> +done:
> + rv = 0;
> +
> +cleanup:
> + if (rv < 0)
> + virNetMessageSaveError(rerr);
> + if (dom)
> + virDomainFree(dom);
> + VIR_FREE(seclabels);
> + return rv;
> +}
Side note - since we have some APIs malloc()-ating on client side it
might be worth learning RPC generator to deal with them so we don't have
to implement them by hand.
> +
> +static int
> remoteDispatchNodeGetSecurityModel(virNetServerPtr server ATTRIBUTE_UNUSED,
> virNetServerClientPtr client ATTRIBUTE_UNUSED,
> virNetMessagePtr msg ATTRIBUTE_UNUSED,
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index e34438c..eb48b99 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -1540,6 +1540,8 @@ int virDomainSetMemoryFlags (virDomainPtr domain,
> int virDomainGetMaxVcpus (virDomainPtr domain);
> int virDomainGetSecurityLabel (virDomainPtr domain,
> virSecurityLabelPtr seclabel);
> +int virDomainGetSecurityLabelList (virDomainPtr domain,
> + virSecurityLabelPtr* seclabels);
>
> typedef enum {
> VIR_DOMAIN_METADATA_DESCRIPTION = 0, /* Operate on <description> */
> diff --git a/python/generator.py b/python/generator.py
> index 2dada6e..f3c8912 100755
> --- a/python/generator.py
> +++ b/python/generator.py
> @@ -446,6 +446,7 @@ skip_function = (
> 'virConnectOpenAuth', # Python C code is manually written
> 'virDefaultErrorFunc', # Python virErrorFuncHandler impl calls this from C
> 'virDomainGetSecurityLabel', # Needs investigation...
> + 'virDomainGetSecurityLabelList', # Needs investigation...
> 'virNodeGetSecurityModel', # Needs investigation...
> 'virConnectDomainEventRegister', # overridden in virConnect.py
> 'virConnectDomainEventDeregister', # overridden in virConnect.py
> diff --git a/src/driver.h b/src/driver.h
> index b3c1740..fd6a1fe 100644
> --- a/src/driver.h
> +++ b/src/driver.h
> @@ -314,6 +314,9 @@ typedef int
> (*virDrvDomainGetSecurityLabel) (virDomainPtr domain,
> virSecurityLabelPtr seclabel);
> typedef int
> + (*virDrvDomainGetSecurityLabelList) (virDomainPtr domain,
> + virSecurityLabelPtr* seclabels);
> +typedef int
> (*virDrvNodeGetSecurityModel) (virConnectPtr conn,
> virSecurityModelPtr secmodel);
> typedef int
> @@ -934,6 +937,7 @@ struct _virDriver {
> virDrvDomainGetVcpus domainGetVcpus;
> virDrvDomainGetMaxVcpus domainGetMaxVcpus;
> virDrvDomainGetSecurityLabel domainGetSecurityLabel;
> + virDrvDomainGetSecurityLabelList domainGetSecurityLabelList;
> virDrvNodeGetSecurityModel nodeGetSecurityModel;
> virDrvDomainGetXMLDesc domainGetXMLDesc;
> virDrvConnectDomainXMLFromNative domainXMLFromNative;
> diff --git a/src/libvirt.c b/src/libvirt.c
> index df78e8a..912fd10 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -9008,6 +9008,53 @@ error:
> }
>
> /**
> + * virDomainGetSecurityLabelList:
> + * @domain: a domain object
> + * @seclabels: will be auto-allocated and filled with domains' security labels.
> + * Caller must free memory on return.
> + *
> + * Extract the security labels of an active domain. The 'label' field
> + * in the @seclabels argument will be initialized to the empty
> + * string if the domain is not running under a security model.
> + *
> + * Returns 0 in case of success, -1 in case of failure
> + */
> +int
> +virDomainGetSecurityLabelList(virDomainPtr domain,
> + virSecurityLabelPtr* seclabels)
> +{
> + virConnectPtr conn;
> +
> + VIR_DOMAIN_DEBUG(domain, "seclabels=%p", seclabels);
> +
> + if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virDispatchError(NULL);
> + return -1;
> + }
> +
> + if (seclabels == NULL) {
> + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
> + goto error;
> + }
> +
> + conn = domain->conn;
> +
> + if (conn->driver->domainGetSecurityLabelList) {
> + int ret;
> + ret = conn->driver->domainGetSecurityLabelList(domain, seclabels);
> + if (ret < 0)
> + goto error;
> + return ret;
> + }
> +
> + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
> +
> +error:
> + virDispatchError(domain->conn);
> + return -1;
> +}
> +/**
> * virDomainSetMetadata:
> * @domain: a domain object
> * @type: type of description, from virDomainMetadataType
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index 2913a81..21b83e7 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -542,6 +542,13 @@ LIBVIRT_0.9.13 {
> virDomainSnapshotIsCurrent;
> virDomainSnapshotListAllChildren;
> virDomainSnapshotRef;
> + virDomainGetSecurityLabelList;
No! This API was N/A in 0.9.13 release.
> } LIBVIRT_0.9.11;
>
> +LIBVIRT_0.9.14 {
I guess Eric mentioned on the list that the next release is going to be
0.10.0 since we are adding support for new hypervisor. But you are not
the first to mention 0.9.14 so we can substitute all occurrences just
before release.
> + global:
> + virDomainGetSecurityLabelList;
> +} LIBVIRT_0.9.13;
> +
> +
> # .... define new API here using predicted next version number ....
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index f01566b..06a4e44 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4091,6 +4091,76 @@ cleanup:
> return ret;
> }
>
> +static int qemudDomainGetSecurityLabelList(virDomainPtr dom,
> + virSecurityLabelPtr* seclabels)
qemud prefix is ancient and kept in some internal APIs for historical
reasons (when libvirtd wasn't called libvirtd but qemud). Not a show
stopper, though.
> +{
> + struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData;
Typecasting is not necessary.
> + virDomainObjPtr vm;
> + int i, ret = -1;
> +
> + qemuDriverLock(driver);
> + vm = virDomainFindByUUID(&driver->domains, dom->uuid);
> +
> + if (!vm) {
> + char uuidstr[VIR_UUID_STRING_BUFLEN];
> + virUUIDFormat(dom->uuid, uuidstr);
> + qemuReportError(VIR_ERR_NO_DOMAIN,
> + _("no domain with matching uuid '%s'"), uuidstr);
> + goto cleanup;
> + }
> +
> + if (!virDomainVirtTypeToString(vm->def->virtType)) {
> + qemuReportError(VIR_ERR_INTERNAL_ERROR,
> + _("unknown virt type in domain definition '%d'"),
> + vm->def->virtType);
> + goto cleanup;
> + }
> +
> + /*
> + * Check the comment in qemudDomainGetSecurityLabel function.
> + */
> + if (!virDomainObjIsActive(vm)) {
> + /* No seclabels */
> + *seclabels = NULL;
> + ret = 0;
> + } else {
> + int len = 0;
> + virSecurityManagerPtr* mgrs = virSecurityManagerGetNested(
> + driver->securityManager);
I wonder if we need to keep qemu driver locked through whole API even if
we are dereferencing securityManager (which, however, doesn't change
over time).
> + if (!mgrs)
> + goto cleanup;
> +
> + /* Allocate seclabels array */
> + for (i = 0; mgrs[i]; i++)
> + len++;
> +
> + if (VIR_ALLOC_N((*seclabels), len) < 0) {
mgrs just leaked here
> + virReportOOMError();
> + goto cleanup;
> + }
> + memset(*seclabels, 0, sizeof(**seclabels) * len);
> +
> + /* Fill the array */
> + for (i = 0; i < len; i++) {
> + if (virSecurityManagerGetProcessLabel(mgrs[i], vm->def, vm->pid,
> + &(*seclabels)[i]) < 0) {
> + qemuReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("Failed to get security label"));
> + VIR_FREE(mgrs);
> + VIR_FREE(*seclabels);
> + goto cleanup;
> + }
> + }
> + ret = len;
> + VIR_FREE(mgrs);
> + }
> +
> +cleanup:
> + if (vm)
> + virDomainObjUnlock(vm);
> + qemuDriverUnlock(driver);
> + return ret;
> +}
> static int qemudNodeGetSecurityModel(virConnectPtr conn,
> virSecurityModelPtr secmodel)
> {
> @@ -13296,6 +13366,7 @@ static virDriver qemuDriver = {
> .domainGetVcpus = qemudDomainGetVcpus, /* 0.4.4 */
> .domainGetMaxVcpus = qemudDomainGetMaxVcpus, /* 0.4.4 */
> .domainGetSecurityLabel = qemudDomainGetSecurityLabel, /* 0.6.1 */
> + .domainGetSecurityLabelList = qemudDomainGetSecurityLabelList, /* ? */
s/\?/0\.9\.14/
> .nodeGetSecurityModel = qemudNodeGetSecurityModel, /* 0.6.1 */
> .domainGetXMLDesc = qemuDomainGetXMLDesc, /* 0.2.0 */
> .domainXMLFromNative = qemuDomainXMLFromNative, /* 0.6.4 */
> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
> index 3314f80..3c08ae9 100644
> --- a/src/remote/remote_driver.c
> +++ b/src/remote/remote_driver.c
> @@ -1929,6 +1929,51 @@ done:
> }
>
> static int
> +remoteDomainGetSecurityLabelList (virDomainPtr domain, virSecurityLabelPtr* seclabels)
> +{
> + remote_domain_get_security_label_list_args args;
> + remote_domain_get_security_label_list_ret ret;
> + struct private_data *priv = domain->conn->privateData;
> + int i, rv = -1;
> +
> + remoteDriverLock(priv);
> +
> + make_nonnull_domain (&args.dom, domain);
> + memset(&ret, 0, sizeof(ret));
> +
> + if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_SECURITY_LABEL_LIST,
> + (xdrproc_t) xdr_remote_domain_get_security_label_list_args, (char *)&args,
> + (xdrproc_t) xdr_remote_domain_get_security_label_list_ret, (char *)&ret) == -1) {
> + goto done;
> + }
> +
> + if (VIR_ALLOC_N(*seclabels, ret.labels.labels_len) < 0)
> + goto cleanup;
> +
> + for (i = 0; i < ret.labels.labels_len; i++) {
> + remote_domain_get_security_label_ret *cur = &ret.labels.labels_val[i];
> + if (cur->label.label_val != NULL) {
> + if (strlen(cur->label.label_val) >= sizeof((*seclabels)->label)) {
> + remoteError(VIR_ERR_RPC, _("security label exceeds maximum: %zd"),
> + sizeof((*seclabels)->label) - 1);
> + VIR_FREE(*seclabels);
> + goto cleanup;
> + }
> + strcpy((*seclabels)[i].label, cur->label.label_val);
> + (*seclabels)[i].enforcing = cur->enforcing;
> + }
> + }
> + rv = ret.ret;
> +
> +cleanup:
> + xdr_free((xdrproc_t) xdr_remote_domain_get_security_label_list_ret, (char *)&ret);
> +
> +done:
> + remoteDriverUnlock(priv);
> + return rv;
> +}
> +
> +static int
> remoteDomainGetState(virDomainPtr domain,
> int *state,
> int *reason,
> @@ -5226,6 +5271,7 @@ static virDriver remote_driver = {
> .domainGetVcpus = remoteDomainGetVcpus, /* 0.3.0 */
> .domainGetMaxVcpus = remoteDomainGetMaxVcpus, /* 0.3.0 */
> .domainGetSecurityLabel = remoteDomainGetSecurityLabel, /* 0.6.1 */
> + .domainGetSecurityLabelList = remoteDomainGetSecurityLabelList, /* 0.9.13 */
s/13/14/
> .nodeGetSecurityModel = remoteNodeGetSecurityModel, /* 0.6.1 */
> .domainGetXMLDesc = remoteDomainGetXMLDesc, /* 0.3.0 */
> .domainXMLFromNative = remoteDomainXMLFromNative, /* 0.6.4 */
> diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
> index 8f1d9b5..27d34b1 100644
> --- a/src/remote/remote_protocol.x
> +++ b/src/remote/remote_protocol.x
> @@ -169,6 +169,11 @@ const REMOTE_DOMAIN_BLOCK_PEEK_BUFFER_MAX = 1048576;
> const REMOTE_DOMAIN_MEMORY_PEEK_BUFFER_MAX = 1048576;
>
> /*
> + * Maximum length of a security label list.
> + */
> +const REMOTE_SECURITY_LABEL_LIST_MAX=64;
> +
> +/*
> * Maximum length of a security model field.
> */
> const REMOTE_SECURITY_MODEL_MAX = VIR_SECURITY_MODEL_BUFLEN;
> @@ -1082,6 +1087,15 @@ struct remote_domain_get_security_label_ret {
> int enforcing;
> };
>
> +struct remote_domain_get_security_label_list_args {
> + remote_nonnull_domain dom;
> +};
> +
> +struct remote_domain_get_security_label_list_ret {
> + remote_domain_get_security_label_ret labels<REMOTE_SECURITY_LABEL_LIST_MAX>;
> + int ret;
> +};
> +
> struct remote_node_get_security_model_ret {
> char model<REMOTE_SECURITY_MODEL_MAX>;
> char doi<REMOTE_SECURITY_DOI_MAX>;
> @@ -2844,8 +2858,8 @@ enum remote_procedure {
> REMOTE_PROC_CONNECT_LIST_ALL_DOMAINS = 273, /* skipgen skipgen priority:high */
> REMOTE_PROC_DOMAIN_LIST_ALL_SNAPSHOTS = 274, /* skipgen skipgen priority:high */
> REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_ALL_CHILDREN = 275, /* skipgen skipgen priority:high */
> - REMOTE_PROC_DOMAIN_EVENT_BALLOON_CHANGE = 276 /* autogen autogen */
> -
> + REMOTE_PROC_DOMAIN_EVENT_BALLOON_CHANGE = 276, /* autogen autogen */
> + REMOTE_PROC_DOMAIN_GET_SECURITY_LABEL_LIST = 277 /* skipgen skipgen priority:high */
> /*
> * Notice how the entries are grouped in sets of 10 ?
> * Nice isn't it. Please keep it this way when adding more.
> diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
> index 511284c..fc7ba73 100644
> --- a/src/remote_protocol-structs
> +++ b/src/remote_protocol-structs
> @@ -2251,4 +2251,5 @@ enum remote_procedure {
> REMOTE_PROC_DOMAIN_LIST_ALL_SNAPSHOTS = 274,
> REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_ALL_CHILDREN = 275,
> REMOTE_PROC_DOMAIN_EVENT_BALLOON_CHANGE = 276,
> + REMOTE_PROC_DOMAIN_GET_SECURITY_LABEL_list = 277,
s/list/LIST/
> };
>
More information about the libvir-list
mailing list