[libvirt] [PATCH 0/4] Changes to API/ABI for secrets handling
This series makes two changes to the public API and wire protocol ABI for the secrets handling support recently committed. As such this is a must fix before we do the release, since we can't break compatability once released. The first patch changes all the code to store UUIDs in raw format instead of printable format. This is important because it ensures that e3a9758f-b0c6-7a3a-ebb9-71a69c930289 is treated exactly the same as e3a9758fb0c67a3aebb971a69c930289, which is not the case with the currently committed code. It also makes the secrets and storage encryption XML parsers use the internal UUID handling APIs The second patch adds a new unique property to virSecretPtr. The "owner ID" is intended to be unique on a host wrt all secrets with matching usage type. This entails adding 2 APIs to querying the usage type, and another API to lookup a secret based on its usage. In testing the current code I frequently found myself getting into the situation where there were many secrets for the same volume which was not at all good. So if defining a secret to be used with a storage volume this ensures that you only ever have one secret associated with each storage volume path. The third patch ensures that when you query the XML for a qcow encrypted voluem, it will include the UUID of the secret. Previously if you created a volume, and then restarted libvirtd, the secret UUID would be lost. This patch is why we need the new API from the second patch virLookupSecretByUsage - without that it would be impossible to lookup the secret efficiently. THe final patch is a minor tweak to the RNG
Convert all the secret/storage encryption APIs / wire format to handle UUIDs in raw format instead of non-canonical printable format. Guarentees data format correctness. * docs/schemas/storageencryption.rng: Make UUID mandatory for a secret and validate fully * docs/schemas/secret.rng: Fully validate UUID * include/libvirt/libvirt.h, include/libvirt/libvirt.h.in, Add virSecretLookupByUUID and virSecretGetUUID. Make virSecretGetUUIDString follow normal API design pattern * python/generator.py: Skip generation of virSecretGetUUID, virSecretGetUUIDString and virSecretLookupByUUID * python/libvir.c, python/libvirt-python-api.xml: Manual impl of virSecretGetUUID,virSecretGetUUIDString and virSecretLookupByUUID * qemud/remote.c: s/virSecretLookupByUUIDString/virSecretLookupByUUID/ Fix get_nonnull_secret/make_nonnull_secret to use unsigned char * qemud/remote_protocol.x: Fix remote_nonnull_secret to use a remote_uuid instead of remote_nonnull_string for UUID field. Rename REMOTE_PROC_SECRET_LOOKUP_BY_UUID_STRING to REMOTE_PROC_SECRET_LOOKUP_BY_UUID_STRING and make it take an remote_uuid value * qemud/remote_dispatch_args.h, qemud/remote_dispatch_prototypes.h, qemud/remote_dispatch_ret.h, qemud/remote_dispatch_table.h, qemud/remote_protocol.c, qemud/remote_protocol.h: Re-generate * src/datatypes.h, src/datatypes.c: Store UUID in raw format instead of printable. Change virGetSecret to use raw format UUID * src/driver.h: Rename virDrvSecretLookupByUUIDString to virDrvSecretLookupByUUID and use raw format UUID * src/libvirt.c: Add virSecretLookupByUUID and virSecretGetUUID and re-implement virSecretLookupByUUIDString and virSecretGetUUIDString in terms of those * src/libvirt_public.syms: Add virSecretLookupByUUID and virSecretGetUUID * src/remote_internal.c: Rename remoteSecretLookupByUUIDString to remoteSecretLookupByUUID. Fix typo in args for remoteSecretDefineXML impl. Use raw UUID format for get_nonnull_secret and make_nonnull_secret * src/storage_encryption_conf.c, src/storage_encryption_conf.h: Storage UUID in raw format, and require it to be present in XML. Use UUID parser to validate. * secret_conf.h, secret_conf.c: Generate a UUID if none is provided. Storage UUID in raw format. * src/secret_driver.c: Adjust to deal with raw UUIDs. Save secrets in a filed with printable UUID, instead of base64 UUID. * src/virsh.c: Adjust for changed public API contract of virSecretGetUUIDString. * src/storage_Backend.c: DOn't undefine secret we just generated upon successful volume creation. Fix to handle raw UUIDs. Generate a non-clashing UUID * src/qemu_driver.c: Change to use lookupByUUID instead of lookupByUUIDString --- docs/schemas/secret.rng | 14 +++- docs/schemas/storageencryption.rng | 19 ++++- include/libvirt/libvirt.h | 7 ++- include/libvirt/libvirt.h.in | 7 ++- python/generator.py | 3 + python/libvir.c | 79 +++++++++++++++++++ python/libvirt-python-api.xml | 16 ++++ qemud/remote.c | 16 ++-- qemud/remote_dispatch_args.h | 2 +- qemud/remote_dispatch_prototypes.h | 6 +- qemud/remote_dispatch_ret.h | 2 +- qemud/remote_dispatch_table.h | 8 +- qemud/remote_protocol.c | 8 +- qemud/remote_protocol.h | 22 +++--- qemud/remote_protocol.x | 10 +- src/datatypes.c | 26 +++--- src/datatypes.h | 4 +- src/driver.h | 6 +- src/libvirt.c | 152 +++++++++++++++++++++++++++++------ src/libvirt_public.syms | 2 + src/qemu_driver.c | 11 +-- src/remote_internal.c | 26 +++--- src/secret_conf.c | 28 ++++++- src/secret_conf.h | 2 +- src/secret_driver.c | 129 +++++++++++-------------------- src/storage_backend.c | 56 +++++++++----- src/storage_encryption_conf.c | 29 +++++-- src/storage_encryption_conf.h | 3 +- src/virsh.c | 7 +- 29 files changed, 470 insertions(+), 230 deletions(-) diff --git a/docs/schemas/secret.rng b/docs/schemas/secret.rng index 8cfbd8f..2aab1db 100644 --- a/docs/schemas/secret.rng +++ b/docs/schemas/secret.rng @@ -25,7 +25,7 @@ <interleave> <optional> <element name='uuid'> - <text/> + <ref name='UUID'/> </element> </optional> <optional> @@ -53,4 +53,16 @@ <text/> </element> </define> + + <define name="UUID"> + <choice> + <data type="string"> + <param name="pattern">[a-fA-F0-9]{32}</param> + </data> + <data type="string"> + <param name="pattern">[a-fA-F0-9]{8}\-([a-fA-F0-9]{4}\-){3}[a-fA-F0-9]{12}</param> + </data> + </choice> + </define> + </grammar> diff --git a/docs/schemas/storageencryption.rng b/docs/schemas/storageencryption.rng index 8f9cd62..2739d79 100644 --- a/docs/schemas/storageencryption.rng +++ b/docs/schemas/storageencryption.rng @@ -23,12 +23,21 @@ <value>passphrase</value> </choice> </attribute> - <optional> - <attribute name='uuid'> - <text/> - </attribute> - </optional> + <attribute name='uuid'> + <ref name="UUID"/> + </attribute> </element> </define> + <define name="UUID"> + <choice> + <data type="string"> + <param name="pattern">[a-fA-F0-9]{32}</param> + </data> + <data type="string"> + <param name="pattern">[a-fA-F0-9]{8}\-([a-fA-F0-9]{4}\-){3}[a-fA-F0-9]{12}</param> + </data> + </choice> + </define> + </grammar> diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h index 1779b08..eadf420 100644 --- a/include/libvirt/libvirt.h +++ b/include/libvirt/libvirt.h @@ -1467,12 +1467,17 @@ int virConnectNumOfSecrets (virConnectPtr conn); int virConnectListSecrets (virConnectPtr conn, char **uuids, int maxuuids); +virSecretPtr virSecretLookupByUUID(virConnectPtr conn, + const unsigned char *uuid); virSecretPtr virSecretLookupByUUIDString(virConnectPtr conn, const char *uuid); virSecretPtr virSecretDefineXML (virConnectPtr conn, const char *xml, unsigned int flags); -char * virSecretGetUUIDString (virSecretPtr secret); +int virSecretGetUUID (virSecretPtr secret, + unsigned char *buf); +int virSecretGetUUIDString (virSecretPtr secret, + char *buf); char * virSecretGetXMLDesc (virSecretPtr secret, unsigned int flags); int virSecretSetValue (virSecretPtr secret, diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 8e26e48..1391af8 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1467,12 +1467,17 @@ int virConnectNumOfSecrets (virConnectPtr conn); int virConnectListSecrets (virConnectPtr conn, char **uuids, int maxuuids); +virSecretPtr virSecretLookupByUUID(virConnectPtr conn, + const unsigned char *uuid); virSecretPtr virSecretLookupByUUIDString(virConnectPtr conn, const char *uuid); virSecretPtr virSecretDefineXML (virConnectPtr conn, const char *xml, unsigned int flags); -char * virSecretGetUUIDString (virSecretPtr secret); +int virSecretGetUUID (virSecretPtr secret, + unsigned char *buf); +int virSecretGetUUIDString (virSecretPtr secret, + char *buf); char * virSecretGetXMLDesc (virSecretPtr secret, unsigned int flags); int virSecretSetValue (virSecretPtr secret, diff --git a/python/generator.py b/python/generator.py index 4dbad1c..c25ff55 100755 --- a/python/generator.py +++ b/python/generator.py @@ -328,6 +328,9 @@ skip_impl = ( 'virDomainPinVcpu', 'virSecretGetValue', 'virSecretSetValue', + 'virSecretGetUUID', + 'virSecretGetUUIDString', + 'virSecretLookupByUUID', 'virStoragePoolGetUUID', 'virStoragePoolGetUUIDString', 'virStoragePoolLookupByUUID', diff --git a/python/libvir.c b/python/libvir.c index 0c00b80..d4f1eb2 100644 --- a/python/libvir.c +++ b/python/libvir.c @@ -1563,6 +1563,82 @@ libvirt_virNodeDeviceListCaps(PyObject *self ATTRIBUTE_UNUSED, } static PyObject * +libvirt_virSecretGetUUID(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { + PyObject *py_retval; + unsigned char uuid[VIR_UUID_BUFLEN]; + virSecretPtr secret; + PyObject *pyobj_secret; + int c_retval; + + if (!PyArg_ParseTuple(args, (char *)"O:virSecretGetUUID", &pyobj_secret)) + return(NULL); + secret = (virSecretPtr) PyvirSecret_Get(pyobj_secret); + + if (secret == NULL) + return VIR_PY_NONE; + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virSecretGetUUID(secret, &uuid[0]); + LIBVIRT_END_ALLOW_THREADS; + + if (c_retval < 0) + return VIR_PY_NONE; + py_retval = PyString_FromStringAndSize((char *) &uuid[0], VIR_UUID_BUFLEN); + + return(py_retval); +} + +static PyObject * +libvirt_virSecretGetUUIDString(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) { + PyObject *py_retval; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virSecretPtr dom; + PyObject *pyobj_dom; + int c_retval; + + if (!PyArg_ParseTuple(args, (char *)"O:virSecretGetUUIDString", + &pyobj_dom)) + return(NULL); + dom = (virSecretPtr) PyvirSecret_Get(pyobj_dom); + + if (dom == NULL) + return VIR_PY_NONE; + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virSecretGetUUIDString(dom, &uuidstr[0]); + LIBVIRT_END_ALLOW_THREADS; + + if (c_retval < 0) + return VIR_PY_NONE; + + py_retval = PyString_FromString((char *) &uuidstr[0]); + return(py_retval); +} + +static PyObject * +libvirt_virSecretLookupByUUID(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { + PyObject *py_retval; + virSecretPtr c_retval; + virConnectPtr conn; + PyObject *pyobj_conn; + unsigned char * uuid; + int len; + + if (!PyArg_ParseTuple(args, (char *)"Oz#:virSecretLookupByUUID", &pyobj_conn, &uuid, &len)) + return(NULL); + conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn); + + if ((uuid == NULL) || (len != VIR_UUID_BUFLEN)) + return VIR_PY_NONE; + + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virSecretLookupByUUID(conn, uuid); + LIBVIRT_END_ALLOW_THREADS; + py_retval = libvirt_virSecretPtrWrap((virSecretPtr) c_retval); + return(py_retval); +} + + +static PyObject * libvirt_virConnectListSecrets(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { PyObject *py_retval; @@ -2358,6 +2434,9 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virEventInvokeTimeoutCallback", libvirt_virEventInvokeTimeoutCallback, METH_VARARGS, NULL}, {(char *) "virNodeListDevices", libvirt_virNodeListDevices, METH_VARARGS, NULL}, {(char *) "virNodeDeviceListCaps", libvirt_virNodeDeviceListCaps, METH_VARARGS, NULL}, + {(char *) "virSecretGetUUID", libvirt_virSecretGetUUID, METH_VARARGS, NULL}, + {(char *) "virSecretGetUUIDString", libvirt_virSecretGetUUIDString, METH_VARARGS, NULL}, + {(char *) "virSecretLookupByUUID", libvirt_virSecretLookupByUUID, METH_VARARGS, NULL}, {(char *) "virConnectListSecrets", libvirt_virConnectListSecrets, METH_VARARGS, NULL}, {(char *) "virSecretGetValue", libvirt_virSecretGetValue, METH_VARARGS, NULL}, {(char *) "virSecretSetValue", libvirt_virSecretSetValue, METH_VARARGS, NULL}, diff --git a/python/libvirt-python-api.xml b/python/libvirt-python-api.xml index e5c1fb9..148b89b 100644 --- a/python/libvirt-python-api.xml +++ b/python/libvirt-python-api.xml @@ -190,5 +190,21 @@ <arg name='value' type='const char *' info='The secret value'/> <arg name='flags' type='unsigned int' info='flags (unused; pass 0)'/> </function> + <function name='virSecretLookupByUUID' file='python'> + <info>Try to lookup a secret on the given hypervisor based on its UUID.</info> + <return type='virSecretPtr' info='a new secret object or NULL in case of failure'/> + <arg name='conn' type='virConnectPtr' info='pointer to the hypervisor connection'/> + <arg name='uuid' type='const unsigned char *' info='the UUID string for the secret, must be 16 bytes'/> + </function> + <function name='virSecretGetUUID' file='python'> + <info>Extract the UUID unique Identifier of a secret.</info> + <return type='char *' info='the 16 bytes string or None in case of error'/> + <arg name='secret' type='virSecretPtr' info='a secret object'/> + </function> + <function name='virSecretGetUUIDString' file='python'> + <info>Fetch globally unique ID of the secret as a string.</info> + <return type='char *' info='the UUID string or None in case of error'/> + <arg name='secret' type='virSecretPtr' info='a secret object'/> + </function> </symbols> </api> diff --git a/qemud/remote.c b/qemud/remote.c index 5d58611..a9fcc58 100644 --- a/qemud/remote.c +++ b/qemud/remote.c @@ -4710,15 +4710,15 @@ remoteDispatchSecretGetXmlDesc (struct qemud_server *server ATTRIBUTE_UNUSED, } static int -remoteDispatchSecretLookupByUuidString (struct qemud_server *server ATTRIBUTE_UNUSED, - struct qemud_client *client ATTRIBUTE_UNUSED, - virConnectPtr conn, remote_error *err, - remote_secret_lookup_by_uuid_string_args *args, - remote_secret_lookup_by_uuid_string_ret *ret) +remoteDispatchSecretLookupByUuid (struct qemud_server *server ATTRIBUTE_UNUSED, + struct qemud_client *client ATTRIBUTE_UNUSED, + virConnectPtr conn, remote_error *err, + remote_secret_lookup_by_uuid_args *args, + remote_secret_lookup_by_uuid_ret *ret) { virSecretPtr secret; - secret = virSecretLookupByUUIDString (conn, args->uuid); + secret = virSecretLookupByUUID (conn, (unsigned char *)args->uuid); if (secret == NULL) { remoteDispatchConnError (err, conn); return -1; @@ -4828,7 +4828,7 @@ get_nonnull_storage_vol (virConnectPtr conn, remote_nonnull_storage_vol vol) static virSecretPtr get_nonnull_secret (virConnectPtr conn, remote_nonnull_secret secret) { - return virGetSecret (conn, secret.uuid); + return virGetSecret (conn, BAD_CAST secret.uuid); } /* Make remote_nonnull_domain and remote_nonnull_network. */ @@ -4879,5 +4879,5 @@ make_nonnull_node_device (remote_nonnull_node_device *dev_dst, virNodeDevicePtr static void make_nonnull_secret (remote_nonnull_secret *secret_dst, virSecretPtr secret_src) { - secret_dst->uuid = strdup(secret_src->uuid); + memcpy (secret_dst->uuid, secret_src->uuid, VIR_UUID_BUFLEN); } diff --git a/qemud/remote_dispatch_args.h b/qemud/remote_dispatch_args.h index dcf7ddf..9f37963 100644 --- a/qemud/remote_dispatch_args.h +++ b/qemud/remote_dispatch_args.h @@ -118,7 +118,7 @@ remote_domain_xml_to_native_args val_remote_domain_xml_to_native_args; remote_list_defined_interfaces_args val_remote_list_defined_interfaces_args; remote_list_secrets_args val_remote_list_secrets_args; - remote_secret_lookup_by_uuid_string_args val_remote_secret_lookup_by_uuid_string_args; + remote_secret_lookup_by_uuid_args val_remote_secret_lookup_by_uuid_args; remote_secret_define_xml_args val_remote_secret_define_xml_args; remote_secret_get_xml_desc_args val_remote_secret_get_xml_desc_args; remote_secret_set_value_args val_remote_secret_set_value_args; diff --git a/qemud/remote_dispatch_prototypes.h b/qemud/remote_dispatch_prototypes.h index 647f5bb..7773cd9 100644 --- a/qemud/remote_dispatch_prototypes.h +++ b/qemud/remote_dispatch_prototypes.h @@ -807,13 +807,13 @@ static int remoteDispatchSecretGetXmlDesc( remote_error *err, remote_secret_get_xml_desc_args *args, remote_secret_get_xml_desc_ret *ret); -static int remoteDispatchSecretLookupByUuidString( +static int remoteDispatchSecretLookupByUuid( struct qemud_server *server, struct qemud_client *client, virConnectPtr conn, remote_error *err, - remote_secret_lookup_by_uuid_string_args *args, - remote_secret_lookup_by_uuid_string_ret *ret); + remote_secret_lookup_by_uuid_args *args, + remote_secret_lookup_by_uuid_ret *ret); static int remoteDispatchSecretSetValue( struct qemud_server *server, struct qemud_client *client, diff --git a/qemud/remote_dispatch_ret.h b/qemud/remote_dispatch_ret.h index 9d74a27..15d3386 100644 --- a/qemud/remote_dispatch_ret.h +++ b/qemud/remote_dispatch_ret.h @@ -101,7 +101,7 @@ remote_list_defined_interfaces_ret val_remote_list_defined_interfaces_ret; remote_num_of_secrets_ret val_remote_num_of_secrets_ret; remote_list_secrets_ret val_remote_list_secrets_ret; - remote_secret_lookup_by_uuid_string_ret val_remote_secret_lookup_by_uuid_string_ret; + remote_secret_lookup_by_uuid_ret val_remote_secret_lookup_by_uuid_ret; remote_secret_define_xml_ret val_remote_secret_define_xml_ret; remote_secret_get_xml_desc_ret val_remote_secret_get_xml_desc_ret; remote_secret_get_value_ret val_remote_secret_get_value_ret; diff --git a/qemud/remote_dispatch_table.h b/qemud/remote_dispatch_table.h index 02d7bb5..07e36bb 100644 --- a/qemud/remote_dispatch_table.h +++ b/qemud/remote_dispatch_table.h @@ -707,10 +707,10 @@ .args_filter = (xdrproc_t) xdr_remote_list_secrets_args, .ret_filter = (xdrproc_t) xdr_remote_list_secrets_ret, }, -{ /* SecretLookupByUuidString => 141 */ - .fn = (dispatch_fn) remoteDispatchSecretLookupByUuidString, - .args_filter = (xdrproc_t) xdr_remote_secret_lookup_by_uuid_string_args, - .ret_filter = (xdrproc_t) xdr_remote_secret_lookup_by_uuid_string_ret, +{ /* SecretLookupByUuid => 141 */ + .fn = (dispatch_fn) remoteDispatchSecretLookupByUuid, + .args_filter = (xdrproc_t) xdr_remote_secret_lookup_by_uuid_args, + .ret_filter = (xdrproc_t) xdr_remote_secret_lookup_by_uuid_ret, }, { /* SecretDefineXml => 142 */ .fn = (dispatch_fn) remoteDispatchSecretDefineXml, diff --git a/qemud/remote_protocol.c b/qemud/remote_protocol.c index db4d794..b6666a1 100644 --- a/qemud/remote_protocol.c +++ b/qemud/remote_protocol.c @@ -107,7 +107,7 @@ bool_t xdr_remote_nonnull_secret (XDR *xdrs, remote_nonnull_secret *objp) { - if (!xdr_remote_nonnull_string (xdrs, &objp->uuid)) + if (!xdr_remote_uuid (xdrs, objp->uuid)) return FALSE; return TRUE; } @@ -2572,16 +2572,16 @@ xdr_remote_list_secrets_ret (XDR *xdrs, remote_list_secrets_ret *objp) } bool_t -xdr_remote_secret_lookup_by_uuid_string_args (XDR *xdrs, remote_secret_lookup_by_uuid_string_args *objp) +xdr_remote_secret_lookup_by_uuid_args (XDR *xdrs, remote_secret_lookup_by_uuid_args *objp) { - if (!xdr_remote_nonnull_string (xdrs, &objp->uuid)) + if (!xdr_remote_uuid (xdrs, objp->uuid)) return FALSE; return TRUE; } bool_t -xdr_remote_secret_lookup_by_uuid_string_ret (XDR *xdrs, remote_secret_lookup_by_uuid_string_ret *objp) +xdr_remote_secret_lookup_by_uuid_ret (XDR *xdrs, remote_secret_lookup_by_uuid_ret *objp) { if (!xdr_remote_nonnull_secret (xdrs, &objp->secret)) diff --git a/qemud/remote_protocol.h b/qemud/remote_protocol.h index b54b3ae..4b73ee1 100644 --- a/qemud/remote_protocol.h +++ b/qemud/remote_protocol.h @@ -86,7 +86,7 @@ struct remote_nonnull_node_device { typedef struct remote_nonnull_node_device remote_nonnull_node_device; struct remote_nonnull_secret { - remote_nonnull_string uuid; + remote_uuid uuid; }; typedef struct remote_nonnull_secret remote_nonnull_secret; @@ -1453,15 +1453,15 @@ struct remote_list_secrets_ret { }; typedef struct remote_list_secrets_ret remote_list_secrets_ret; -struct remote_secret_lookup_by_uuid_string_args { - remote_nonnull_string uuid; +struct remote_secret_lookup_by_uuid_args { + remote_uuid uuid; }; -typedef struct remote_secret_lookup_by_uuid_string_args remote_secret_lookup_by_uuid_string_args; +typedef struct remote_secret_lookup_by_uuid_args remote_secret_lookup_by_uuid_args; -struct remote_secret_lookup_by_uuid_string_ret { +struct remote_secret_lookup_by_uuid_ret { remote_nonnull_secret secret; }; -typedef struct remote_secret_lookup_by_uuid_string_ret remote_secret_lookup_by_uuid_string_ret; +typedef struct remote_secret_lookup_by_uuid_ret remote_secret_lookup_by_uuid_ret; struct remote_secret_define_xml_args { remote_nonnull_string xml; @@ -1657,7 +1657,7 @@ enum remote_procedure { REMOTE_PROC_LIST_DEFINED_INTERFACES = 138, REMOTE_PROC_NUM_OF_SECRETS = 139, REMOTE_PROC_LIST_SECRETS = 140, - REMOTE_PROC_SECRET_LOOKUP_BY_UUID_STRING = 141, + REMOTE_PROC_SECRET_LOOKUP_BY_UUID = 141, REMOTE_PROC_SECRET_DEFINE_XML = 142, REMOTE_PROC_SECRET_GET_XML_DESC = 143, REMOTE_PROC_SECRET_SET_VALUE = 144, @@ -1929,8 +1929,8 @@ extern bool_t xdr_remote_domain_xml_to_native_ret (XDR *, remote_domain_xml_to_ extern bool_t xdr_remote_num_of_secrets_ret (XDR *, remote_num_of_secrets_ret*); extern bool_t xdr_remote_list_secrets_args (XDR *, remote_list_secrets_args*); extern bool_t xdr_remote_list_secrets_ret (XDR *, remote_list_secrets_ret*); -extern bool_t xdr_remote_secret_lookup_by_uuid_string_args (XDR *, remote_secret_lookup_by_uuid_string_args*); -extern bool_t xdr_remote_secret_lookup_by_uuid_string_ret (XDR *, remote_secret_lookup_by_uuid_string_ret*); +extern bool_t xdr_remote_secret_lookup_by_uuid_args (XDR *, remote_secret_lookup_by_uuid_args*); +extern bool_t xdr_remote_secret_lookup_by_uuid_ret (XDR *, remote_secret_lookup_by_uuid_ret*); extern bool_t xdr_remote_secret_define_xml_args (XDR *, remote_secret_define_xml_args*); extern bool_t xdr_remote_secret_define_xml_ret (XDR *, remote_secret_define_xml_ret*); extern bool_t xdr_remote_secret_get_xml_desc_args (XDR *, remote_secret_get_xml_desc_args*); @@ -2181,8 +2181,8 @@ extern bool_t xdr_remote_domain_xml_to_native_ret (); extern bool_t xdr_remote_num_of_secrets_ret (); extern bool_t xdr_remote_list_secrets_args (); extern bool_t xdr_remote_list_secrets_ret (); -extern bool_t xdr_remote_secret_lookup_by_uuid_string_args (); -extern bool_t xdr_remote_secret_lookup_by_uuid_string_ret (); +extern bool_t xdr_remote_secret_lookup_by_uuid_args (); +extern bool_t xdr_remote_secret_lookup_by_uuid_ret (); extern bool_t xdr_remote_secret_define_xml_args (); extern bool_t xdr_remote_secret_define_xml_ret (); extern bool_t xdr_remote_secret_get_xml_desc_args (); diff --git a/qemud/remote_protocol.x b/qemud/remote_protocol.x index 006dfa1..5712d98 100644 --- a/qemud/remote_protocol.x +++ b/qemud/remote_protocol.x @@ -188,7 +188,7 @@ struct remote_nonnull_node_device { /* A secret which may not be null. */ struct remote_nonnull_secret { - remote_nonnull_string uuid; + remote_uuid uuid; }; /* A domain or network which may be NULL. */ @@ -1293,11 +1293,11 @@ struct remote_list_secrets_ret { remote_nonnull_string uuids<REMOTE_SECRET_UUID_LIST_MAX>; }; -struct remote_secret_lookup_by_uuid_string_args { - remote_nonnull_string uuid; +struct remote_secret_lookup_by_uuid_args { + remote_uuid uuid; }; -struct remote_secret_lookup_by_uuid_string_ret { +struct remote_secret_lookup_by_uuid_ret { remote_nonnull_secret secret; }; @@ -1500,7 +1500,7 @@ enum remote_procedure { REMOTE_PROC_NUM_OF_SECRETS = 139, REMOTE_PROC_LIST_SECRETS = 140, - REMOTE_PROC_SECRET_LOOKUP_BY_UUID_STRING = 141, + REMOTE_PROC_SECRET_LOOKUP_BY_UUID = 141, REMOTE_PROC_SECRET_DEFINE_XML = 142, REMOTE_PROC_SECRET_GET_XML_DESC = 143, REMOTE_PROC_SECRET_SET_VALUE = 144, diff --git a/src/datatypes.c b/src/datatypes.c index 2e85196..b0067f6 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -25,6 +25,7 @@ #include "virterror_internal.h" #include "logging.h" #include "memory.h" +#include "uuid.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -1169,9 +1170,10 @@ virUnrefNodeDevice(virNodeDevicePtr dev) { * Returns a pointer to the secret, or NULL in case of failure */ virSecretPtr -virGetSecret(virConnectPtr conn, const char *uuid) +virGetSecret(virConnectPtr conn, const unsigned char *uuid) { virSecretPtr ret = NULL; + char uuidstr[VIR_UUID_STRING_BUFLEN]; if (!VIR_IS_CONNECT(conn) || uuid == NULL) { virLibConnError(NULL, VIR_ERR_INVALID_ARG, __FUNCTION__); @@ -1179,7 +1181,9 @@ virGetSecret(virConnectPtr conn, const char *uuid) } virMutexLock(&conn->lock); - ret = virHashLookup(conn->secrets, uuid); + virUUIDFormat(uuid, uuidstr); + + ret = virHashLookup(conn->secrets, uuidstr); if (ret == NULL) { if (VIR_ALLOC(ret) < 0) { virMutexUnlock(&conn->lock); @@ -1188,14 +1192,9 @@ virGetSecret(virConnectPtr conn, const char *uuid) } ret->magic = VIR_SECRET_MAGIC; ret->conn = conn; - ret->uuid = strdup(uuid); - if (ret->uuid == NULL) { - virMutexUnlock(&conn->lock); - virReportOOMError(conn); - goto error; - } + memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN); - if (virHashAddEntry(conn->secrets, uuid, ret) < 0) { + if (virHashAddEntry(conn->secrets, uuidstr, ret) < 0) { virMutexUnlock(&conn->lock); virLibConnError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("failed to add secret to conn hash table")); @@ -1229,9 +1228,11 @@ error: static void virReleaseSecret(virSecretPtr secret) { virConnectPtr conn = secret->conn; - DEBUG("release secret %p %s", secret, secret->uuid); + char uuidstr[VIR_UUID_STRING_BUFLEN]; + DEBUG("release secret %p %p", secret, secret->uuid); - if (virHashRemoveEntry(conn->secrets, secret->uuid, NULL) < 0) { + virUUIDFormat(secret->uuid, uuidstr); + if (virHashRemoveEntry(conn->secrets, uuidstr, NULL) < 0) { virMutexUnlock(&conn->lock); virLibConnError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("secret missing from connection hash table")); @@ -1239,7 +1240,6 @@ virReleaseSecret(virSecretPtr secret) { } secret->magic = -1; - VIR_FREE(secret->uuid); VIR_FREE(secret); if (conn) { @@ -1272,7 +1272,7 @@ virUnrefSecret(virSecretPtr secret) { return -1; } virMutexLock(&secret->conn->lock); - DEBUG("unref secret %p %s %d", secret, secret->uuid, secret->refs); + DEBUG("unref secret %p %p %d", secret, secret->uuid, secret->refs); secret->refs--; refs = secret->refs; if (refs == 0) { diff --git a/src/datatypes.h b/src/datatypes.h index aa60b63..5319308 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -255,7 +255,7 @@ struct _virSecret { unsigned int magic; /* specific value to check */ int refs; /* reference count */ virConnectPtr conn; /* pointer back to the connection */ - char *uuid; /* ID of the secret */ + unsigned char uuid[VIR_UUID_BUFLEN]; /* the domain unique identifier */ }; @@ -296,7 +296,7 @@ virNodeDevicePtr virGetNodeDevice(virConnectPtr conn, int virUnrefNodeDevice(virNodeDevicePtr dev); virSecretPtr virGetSecret(virConnectPtr conn, - const char *uuid); + const unsigned char *uuid); int virUnrefSecret(virSecretPtr secret); #endif diff --git a/src/driver.h b/src/driver.h index 447b7a2..9f197d9 100644 --- a/src/driver.h +++ b/src/driver.h @@ -819,8 +819,8 @@ verify((VIR_SECRET_GET_VALUE_INTERNAL_CALL & VIR_SECRET_GET_VALUE_FLAGS_MASK) == 0); typedef virSecretPtr - (*virDrvSecretLookupByUUIDString) (virConnectPtr conn, - const char *uuid); + (*virDrvSecretLookupByUUID) (virConnectPtr conn, + const unsigned char *uuid); typedef virSecretPtr (*virDrvSecretDefineXML) (virConnectPtr conn, const char *xml, @@ -866,7 +866,7 @@ struct _virSecretDriver { virDrvSecretNumOfSecrets numOfSecrets; virDrvSecretListSecrets listSecrets; - virDrvSecretLookupByUUIDString lookupByUUIDString; + virDrvSecretLookupByUUID lookupByUUID; virDrvSecretDefineXML defineXML; virDrvSecretGetXMLDesc getXMLDesc; virDrvSecretSetValue setValue; diff --git a/src/libvirt.c b/src/libvirt.c index 96d204c..7a915fa 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -8811,36 +8811,36 @@ error: } /** - * virSecretLookupByUUIDString: - * @conn: virConnect connection - * @uuid: ID of a secret + * virSecretLookupByUUID: + * @conn: pointer to the hypervisor connection + * @uuid: the raw UUID for the secret * - * Fetches a secret based on uuid. + * Try to lookup a secret on the given hypervisor based on its UUID. * - * Returns the secret on success, or NULL on failure. + * Returns a new secret object or NULL in case of failure. If the + * secret cannot be found, then VIR_ERR_NO_SECRET error is raised. */ virSecretPtr -virSecretLookupByUUIDString(virConnectPtr conn, const char *uuid) +virSecretLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { - VIR_DEBUG("conn=%p, uuid=%s", conn, uuid); + DEBUG("conn=%p, uuid=%s", conn, uuid); virResetLastError(); if (!VIR_IS_CONNECT(conn)) { virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__); - return NULL; + return (NULL); } if (uuid == NULL) { virLibConnError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__); goto error; } - if (conn->secretDriver != NULL && - conn->secretDriver->lookupByUUIDString != NULL) { + if (conn->secretDriver && + conn->secretDriver->lookupByUUID) { virSecretPtr ret; - - ret = conn->secretDriver->lookupByUUIDString(conn, uuid); - if (ret == NULL) + ret = conn->secretDriver->lookupByUUID (conn, uuid); + if (!ret) goto error; return ret; } @@ -8854,6 +8854,65 @@ error: } /** + * virSecretLookupByUUIDString: + * @conn: pointer to the hypervisor connection + * @uuidstr: the string UUID for the secret + * + * Try to lookup a secret on the given hypervisor based on its UUID. + * + * Returns a new secret object or NULL in case of failure. If the + * secret cannot be found, then VIR_ERR_NO_SECRET error is raised. + */ +virSecretPtr +virSecretLookupByUUIDString(virConnectPtr conn, const char *uuidstr) +{ + int raw[VIR_UUID_BUFLEN], i; + unsigned char uuid[VIR_UUID_BUFLEN]; + int ret; + + DEBUG("conn=%p, uuidstr=%s", conn, uuidstr); + + virResetLastError(); + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__); + return (NULL); + } + if (uuidstr == NULL) { + virLibConnError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + /* XXX: sexpr_uuid() also supports 'xxxx-xxxx-xxxx-xxxx' format. + * We needn't it here. Right? + */ + ret = sscanf(uuidstr, + "%02x%02x%02x%02x-" + "%02x%02x-" + "%02x%02x-" + "%02x%02x-" + "%02x%02x%02x%02x%02x%02x", + raw + 0, raw + 1, raw + 2, raw + 3, + raw + 4, raw + 5, raw + 6, raw + 7, + raw + 8, raw + 9, raw + 10, raw + 11, + raw + 12, raw + 13, raw + 14, raw + 15); + + if (ret!=VIR_UUID_BUFLEN) { + virLibConnError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + for (i = 0; i < VIR_UUID_BUFLEN; i++) + uuid[i] = raw[i] & 0xFF; + + return virSecretLookupByUUID(conn, &uuid[0]); + +error: + /* Copy to connection error object for back compatability */ + virSetConnError(conn); + return NULL; +} + + +/** * virSecretDefineXML: * @conn: virConnect connection * @xml: XML describing the secret. @@ -8906,37 +8965,78 @@ error: } /** - * virSecretGetUUIDString: + * virSecretGetUUID: * @secret: A virSecret secret + * @uuid: buffer of VIR_UUID_BUFLEN bytes in size * * Fetches the UUID of the secret. * - * Returns ID of the secret (not necessarily in the UUID format) on success, - * NULL on failure. The caller must free() the ID. + * Returns 0 on success with the uuid buffer being filled, or + * -1 upon failure. */ -char * -virSecretGetUUIDString(virSecretPtr secret) +int +virSecretGetUUID(virSecretPtr secret, unsigned char *uuid) { - char *ret; - VIR_DEBUG("secret=%p", secret); virResetLastError(); if (!VIR_IS_CONNECTED_SECRET(secret)) { virLibSecretError(NULL, VIR_ERR_INVALID_SECRET, __FUNCTION__); - return NULL; + return -1; + } + if (uuid == NULL) { + virLibSecretError(secret, VIR_ERR_INVALID_ARG, __FUNCTION__); + /* Copy to connection error object for back compatability */ + virSetConnError(secret->conn); + return -1; } - ret = strdup(secret->uuid); - if (ret != NULL) - return ret; + memcpy(uuid, &secret->uuid[0], VIR_UUID_BUFLEN); + + return 0; +} - virReportOOMError(secret->conn); +/** + * virSecretGetUUIDString: + * @secret: a secret object + * @buf: pointer to a VIR_UUID_STRING_BUFLEN bytes array + * + * Get the UUID for a secret as string. For more information about + * UUID see RFC4122. + * + * Returns -1 in case of error, 0 in case of success + */ +int +virSecretGetUUIDString(virSecretPtr secret, char *buf) +{ + unsigned char uuid[VIR_UUID_BUFLEN]; + DEBUG("secret=%p, buf=%p", secret, buf); + + virResetLastError(); + + if (!VIR_IS_SECRET(secret)) { + virLibSecretError(NULL, VIR_ERR_INVALID_SECRET, __FUNCTION__); + return (-1); + } + if (buf == NULL) { + virLibSecretError(secret, VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + if (virSecretGetUUID(secret, &uuid[0])) + goto error; + + virUUIDFormat(uuid, buf); + return (0); + +error: + /* Copy to connection error object for back compatability */ virSetConnError(secret->conn); - return NULL; + return -1; } + /** * virSecretGetXMLDesc: * @secret: A virSecret secret diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 65080ed..c34bbae 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -298,8 +298,10 @@ LIBVIRT_0.7.1 { virSecretGetConnect; virConnectNumOfSecrets; virConnectListSecrets; + virSecretLookupByUUID; virSecretLookupByUUIDString; virSecretDefineXML; + virSecretGetUUID; virSecretGetUUIDString; virSecretGetXMLDesc; virSecretSetValue; diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 5a8a686..0e7d8d4 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -2695,7 +2695,7 @@ findVolumeQcowPassphrase(virConnectPtr conn, virDomainObjPtr vm, size_t size; if (conn->secretDriver == NULL || - conn->secretDriver->lookupByUUIDString == NULL || + conn->secretDriver->lookupByUUID == NULL || conn->secretDriver->getValue == NULL) { qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, "%s", _("secret storage not supported")); @@ -2715,13 +2715,8 @@ findVolumeQcowPassphrase(virConnectPtr conn, virDomainObjPtr vm, return NULL; } - if (enc->secrets[0]->uuid == NULL) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INVALID_DOMAIN, - _("missing secret uuid for volume %s"), path); - return NULL; - } - secret = conn->secretDriver->lookupByUUIDString(conn, - enc->secrets[0]->uuid); + secret = conn->secretDriver->lookupByUUID(conn, + enc->secrets[0]->uuid); if (secret == NULL) return NULL; data = conn->secretDriver->getValue(secret, &size, diff --git a/src/remote_internal.c b/src/remote_internal.c index 3dd4609..e7f0186 100644 --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -6475,25 +6475,25 @@ done: } static virSecretPtr -remoteSecretLookupByUUIDString (virConnectPtr conn, const char *uuid) +remoteSecretLookupByUUID (virConnectPtr conn, const unsigned char *uuid) { virSecretPtr rv = NULL; - remote_secret_lookup_by_uuid_string_args args; - remote_secret_lookup_by_uuid_string_ret ret; + remote_secret_lookup_by_uuid_args args; + remote_secret_lookup_by_uuid_ret ret; struct private_data *priv = conn->secretPrivateData; remoteDriverLock (priv); - args.uuid = (char *) uuid; + memcpy (args.uuid, uuid, VIR_UUID_BUFLEN); memset (&ret, 0, sizeof (ret)); - if (call (conn, priv, 0, REMOTE_PROC_SECRET_LOOKUP_BY_UUID_STRING, - (xdrproc_t) xdr_remote_secret_lookup_by_uuid_string_args, (char *) &args, - (xdrproc_t) xdr_remote_secret_lookup_by_uuid_string_ret, (char *) &ret) == -1) + if (call (conn, priv, 0, REMOTE_PROC_SECRET_LOOKUP_BY_UUID, + (xdrproc_t) xdr_remote_secret_lookup_by_uuid_args, (char *) &args, + (xdrproc_t) xdr_remote_secret_lookup_by_uuid_ret, (char *) &ret) == -1) goto done; rv = get_nonnull_secret (conn, ret.secret); - xdr_free ((xdrproc_t) xdr_remote_secret_lookup_by_uuid_string_ret, + xdr_free ((xdrproc_t) xdr_remote_secret_lookup_by_uuid_ret, (char *) &ret); done: @@ -6506,7 +6506,7 @@ remoteSecretDefineXML (virConnectPtr conn, const char *xml, unsigned int flags) { virSecretPtr rv = NULL; remote_secret_define_xml_args args; - remote_secret_lookup_by_uuid_string_ret ret; + remote_secret_define_xml_ret ret; struct private_data *priv = conn->secretPrivateData; remoteDriverLock (priv); @@ -6521,7 +6521,7 @@ remoteSecretDefineXML (virConnectPtr conn, const char *xml, unsigned int flags) goto done; rv = get_nonnull_secret (conn, ret.secret); - xdr_free ((xdrproc_t) xdr_remote_secret_lookup_by_uuid_string_ret, + xdr_free ((xdrproc_t) xdr_remote_secret_define_xml_ret, (char *) &ret); done: @@ -7733,7 +7733,7 @@ get_nonnull_node_device (virConnectPtr conn, remote_nonnull_node_device dev) static virSecretPtr get_nonnull_secret (virConnectPtr conn, remote_nonnull_secret secret) { - return virGetSecret(conn, secret.uuid); + return virGetSecret(conn, BAD_CAST secret.uuid); } /* Make remote_nonnull_domain and remote_nonnull_network. */ @@ -7778,7 +7778,7 @@ make_nonnull_storage_vol (remote_nonnull_storage_vol *vol_dst, virStorageVolPtr static void make_nonnull_secret (remote_nonnull_secret *secret_dst, virSecretPtr secret_src) { - secret_dst->uuid = secret_src->uuid; + memcpy (secret_dst->uuid, secret_src->uuid, VIR_UUID_BUFLEN); } /*----------------------------------------------------------------------*/ @@ -7940,7 +7940,7 @@ static virSecretDriver secret_driver = { .close = remoteSecretClose, .numOfSecrets = remoteSecretNumOfSecrets, .listSecrets = remoteSecretListSecrets, - .lookupByUUIDString = remoteSecretLookupByUUIDString, + .lookupByUUID = remoteSecretLookupByUUID, .defineXML = remoteSecretDefineXML, .getXMLDesc = remoteSecretGetXMLDesc, .setValue = remoteSecretSetValue, diff --git a/src/secret_conf.c b/src/secret_conf.c index c7b11a9..51ac13d 100644 --- a/src/secret_conf.c +++ b/src/secret_conf.c @@ -31,6 +31,7 @@ #include "virterror_internal.h" #include "util.h" #include "xml.h" +#include "uuid.h" #define VIR_FROM_THIS VIR_FROM_SECRET @@ -42,7 +43,6 @@ virSecretDefFree(virSecretDefPtr def) if (def == NULL) return; - VIR_FREE(def->id); VIR_FREE(def->description); switch (def->usage_type) { case VIR_SECRET_USAGE_TYPE_NONE: @@ -105,6 +105,7 @@ secretXMLParseNode(virConnectPtr conn, xmlDocPtr xml, xmlNodePtr root) xmlXPathContextPtr ctxt = NULL; virSecretDefPtr def = NULL, ret = NULL; char *prop = NULL; + char *uuidstr = NULL; if (!xmlStrEqual(root->name, BAD_CAST "secret")) { virSecretReportError(conn, VIR_ERR_XML_ERROR, "%s", @@ -152,7 +153,22 @@ secretXMLParseNode(virConnectPtr conn, xmlDocPtr xml, xmlNodePtr root) VIR_FREE(prop); } - def->id = virXPathString(conn, "string(./uuid)", ctxt); + uuidstr = virXPathString(conn, "string(./uuid)", ctxt); + if (!uuidstr) { + if (virUUIDGenerate(def->uuid)) { + virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("Failed to generate UUID")); + goto cleanup; + } + } else { + if (virUUIDParse(uuidstr, def->uuid) < 0) { + virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("malformed uuid element")); + goto cleanup; + } + VIR_FREE(uuidstr); + } + def->description = virXPathString(conn, "string(./description)", ctxt); if (virXPathNode(conn, "./usage", ctxt) != NULL && virSecretDefParseUsage(conn, ctxt, def) < 0) @@ -280,13 +296,17 @@ char * virSecretDefFormat(virConnectPtr conn, const virSecretDefPtr def) { virBuffer buf = VIR_BUFFER_INITIALIZER; + unsigned char *uuid; + char uuidstr[VIR_UUID_STRING_BUFLEN]; char *tmp; virBufferVSprintf(&buf, "<secret ephemeral='%s' private='%s'>\n", def->ephemeral ? "yes" : "no", def->private ? "yes" : "no"); - if (def->id != NULL) - virBufferEscapeString(&buf, " <uuid>%s</uuid>\n", def->id); + + uuid = def->uuid; + virUUIDFormat(uuid, uuidstr); + virBufferEscapeString(&buf, " <uuid>%s</uuid>\n", uuidstr); if (def->description != NULL) virBufferEscapeString(&buf, " <description>%s</description>\n", def->description); diff --git a/src/secret_conf.h b/src/secret_conf.h index 95beedf..556f5a4 100644 --- a/src/secret_conf.h +++ b/src/secret_conf.h @@ -43,7 +43,7 @@ typedef virSecretDef *virSecretDefPtr; struct _virSecretDef { unsigned ephemeral : 1; unsigned private : 1; - char *id; /* May be NULL */ + unsigned char uuid[VIR_UUID_BUFLEN]; char *description; /* May be NULL */ int usage_type; union { diff --git a/src/secret_driver.c b/src/secret_driver.c index 40c3ede..c1e0c67 100644 --- a/src/secret_driver.c +++ b/src/secret_driver.c @@ -111,13 +111,13 @@ secretFree(virSecretEntryPtr secret) } static virSecretEntryPtr * -secretFind(virSecretDriverStatePtr driver, const char *uuid) +secretFind(virSecretDriverStatePtr driver, const unsigned char *uuid) { virSecretEntryPtr *pptr, s; for (pptr = &driver->secrets; *pptr != NULL; pptr = &s->next) { s = *pptr; - if (STREQ(s->def->id, uuid)) + if (memcmp(s->def->uuid, uuid, VIR_UUID_BUFLEN) == 0) return pptr; } return NULL; @@ -125,15 +125,13 @@ secretFind(virSecretDriverStatePtr driver, const char *uuid) static virSecretEntryPtr secretCreate(virConnectPtr conn, virSecretDriverStatePtr driver, - const char *uuid) + const unsigned char *uuid) { virSecretEntryPtr secret = NULL; if (VIR_ALLOC(secret) < 0 || VIR_ALLOC(secret->def)) goto no_memory; - secret->def->id = strdup(uuid); - if (secret->def->id == NULL) - goto no_memory; + memcpy(secret->def->uuid, uuid, VIR_UUID_BUFLEN); listInsert(&driver->secrets, secret); return secret; @@ -145,7 +143,7 @@ secretCreate(virConnectPtr conn, virSecretDriverStatePtr driver, static virSecretEntryPtr secretFindOrCreate(virConnectPtr conn, virSecretDriverStatePtr driver, - const char *uuid, bool *created_new) + const unsigned char *uuid, bool *created_new) { virSecretEntryPtr *pptr, secret; @@ -223,18 +221,15 @@ static char * secretComputePath(virConnectPtr conn, virSecretDriverStatePtr driver, const virSecretEntry *secret, const char *suffix) { - char *ret, *base64_id; + char *ret; + char uuidstr[VIR_UUID_STRING_BUFLEN]; - base64_encode_alloc(secret->def->id, strlen(secret->def->id), &base64_id); - if (base64_id == NULL) { - virReportOOMError(conn); - return NULL; - } + virUUIDFormat(secret->def->uuid, uuidstr); - if (virAsprintf(&ret, "%s/%s%s", driver->directory, base64_id, suffix) < 0) + if (virAsprintf(&ret, "%s/%s%s", driver->directory, uuidstr, suffix) < 0) /* ret is NULL */ virReportOOMError(conn); - VIR_FREE(base64_id); + return ret; } @@ -357,28 +352,16 @@ static int secretLoadValidateUUID(virConnectPtr conn, virSecretDefPtr def, const char *xml_basename) { - char *base64_id; + char uuidstr[VIR_UUID_STRING_BUFLEN]; - if (def->id == NULL) { - virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("<uuid> missing in secret description in '%s'"), - xml_basename); - return -1; - } + virUUIDFormat(def->uuid, uuidstr); - base64_encode_alloc(def->id, strlen(def->id), &base64_id); - if (base64_id == NULL) { - virReportOOMError(conn); - return -1; - } - if (!virFileMatchesNameSuffix(xml_basename, base64_id, ".xml")) { + if (!virFileMatchesNameSuffix(xml_basename, uuidstr, ".xml")) { virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, _("<uuid> does not match secret file name '%s'"), xml_basename); - VIR_FREE(base64_id); return -1; } - VIR_FREE(base64_id); return 0; } @@ -604,11 +587,13 @@ secretListSecrets(virConnectPtr conn, char **uuids, int maxuuids) i = 0; for (secret = driver->secrets; secret != NULL; secret = secret->next) { + char *uuidstr; if (i == maxuuids) break; - uuids[i] = strdup(secret->def->id); - if (uuids[i] == NULL) + if (VIR_ALLOC_N(uuidstr, VIR_UUID_STRING_BUFLEN) < 0) goto cleanup; + virUUIDFormat(secret->def->uuid, uuidstr); + uuids[i] = uuidstr; i++; } @@ -625,7 +610,7 @@ cleanup: } static virSecretPtr -secretLookupByUUIDString(virConnectPtr conn, const char *uuid) +secretLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { virSecretDriverStatePtr driver = conn->secretPrivateData; virSecretPtr ret = NULL; @@ -635,49 +620,25 @@ secretLookupByUUIDString(virConnectPtr conn, const char *uuid) pptr = secretFind(driver, uuid); if (pptr == NULL) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(uuid, uuidstr); virSecretReportError(conn, VIR_ERR_NO_SECRET, - _("no secret with matching id '%s'"), uuid); + _("no secret with matching uuid '%s'"), uuidstr); goto cleanup; } - ret = virGetSecret(conn, (*pptr)->def->id); + ret = virGetSecret(conn, (*pptr)->def->uuid); cleanup: + if (1) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(uuid, uuidstr); + VIR_ERROR("Lookup %s got %p", uuidstr, ret); + } secretDriverUnlock(driver); return ret; } -static char * -secretGenerateUUID(virConnectPtr conn, virSecretDriverStatePtr driver) -{ - char *uuid = NULL; - unsigned attempt; - - if (VIR_ALLOC_N(uuid, VIR_UUID_STRING_BUFLEN) < 0) { - virReportOOMError(conn); - goto error; - } - - for (attempt = 0; attempt < 65536; attempt++) { - unsigned char uuid_data[VIR_UUID_BUFLEN]; - - if (virUUIDGenerate(uuid_data) < 0) { - virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", - _("unable to generate uuid")); - goto error; - } - virUUIDFormat(uuid_data, uuid); - if (secretFind(driver, uuid) == NULL) - return uuid; - } - virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", - _("too many conflicts when generating an uuid")); - goto error; - -error: - VIR_FREE(uuid); - return NULL; -} static virSecretPtr secretDefineXML(virConnectPtr conn, const char *xml, @@ -695,16 +656,8 @@ secretDefineXML(virConnectPtr conn, const char *xml, secretDriverLock(driver); - if (new_attrs->id != NULL) - secret = secretFindOrCreate(conn, driver, new_attrs->id, - &secret_is_new); - else { - new_attrs->id = secretGenerateUUID(conn, driver); - if (new_attrs->id == NULL) - goto cleanup; - secret = secretCreate(conn, driver, new_attrs->id); - secret_is_new = true; - } + secret = secretFindOrCreate(conn, driver, new_attrs->uuid, + &secret_is_new); if (secret == NULL) goto cleanup; @@ -743,7 +696,7 @@ secretDefineXML(virConnectPtr conn, const char *xml, new_attrs = NULL; virSecretDefFree(backup); - ret = virGetSecret(conn, secret->def->id); + ret = virGetSecret(conn, secret->def->uuid); goto cleanup; restore_backup: @@ -776,8 +729,10 @@ secretGetXMLDesc(virSecretPtr obj, unsigned int flags ATTRIBUTE_UNUSED) pptr = secretFind(driver, obj->uuid); if (pptr == NULL) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(obj->uuid, uuidstr); virSecretReportError(obj->conn, VIR_ERR_NO_SECRET, - _("no secret with matching id '%s'"), obj->uuid); + _("no secret with matching uuid '%s'"), uuidstr); goto cleanup; } @@ -808,8 +763,10 @@ secretSetValue(virSecretPtr obj, const unsigned char *value, pptr = secretFind(driver, obj->uuid); if (pptr == NULL) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(obj->uuid, uuidstr); virSecretReportError(obj->conn, VIR_ERR_NO_SECRET, - _("no secret with matching id '%s'"), obj->uuid); + _("no secret with matching uuid '%s'"), uuidstr); goto cleanup; } secret = *pptr; @@ -859,14 +816,18 @@ secretGetValue(virSecretPtr obj, size_t *value_size, unsigned int flags) pptr = secretFind(driver, obj->uuid); if (pptr == NULL) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(obj->uuid, uuidstr); virSecretReportError(obj->conn, VIR_ERR_NO_SECRET, - _("no secret with matching id '%s'"), obj->uuid); + _("no secret with matching uuid '%s'"), uuidstr); goto cleanup; } secret = *pptr; if (secret->value == NULL) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(obj->uuid, uuidstr); virSecretReportError(obj->conn, VIR_ERR_NO_SECRET, - _("secret '%s' does not have a value"), obj->uuid); + _("secret '%s' does not have a value"), uuidstr); goto cleanup; } @@ -901,8 +862,10 @@ secretUndefine(virSecretPtr obj) pptr = secretFind(driver, obj->uuid); if (pptr == NULL) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(obj->uuid, uuidstr); virSecretReportError(obj->conn, VIR_ERR_NO_SECRET, - _("no secret with matching id '%s'"), obj->uuid); + _("no secret with matching uuid '%s'"), uuidstr); goto cleanup; } @@ -1036,7 +999,7 @@ static virSecretDriver secretDriver = { .close = secretClose, .numOfSecrets = secretNumOfSecrets, .listSecrets = secretListSecrets, - .lookupByUUIDString = secretLookupByUUIDString, + .lookupByUUID = secretLookupByUUID, .defineXML = secretDefineXML, .getXMLDesc = secretGetXMLDesc, .setValue = secretSetValue, diff --git a/src/storage_backend.c b/src/storage_backend.c index cf5a593..800d4ea 100644 --- a/src/storage_backend.c +++ b/src/storage_backend.c @@ -50,8 +50,9 @@ #include "node_device.h" #include "internal.h" #include "secret_conf.h" - +#include "uuid.h" #include "storage_backend.h" +#include "logging.h" #if WITH_STORAGE_LVM #include "storage_backend_logical.h" @@ -338,6 +339,32 @@ cleanup: } static int +virStorageGenerateSecretUUID(virConnectPtr conn, + unsigned char *uuid) +{ + unsigned attempt; + + for (attempt = 0; attempt < 65536; attempt++) { + virSecretPtr tmp; + if (virUUIDGenerate(uuid) < 0) { + virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("unable to generate uuid")); + return -1; + } + tmp = conn->secretDriver->lookupByUUID(conn, uuid); + if (tmp == NULL) + return 0; + + virSecretFree(tmp); + } + + virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("too many conflicts when generating an uuid")); + + return -1; +} + +static int virStorageGenerateQcowEncryption(virConnectPtr conn, virStorageVolDefPtr vol) { @@ -346,11 +373,13 @@ virStorageGenerateQcowEncryption(virConnectPtr conn, virStorageEncryptionPtr enc; virStorageEncryptionSecretPtr enc_secret = NULL; virSecretPtr secret = NULL; - char *uuid = NULL, *xml; + char *xml; unsigned char value[VIR_STORAGE_QCOW_PASSPHRASE_SIZE]; int ret = -1; - if (conn->secretDriver == NULL || conn->secretDriver->defineXML == NULL || + if (conn->secretDriver == NULL || + conn->secretDriver->lookupByUUID == NULL || + conn->secretDriver->defineXML == NULL || conn->secretDriver->setValue == NULL) { virStorageReportError(conn, VIR_ERR_NO_SUPPORT, "%s", _("secret storage not supported")); @@ -372,12 +401,9 @@ virStorageGenerateQcowEncryption(virConnectPtr conn, def->ephemeral = 0; def->private = 0; - def->id = NULL; /* Chosen by the secret driver */ - if (virAsprintf(&def->description, "qcow passphrase for %s", - vol->target.path) == -1) { - virReportOOMError(conn); + if (virStorageGenerateSecretUUID(conn, def->uuid) < 0) goto cleanup; - } + def->usage_type = VIR_SECRET_USAGE_TYPE_VOLUME; def->usage.volume = strdup(vol->target.path); if (def->usage.volume == NULL) { @@ -397,22 +423,14 @@ virStorageGenerateQcowEncryption(virConnectPtr conn, } VIR_FREE(xml); - uuid = strdup(secret->uuid); - if (uuid == NULL) { - virReportOOMError(conn); - goto cleanup; - } - if (virStorageGenerateQcowPassphrase(conn, value) < 0) goto cleanup; if (conn->secretDriver->setValue(secret, value, sizeof(value), 0) < 0) goto cleanup; - secret = NULL; enc_secret->type = VIR_STORAGE_ENCRYPTION_SECRET_TYPE_PASSPHRASE; - enc_secret->uuid = uuid; - uuid = NULL; + memcpy(enc_secret->uuid, secret->uuid, VIR_UUID_BUFLEN); enc->format = VIR_STORAGE_ENCRYPTION_FORMAT_QCOW; enc->secrets[0] = enc_secret; /* Space for secrets[0] allocated above */ enc_secret = NULL; @@ -421,9 +439,9 @@ virStorageGenerateQcowEncryption(virConnectPtr conn, ret = 0; cleanup: - VIR_FREE(uuid); if (secret != NULL) { - if (conn->secretDriver->undefine != NULL) + if (ret != 0 && + conn->secretDriver->undefine != NULL) conn->secretDriver->undefine(secret); virSecretFree(secret); } diff --git a/src/storage_encryption_conf.c b/src/storage_encryption_conf.c index 1ce76b5..b97b989 100644 --- a/src/storage_encryption_conf.c +++ b/src/storage_encryption_conf.c @@ -34,6 +34,7 @@ #include "util.h" #include "xml.h" #include "virterror_internal.h" +#include "uuid.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -49,7 +50,6 @@ virStorageEncryptionSecretFree(virStorageEncryptionSecretPtr secret) { if (!secret) return; - VIR_FREE(secret->uuid); VIR_FREE(secret); } @@ -77,6 +77,7 @@ virStorageEncryptionSecretParse(virConnectPtr conn, xmlXPathContextPtr ctxt, virStorageEncryptionSecretPtr ret; char *type_str; int type; + char *uuidstr = NULL; if (VIR_ALLOC(ret) < 0) { virReportOOMError(conn); @@ -103,12 +104,25 @@ virStorageEncryptionSecretParse(virConnectPtr conn, xmlXPathContextPtr ctxt, VIR_FREE(type_str); ret->type = type; - ret->uuid = virXPathString(conn, "string(./@uuid)", ctxt); + uuidstr = virXPathString(conn, "string(./@uuid)", ctxt); + if (uuidstr) { + if (virUUIDParse(uuidstr, ret->uuid) < 0) { + virStorageReportError(conn, VIR_ERR_XML_ERROR, + _("malformed volume encryption uuid '%s'"), + uuidstr); + goto cleanup; + } + } else { + virStorageReportError(conn, VIR_ERR_XML_ERROR, "%s", + _("missing volume encryption uuid")); + goto cleanup; + } ctxt->node = old_node; return ret; cleanup: virStorageEncryptionSecretFree(ret); + VIR_FREE(uuidstr); ctxt->node = old_node; return NULL; } @@ -205,6 +219,7 @@ virStorageEncryptionSecretFormat(virConnectPtr conn, virStorageEncryptionSecretPtr secret) { const char *type; + char uuidstr[VIR_UUID_STRING_BUFLEN]; type = virStorageEncryptionSecretTypeTypeToString(secret->type); if (!type) { @@ -213,10 +228,8 @@ virStorageEncryptionSecretFormat(virConnectPtr conn, return -1; } - virBufferVSprintf(buf, " <secret type='%s'", type); - if (secret->uuid != NULL) - virBufferEscapeString(buf, " uuid='%s'", secret->uuid); - virBufferAddLit(buf, "/>\n"); + virUUIDFormat(secret->uuid, uuidstr); + virBufferVSprintf(buf, " <secret type='%s' uuid='%s'/>\n", type, uuidstr); return 0; } @@ -234,14 +247,14 @@ virStorageEncryptionFormat(virConnectPtr conn, "%s", _("unexpected encryption format")); return -1; } - virBufferVSprintf(buf, " <encryption format='%s'>\n", format); + virBufferVSprintf(buf, " <encryption format='%s'>\n", format); for (i = 0; i < enc->nsecrets; i++) { if (virStorageEncryptionSecretFormat(conn, buf, enc->secrets[i]) < 0) return -1; } - virBufferAddLit(buf, " </encryption>\n"); + virBufferAddLit(buf, " </encryption>\n"); return 0; } diff --git a/src/storage_encryption_conf.h b/src/storage_encryption_conf.h index cdcf625..5d0bc3c 100644 --- a/src/storage_encryption_conf.h +++ b/src/storage_encryption_conf.h @@ -41,7 +41,7 @@ typedef struct _virStorageEncryptionSecret virStorageEncryptionSecret; typedef virStorageEncryptionSecret *virStorageEncryptionSecretPtr; struct _virStorageEncryptionSecret { int type; /* enum virStorageEncryptionSecretType */ - char *uuid; + unsigned char uuid[VIR_UUID_BUFLEN]; }; enum virStorageEncryptionFormat { @@ -63,6 +63,7 @@ struct _virStorageEncryption { }; void virStorageEncryptionFree(virStorageEncryptionPtr enc); + virStorageEncryptionPtr virStorageEncryptionParseNode(virConnectPtr conn, xmlDocPtr xml, xmlNodePtr root); diff --git a/src/virsh.c b/src/virsh.c index 9dc8857..74147da 100644 --- a/src/virsh.c +++ b/src/virsh.c @@ -5270,8 +5270,9 @@ static const vshCmdOptDef opts_secret_define[] = { static int cmdSecretDefine(vshControl *ctl, const vshCmd *cmd) { - char *from, *buffer, *uuid; + char *from, *buffer; virSecretPtr res; + char uuid[VIR_UUID_STRING_BUFLEN]; if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) return FALSE; @@ -5290,15 +5291,13 @@ cmdSecretDefine(vshControl *ctl, const vshCmd *cmd) vshError(ctl, FALSE, _("Failed to set attributes from %s"), from); return FALSE; } - uuid = virSecretGetUUIDString(res); - if (uuid == NULL) { + if (virSecretGetUUIDString(res, &(uuid[0])) < 0) { vshError(ctl, FALSE, "%s", _("Failed to get UUID of created secret")); virSecretFree(res); return FALSE; } vshPrint(ctl, _("Secret %s created\n"), uuid); - free(uuid); virSecretFree(res); return TRUE; } -- 1.6.2.5
On Fri, Sep 11, 2009 at 03:19:17PM +0100, Daniel P. Berrange wrote:
Convert all the secret/storage encryption APIs / wire format to handle UUIDs in raw format instead of non-canonical printable format. Guarentees data format correctness. [...] +++ b/include/libvirt/libvirt.h @@ -1467,12 +1467,17 @@ int virConnectNumOfSecrets (virConnectPtr conn); int virConnectListSecrets (virConnectPtr conn, char **uuids, int maxuuids); +virSecretPtr virSecretLookupByUUID(virConnectPtr conn, + const unsigned char *uuid); virSecretPtr virSecretLookupByUUIDString(virConnectPtr conn, const char *uuid); virSecretPtr virSecretDefineXML (virConnectPtr conn, const char *xml, unsigned int flags); -char * virSecretGetUUIDString (virSecretPtr secret); +int virSecretGetUUID (virSecretPtr secret, + unsigned char *buf); +int virSecretGetUUIDString (virSecretPtr secret, + char *buf); char * virSecretGetXMLDesc (virSecretPtr secret, unsigned int flags); int virSecretSetValue (virSecretPtr secret,
explicit ACK, this is better ! [..]
@@ -4828,7 +4828,7 @@ get_nonnull_storage_vol (virConnectPtr conn, remote_nonnull_storage_vol vol) static virSecretPtr get_nonnull_secret (virConnectPtr conn, remote_nonnull_secret secret) { - return virGetSecret (conn, secret.uuid); + return virGetSecret (conn, BAD_CAST secret.uuid);
heh I didn't expect BAD_CAST to be used outside libxml2 :-) [...]
@@ -1179,7 +1181,9 @@ virGetSecret(virConnectPtr conn, const char *uuid) } virMutexLock(&conn->lock);
- ret = virHashLookup(conn->secrets, uuid); + virUUIDFormat(uuid, uuidstr); + + ret = virHashLookup(conn->secrets, uuidstr);
Would you mind also removing the Returns 0 in case of success and -1 in case of error. comment for virUUIDFormat() as it's void and doesn't do any checking (it has no way to). [...]
/** - * virSecretLookupByUUIDString: - * @conn: virConnect connection - * @uuid: ID of a secret + * virSecretLookupByUUID: + * @conn: pointer to the hypervisor connection + * @uuid: the raw UUID for the secret * - * Fetches a secret based on uuid. + * Try to lookup a secret on the given hypervisor based on its UUID.
+ * uses the 16 bytes of raw data to describe the UUID
* - * Returns the secret on success, or NULL on failure. + * Returns a new secret object or NULL in case of failure. If the + * secret cannot be found, then VIR_ERR_NO_SECRET error is raised. */ virSecretPtr -virSecretLookupByUUIDString(virConnectPtr conn, const char *uuid) +virSecretLookupByUUID(virConnectPtr conn, const unsigned char *uuid) [...]
/** + * virSecretLookupByUUIDString: + * @conn: pointer to the hypervisor connection + * @uuidstr: the string UUID for the secret + * + * Try to lookup a secret on the given hypervisor based on its UUID.
+ * Uses the printable string value to describe the UUID
+ * + * Returns a new secret object or NULL in case of failure. If the + * secret cannot be found, then VIR_ERR_NO_SECRET error is raised. + */ +virSecretPtr +virSecretLookupByUUIDString(virConnectPtr conn, const char *uuidstr) +{ + int raw[VIR_UUID_BUFLEN], i; + unsigned char uuid[VIR_UUID_BUFLEN]; + int ret; + + DEBUG("conn=%p, uuidstr=%s", conn, uuidstr); + + virResetLastError(); + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__); + return (NULL); + } + if (uuidstr == NULL) { + virLibConnError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + /* XXX: sexpr_uuid() also supports 'xxxx-xxxx-xxxx-xxxx' format. + * We needn't it here. Right? + */
Hum, I would rather use virUUIDParse() to make sure there is no confusion, a public API should be tolerant about this I think and it's cleaner to reuse it !
+ ret = sscanf(uuidstr, + "%02x%02x%02x%02x-" + "%02x%02x-" + "%02x%02x-" + "%02x%02x-" + "%02x%02x%02x%02x%02x%02x", + raw + 0, raw + 1, raw + 2, raw + 3, + raw + 4, raw + 5, raw + 6, raw + 7, + raw + 8, raw + 9, raw + 10, raw + 11, + raw + 12, raw + 13, raw + 14, raw + 15); + + if (ret!=VIR_UUID_BUFLEN) { + virLibConnError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + for (i = 0; i < VIR_UUID_BUFLEN; i++) + uuid[i] = raw[i] & 0xFF;
ACK, overall an important cleanup, as it affects the API and the network format ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Fri, Sep 11, 2009 at 06:00:07PM +0200, Daniel Veillard wrote:
On Fri, Sep 11, 2009 at 03:19:17PM +0100, Daniel P. Berrange wrote:
Convert all the secret/storage encryption APIs / wire format to handle UUIDs in raw format instead of non-canonical printable format. Guarentees data format correctness.
+ * + * Returns a new secret object or NULL in case of failure. If the + * secret cannot be found, then VIR_ERR_NO_SECRET error is raised. + */ +virSecretPtr +virSecretLookupByUUIDString(virConnectPtr conn, const char *uuidstr) +{ + int raw[VIR_UUID_BUFLEN], i; + unsigned char uuid[VIR_UUID_BUFLEN]; + int ret; + + DEBUG("conn=%p, uuidstr=%s", conn, uuidstr); + + virResetLastError(); + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__); + return (NULL); + } + if (uuidstr == NULL) { + virLibConnError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + /* XXX: sexpr_uuid() also supports 'xxxx-xxxx-xxxx-xxxx' format. + * We needn't it here. Right? + */
Hum, I would rather use virUUIDParse() to make sure there is no confusion, a public API should be tolerant about this I think and it's cleaner to reuse it !
This was in fact just cut+paste from the existing APIs of similar name in libvirt.c. I'll send another patch which fixes them all to use virUUIDParse.
+ ret = sscanf(uuidstr, + "%02x%02x%02x%02x-" + "%02x%02x-" + "%02x%02x-" + "%02x%02x-" + "%02x%02x%02x%02x%02x%02x", + raw + 0, raw + 1, raw + 2, raw + 3, + raw + 4, raw + 5, raw + 6, raw + 7, + raw + 8, raw + 9, raw + 10, raw + 11, + raw + 12, raw + 13, raw + 14, raw + 15); + + if (ret!=VIR_UUID_BUFLEN) { + virLibConnError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + for (i = 0; i < VIR_UUID_BUFLEN; i++) + uuid[i] = raw[i] & 0xFF;
ACK, overall an important cleanup, as it affects the API and the network format !
Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
----- "Daniel P. Berrange" <berrange@redhat.com> wrote:
@@ -635,49 +620,25 @@ secretLookupByUUIDString(virConnectPtr conn, const char *uuid)
pptr = secretFind(driver, uuid); if (pptr == NULL) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(uuid, uuidstr); virSecretReportError(conn, VIR_ERR_NO_SECRET, - _("no secret with matching id '%s'"), uuid); + _("no secret with matching uuid '%s'"), uuidstr); goto cleanup; }
- ret = virGetSecret(conn, (*pptr)->def->id); + ret = virGetSecret(conn, (*pptr)->def->uuid);
cleanup: + if (1) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(uuid, uuidstr); + VIR_ERROR("Lookup %s got %p", uuidstr, ret); + } This looks like a debug print that should be removed in the final version. Mirek
On Fri, Sep 11, 2009 at 08:28:59PM -0400, Miloslav Trmac wrote:
----- "Daniel P. Berrange" <berrange@redhat.com> wrote:
@@ -635,49 +620,25 @@ secretLookupByUUIDString(virConnectPtr conn, const char *uuid)
pptr = secretFind(driver, uuid); if (pptr == NULL) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(uuid, uuidstr); virSecretReportError(conn, VIR_ERR_NO_SECRET, - _("no secret with matching id '%s'"), uuid); + _("no secret with matching uuid '%s'"), uuidstr); goto cleanup; }
- ret = virGetSecret(conn, (*pptr)->def->id); + ret = virGetSecret(conn, (*pptr)->def->uuid);
cleanup: + if (1) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(uuid, uuidstr); + VIR_ERROR("Lookup %s got %p", uuidstr, ret); + } This looks like a debug print that should be removed in the final version.
Opps, yes that's killed now Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
* include/libvirt/libvirt.h, include/libvirt/libvirt.h.in: Add virSecretGetUsageType, virSecretGetUsageID and virLookupSecretByUsage * python/generator.py: Mark virSecretGetUsageType, virSecretGetUsageID as not throwing exceptions * qemud/remote.c: Implement dispatch for virLookupSecretByUsage * qemud/remote_protocol.x: Add usage type & ID as attributes of remote_nonnull_secret. Add RPC calls for new public APIs * qemud/remote_dispatch_args.h, qemud/remote_dispatch_prototypes.h, qemud/remote_dispatch_ret.h, qemud/remote_dispatch_table.h, qemud/remote_protocol.c, qemud/remote_protocol.h: Re-generate * src/datatypes.c, src/datatypes.h: Add usageType and usageID as properties of virSecretPtr * src/driver.h: Add virLookupSecretByUsage driver entry point * src/libvirt.c: Implement virSecretGetUsageType, virSecretGetUsageID and virLookupSecretByUsage * src/libvirt_public.syms: Export virSecretGetUsageType, virSecretGetUsageID and virLookupSecretByUsage * src/remote_internal.c: Implement virLookupSecretByUsage entry * src/secret_conf.c, src/secret_conf.h: Remove the virSecretUsageType enum, now in public API. Make volume path mandatory when parsing XML * src/secret_driver.c: Enforce usage uniqueness when defining secrets. Implement virSecretLookupByUsage api method * src/virsh.c: Include usage for secret-list command --- include/libvirt/libvirt.h | 11 ++ include/libvirt/libvirt.h.in | 11 ++ python/generator.py | 2 + qemud/remote.c | 24 ++++- qemud/remote_dispatch_args.h | 1 + qemud/remote_dispatch_prototypes.h | 7 + qemud/remote_dispatch_ret.h | 1 + qemud/remote_dispatch_table.h | 5 + qemud/remote_protocol.c | 24 ++++ qemud/remote_protocol.h | 18 +++ qemud/remote_protocol.x | 14 ++- src/datatypes.c | 14 ++- src/datatypes.h | 6 +- src/driver.h | 5 + src/libvirt.c | 94 +++++++++++++++ src/libvirt_public.syms | 3 + src/remote_internal.c | 33 +++++- src/secret_conf.c | 7 +- src/secret_conf.h | 6 - src/secret_driver.c | 231 +++++++++++++++++++++++------------- src/virsh.c | 28 ++++- 21 files changed, 448 insertions(+), 97 deletions(-) diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h index eadf420..5527600 100644 --- a/include/libvirt/libvirt.h +++ b/include/libvirt/libvirt.h @@ -1462,6 +1462,12 @@ void virEventRegisterImpl(virEventAddHandleFunc addHandle, typedef struct _virSecret virSecret; typedef virSecret *virSecretPtr; +typedef enum { + VIR_SECRET_USAGE_TYPE_NONE = 0, + VIR_SECRET_USAGE_TYPE_VOLUME = 1, + /* Expect more owner types later... */ +} virSecretUsageType; + virConnectPtr virSecretGetConnect (virSecretPtr secret); int virConnectNumOfSecrets (virConnectPtr conn); int virConnectListSecrets (virConnectPtr conn, @@ -1471,6 +1477,9 @@ virSecretPtr virSecretLookupByUUID(virConnectPtr conn, const unsigned char *uuid); virSecretPtr virSecretLookupByUUIDString(virConnectPtr conn, const char *uuid); +virSecretPtr virSecretLookupByUsage(virConnectPtr conn, + int usageType, + const char *usageID); virSecretPtr virSecretDefineXML (virConnectPtr conn, const char *xml, unsigned int flags); @@ -1478,6 +1487,8 @@ int virSecretGetUUID (virSecretPtr secret, unsigned char *buf); int virSecretGetUUIDString (virSecretPtr secret, char *buf); +int virSecretGetUsageType (virSecretPtr secret); +const char * virSecretGetUsageID (virSecretPtr secret); char * virSecretGetXMLDesc (virSecretPtr secret, unsigned int flags); int virSecretSetValue (virSecretPtr secret, diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 1391af8..6028d5f 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1462,6 +1462,12 @@ void virEventRegisterImpl(virEventAddHandleFunc addHandle, typedef struct _virSecret virSecret; typedef virSecret *virSecretPtr; +typedef enum { + VIR_SECRET_USAGE_TYPE_NONE = 0, + VIR_SECRET_USAGE_TYPE_VOLUME = 1, + /* Expect more owner types later... */ +} virSecretUsageType; + virConnectPtr virSecretGetConnect (virSecretPtr secret); int virConnectNumOfSecrets (virConnectPtr conn); int virConnectListSecrets (virConnectPtr conn, @@ -1471,6 +1477,9 @@ virSecretPtr virSecretLookupByUUID(virConnectPtr conn, const unsigned char *uuid); virSecretPtr virSecretLookupByUUIDString(virConnectPtr conn, const char *uuid); +virSecretPtr virSecretLookupByUsage(virConnectPtr conn, + int usageType, + const char *usageID); virSecretPtr virSecretDefineXML (virConnectPtr conn, const char *xml, unsigned int flags); @@ -1478,6 +1487,8 @@ int virSecretGetUUID (virSecretPtr secret, unsigned char *buf); int virSecretGetUUIDString (virSecretPtr secret, char *buf); +int virSecretGetUsageType (virSecretPtr secret); +const char * virSecretGetUsageID (virSecretPtr secret); char * virSecretGetXMLDesc (virSecretPtr secret, unsigned int flags); int virSecretSetValue (virSecretPtr secret, diff --git a/python/generator.py b/python/generator.py index c25ff55..ad9c544 100755 --- a/python/generator.py +++ b/python/generator.py @@ -669,6 +669,8 @@ functions_noexcept = { 'virStorageVolGetkey': True, 'virNodeDeviceGetName': True, 'virNodeDeviceGetParent': True, + 'virSecretGetUsageType': True, + 'virSecretGetUsageID': True, } reference_keepers = { diff --git a/qemud/remote.c b/qemud/remote.c index a9fcc58..17426cd 100644 --- a/qemud/remote.c +++ b/qemud/remote.c @@ -4778,6 +4778,26 @@ remoteDispatchSecretUndefine (struct qemud_server *server ATTRIBUTE_UNUSED, return 0; } +static int +remoteDispatchSecretLookupByUsage (struct qemud_server *server ATTRIBUTE_UNUSED, + struct qemud_client *client ATTRIBUTE_UNUSED, + virConnectPtr conn, remote_error *err, + remote_secret_lookup_by_usage_args *args, + remote_secret_lookup_by_usage_ret *ret) +{ + virSecretPtr secret; + + secret = virSecretLookupByUsage (conn, args->usageType, args->usageID); + if (secret == NULL) { + remoteDispatchConnError (err, conn); + return -1; + } + + make_nonnull_secret (&ret->secret, secret); + virSecretFree (secret); + return 0; +} + /*----- Helpers. -----*/ @@ -4828,7 +4848,7 @@ get_nonnull_storage_vol (virConnectPtr conn, remote_nonnull_storage_vol vol) static virSecretPtr get_nonnull_secret (virConnectPtr conn, remote_nonnull_secret secret) { - return virGetSecret (conn, BAD_CAST secret.uuid); + return virGetSecret (conn, BAD_CAST secret.uuid, secret.usageType, secret.usageID); } /* Make remote_nonnull_domain and remote_nonnull_network. */ @@ -4880,4 +4900,6 @@ static void make_nonnull_secret (remote_nonnull_secret *secret_dst, virSecretPtr secret_src) { memcpy (secret_dst->uuid, secret_src->uuid, VIR_UUID_BUFLEN); + secret_dst->usageType = secret_src->usageType; + secret_dst->usageID = strdup (secret_src->usageID); } diff --git a/qemud/remote_dispatch_args.h b/qemud/remote_dispatch_args.h index 9f37963..95f668a 100644 --- a/qemud/remote_dispatch_args.h +++ b/qemud/remote_dispatch_args.h @@ -124,3 +124,4 @@ remote_secret_set_value_args val_remote_secret_set_value_args; remote_secret_get_value_args val_remote_secret_get_value_args; remote_secret_undefine_args val_remote_secret_undefine_args; + remote_secret_lookup_by_usage_args val_remote_secret_lookup_by_usage_args; diff --git a/qemud/remote_dispatch_prototypes.h b/qemud/remote_dispatch_prototypes.h index 7773cd9..0605542 100644 --- a/qemud/remote_dispatch_prototypes.h +++ b/qemud/remote_dispatch_prototypes.h @@ -807,6 +807,13 @@ static int remoteDispatchSecretGetXmlDesc( remote_error *err, remote_secret_get_xml_desc_args *args, remote_secret_get_xml_desc_ret *ret); +static int remoteDispatchSecretLookupByUsage( + struct qemud_server *server, + struct qemud_client *client, + virConnectPtr conn, + remote_error *err, + remote_secret_lookup_by_usage_args *args, + remote_secret_lookup_by_usage_ret *ret); static int remoteDispatchSecretLookupByUuid( struct qemud_server *server, struct qemud_client *client, diff --git a/qemud/remote_dispatch_ret.h b/qemud/remote_dispatch_ret.h index 15d3386..6ced13a 100644 --- a/qemud/remote_dispatch_ret.h +++ b/qemud/remote_dispatch_ret.h @@ -105,3 +105,4 @@ remote_secret_define_xml_ret val_remote_secret_define_xml_ret; remote_secret_get_xml_desc_ret val_remote_secret_get_xml_desc_ret; remote_secret_get_value_ret val_remote_secret_get_value_ret; + remote_secret_lookup_by_usage_ret val_remote_secret_lookup_by_usage_ret; diff --git a/qemud/remote_dispatch_table.h b/qemud/remote_dispatch_table.h index 07e36bb..6b5df80 100644 --- a/qemud/remote_dispatch_table.h +++ b/qemud/remote_dispatch_table.h @@ -737,3 +737,8 @@ .args_filter = (xdrproc_t) xdr_remote_secret_undefine_args, .ret_filter = (xdrproc_t) xdr_void, }, +{ /* SecretLookupByUsage => 147 */ + .fn = (dispatch_fn) remoteDispatchSecretLookupByUsage, + .args_filter = (xdrproc_t) xdr_remote_secret_lookup_by_usage_args, + .ret_filter = (xdrproc_t) xdr_remote_secret_lookup_by_usage_ret, +}, diff --git a/qemud/remote_protocol.c b/qemud/remote_protocol.c index b6666a1..1d2d242 100644 --- a/qemud/remote_protocol.c +++ b/qemud/remote_protocol.c @@ -109,6 +109,10 @@ xdr_remote_nonnull_secret (XDR *xdrs, remote_nonnull_secret *objp) if (!xdr_remote_uuid (xdrs, objp->uuid)) return FALSE; + if (!xdr_int (xdrs, &objp->usageType)) + return FALSE; + if (!xdr_remote_nonnull_string (xdrs, &objp->usageID)) + return FALSE; return TRUE; } @@ -2674,6 +2678,26 @@ xdr_remote_secret_undefine_args (XDR *xdrs, remote_secret_undefine_args *objp) } bool_t +xdr_remote_secret_lookup_by_usage_args (XDR *xdrs, remote_secret_lookup_by_usage_args *objp) +{ + + if (!xdr_int (xdrs, &objp->usageType)) + return FALSE; + if (!xdr_remote_nonnull_string (xdrs, &objp->usageID)) + return FALSE; + return TRUE; +} + +bool_t +xdr_remote_secret_lookup_by_usage_ret (XDR *xdrs, remote_secret_lookup_by_usage_ret *objp) +{ + + if (!xdr_remote_nonnull_secret (xdrs, &objp->secret)) + return FALSE; + return TRUE; +} + +bool_t xdr_remote_procedure (XDR *xdrs, remote_procedure *objp) { diff --git a/qemud/remote_protocol.h b/qemud/remote_protocol.h index 4b73ee1..ceaf82c 100644 --- a/qemud/remote_protocol.h +++ b/qemud/remote_protocol.h @@ -87,6 +87,8 @@ typedef struct remote_nonnull_node_device remote_nonnull_node_device; struct remote_nonnull_secret { remote_uuid uuid; + int usageType; + remote_nonnull_string usageID; }; typedef struct remote_nonnull_secret remote_nonnull_secret; @@ -1513,6 +1515,17 @@ struct remote_secret_undefine_args { remote_nonnull_secret secret; }; typedef struct remote_secret_undefine_args remote_secret_undefine_args; + +struct remote_secret_lookup_by_usage_args { + int usageType; + remote_nonnull_string usageID; +}; +typedef struct remote_secret_lookup_by_usage_args remote_secret_lookup_by_usage_args; + +struct remote_secret_lookup_by_usage_ret { + remote_nonnull_secret secret; +}; +typedef struct remote_secret_lookup_by_usage_ret remote_secret_lookup_by_usage_ret; #define REMOTE_PROGRAM 0x20008086 #define REMOTE_PROTOCOL_VERSION 1 @@ -1663,6 +1676,7 @@ enum remote_procedure { REMOTE_PROC_SECRET_SET_VALUE = 144, REMOTE_PROC_SECRET_GET_VALUE = 145, REMOTE_PROC_SECRET_UNDEFINE = 146, + REMOTE_PROC_SECRET_LOOKUP_BY_USAGE = 147, }; typedef enum remote_procedure remote_procedure; @@ -1939,6 +1953,8 @@ extern bool_t xdr_remote_secret_set_value_args (XDR *, remote_secret_set_value_ extern bool_t xdr_remote_secret_get_value_args (XDR *, remote_secret_get_value_args*); extern bool_t xdr_remote_secret_get_value_ret (XDR *, remote_secret_get_value_ret*); extern bool_t xdr_remote_secret_undefine_args (XDR *, remote_secret_undefine_args*); +extern bool_t xdr_remote_secret_lookup_by_usage_args (XDR *, remote_secret_lookup_by_usage_args*); +extern bool_t xdr_remote_secret_lookup_by_usage_ret (XDR *, remote_secret_lookup_by_usage_ret*); extern bool_t xdr_remote_procedure (XDR *, remote_procedure*); extern bool_t xdr_remote_message_type (XDR *, remote_message_type*); extern bool_t xdr_remote_message_status (XDR *, remote_message_status*); @@ -2191,6 +2207,8 @@ extern bool_t xdr_remote_secret_set_value_args (); extern bool_t xdr_remote_secret_get_value_args (); extern bool_t xdr_remote_secret_get_value_ret (); extern bool_t xdr_remote_secret_undefine_args (); +extern bool_t xdr_remote_secret_lookup_by_usage_args (); +extern bool_t xdr_remote_secret_lookup_by_usage_ret (); extern bool_t xdr_remote_procedure (); extern bool_t xdr_remote_message_type (); extern bool_t xdr_remote_message_status (); diff --git a/qemud/remote_protocol.x b/qemud/remote_protocol.x index 5712d98..29abdb7 100644 --- a/qemud/remote_protocol.x +++ b/qemud/remote_protocol.x @@ -189,6 +189,8 @@ struct remote_nonnull_node_device { /* A secret which may not be null. */ struct remote_nonnull_secret { remote_uuid uuid; + int usageType; + remote_nonnull_string usageID; }; /* A domain or network which may be NULL. */ @@ -1338,6 +1340,15 @@ struct remote_secret_undefine_args { remote_nonnull_secret secret; }; +struct remote_secret_lookup_by_usage_args { + int usageType; + remote_nonnull_string usageID; +}; + +struct remote_secret_lookup_by_usage_ret { + remote_nonnull_secret secret; +}; + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -1505,7 +1516,8 @@ enum remote_procedure { REMOTE_PROC_SECRET_GET_XML_DESC = 143, REMOTE_PROC_SECRET_SET_VALUE = 144, REMOTE_PROC_SECRET_GET_VALUE = 145, - REMOTE_PROC_SECRET_UNDEFINE = 146 + REMOTE_PROC_SECRET_UNDEFINE = 146, + REMOTE_PROC_SECRET_LOOKUP_BY_USAGE = 147 }; diff --git a/src/datatypes.c b/src/datatypes.c index b0067f6..d7cf2ee 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -1170,12 +1170,13 @@ virUnrefNodeDevice(virNodeDevicePtr dev) { * Returns a pointer to the secret, or NULL in case of failure */ virSecretPtr -virGetSecret(virConnectPtr conn, const unsigned char *uuid) +virGetSecret(virConnectPtr conn, const unsigned char *uuid, + int usageType, const char *usageID) { virSecretPtr ret = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; - if (!VIR_IS_CONNECT(conn) || uuid == NULL) { + if (!VIR_IS_CONNECT(conn) || uuid == NULL || usageID == NULL) { virLibConnError(NULL, VIR_ERR_INVALID_ARG, __FUNCTION__); return NULL; } @@ -1193,7 +1194,12 @@ virGetSecret(virConnectPtr conn, const unsigned char *uuid) ret->magic = VIR_SECRET_MAGIC; ret->conn = conn; memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN); - + ret->usageType = usageType; + if (!(ret->usageID = strdup(usageID))) { + virMutexUnlock(&conn->lock); + virReportOOMError(conn); + goto error; + } if (virHashAddEntry(conn->secrets, uuidstr, ret) < 0) { virMutexUnlock(&conn->lock); virLibConnError(conn, VIR_ERR_INTERNAL_ERROR, @@ -1208,6 +1214,7 @@ virGetSecret(virConnectPtr conn, const unsigned char *uuid) error: if (ret != NULL) { + VIR_FREE(ret->usageID); VIR_FREE(ret->uuid); VIR_FREE(ret); } @@ -1239,6 +1246,7 @@ virReleaseSecret(virSecretPtr secret) { conn = NULL; } + VIR_FREE(secret->usageID); secret->magic = -1; VIR_FREE(secret); diff --git a/src/datatypes.h b/src/datatypes.h index 5319308..a33c365 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -256,6 +256,8 @@ struct _virSecret { int refs; /* reference count */ virConnectPtr conn; /* pointer back to the connection */ unsigned char uuid[VIR_UUID_BUFLEN]; /* the domain unique identifier */ + int usageType; /* the type of usage */ + char *usageID; /* the usage's unique identifier */ }; @@ -296,7 +298,9 @@ virNodeDevicePtr virGetNodeDevice(virConnectPtr conn, int virUnrefNodeDevice(virNodeDevicePtr dev); virSecretPtr virGetSecret(virConnectPtr conn, - const unsigned char *uuid); + const unsigned char *uuid, + int usageType, + const char *usageID); int virUnrefSecret(virSecretPtr secret); #endif diff --git a/src/driver.h b/src/driver.h index 9f197d9..d4f972f 100644 --- a/src/driver.h +++ b/src/driver.h @@ -822,6 +822,10 @@ typedef virSecretPtr (*virDrvSecretLookupByUUID) (virConnectPtr conn, const unsigned char *uuid); typedef virSecretPtr + (*virDrvSecretLookupByUsage) (virConnectPtr conn, + int usageType, + const char *usageID); +typedef virSecretPtr (*virDrvSecretDefineXML) (virConnectPtr conn, const char *xml, unsigned int flags); @@ -867,6 +871,7 @@ struct _virSecretDriver { virDrvSecretNumOfSecrets numOfSecrets; virDrvSecretListSecrets listSecrets; virDrvSecretLookupByUUID lookupByUUID; + virDrvSecretLookupByUsage lookupByUsage; virDrvSecretDefineXML defineXML; virDrvSecretGetXMLDesc getXMLDesc; virDrvSecretSetValue setValue; diff --git a/src/libvirt.c b/src/libvirt.c index 7a915fa..1563ce5 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -8913,6 +8913,53 @@ error: /** + * virSecretLookupByUsage: + * @conn: pointer to the hypervisor connection + * @usageType: the type of secret usage + * @usageID: identifier of the object using the secret + * + * Try to lookup a secret on the given hypervisor based on its usage + * + * Returns a new secret object or NULL in case of failure. If the + * secret cannot be found, then VIR_ERR_NO_SECRET error is raised. + */ +virSecretPtr +virSecretLookupByUsage(virConnectPtr conn, + int usageType, + const char *usageID) +{ + DEBUG("conn=%p, usageType=%d usageID=%s", conn, usageType, NULLSTR(usageID)); + + virResetLastError(); + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__); + return (NULL); + } + if (usageID == NULL) { + virLibConnError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + if (conn->secretDriver && + conn->secretDriver->lookupByUsage) { + virSecretPtr ret; + ret = conn->secretDriver->lookupByUsage (conn, usageType, usageID); + if (!ret) + goto error; + return ret; + } + + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + /* Copy to connection error object for back compatability */ + virSetConnError(conn); + return NULL; +} + + +/** * virSecretDefineXML: * @conn: virConnect connection * @xml: XML describing the secret. @@ -9036,6 +9083,53 @@ error: return -1; } +/** + * virSecretGetUsageType: + * @secret: a secret object + * + * Get the type of object which uses this secret + * + * Returns a positive integer identifying the type of object, + * or -1 upon error. + */ +int +virSecretGetUsageType(virSecretPtr secret) +{ + DEBUG("secret=%p", secret); + + virResetLastError(); + + if (!VIR_IS_SECRET(secret)) { + virLibSecretError(NULL, VIR_ERR_INVALID_SECRET, __FUNCTION__); + return (-1); + } + return (secret->usageType); +} + +/** + * virSecretGetUsageID: + * @secret: a secret object + * + * Get the unique identifier of the object with which this + * secret is to be used + * + * Returns a string identifying the object using the secret, + * or NULL upon error + */ +const char * +virSecretGetUsageID(virSecretPtr secret) +{ + DEBUG("secret=%p", secret); + + virResetLastError(); + + if (!VIR_IS_SECRET(secret)) { + virLibSecretError(NULL, VIR_ERR_INVALID_SECRET, __FUNCTION__); + return (NULL); + } + return (secret->usageID); +} + /** * virSecretGetXMLDesc: diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index c34bbae..cf5be38 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -300,9 +300,12 @@ LIBVIRT_0.7.1 { virConnectListSecrets; virSecretLookupByUUID; virSecretLookupByUUIDString; + virSecretLookupByUsage; virSecretDefineXML; virSecretGetUUID; virSecretGetUUIDString; + virSecretGetUsageType; + virSecretGetUsageID; virSecretGetXMLDesc; virSecretSetValue; virSecretGetValue; diff --git a/src/remote_internal.c b/src/remote_internal.c index e7f0186..eff268f 100644 --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -6502,6 +6502,34 @@ done: } static virSecretPtr +remoteSecretLookupByUsage (virConnectPtr conn, int usageType, const char *usageID) +{ + virSecretPtr rv = NULL; + remote_secret_lookup_by_usage_args args; + remote_secret_lookup_by_usage_ret ret; + struct private_data *priv = conn->secretPrivateData; + + remoteDriverLock (priv); + + args.usageType = usageType; + args.usageID = (char *)usageID; + + memset (&ret, 0, sizeof (ret)); + if (call (conn, priv, 0, REMOTE_PROC_SECRET_LOOKUP_BY_UUID, + (xdrproc_t) xdr_remote_secret_lookup_by_usage_args, (char *) &args, + (xdrproc_t) xdr_remote_secret_lookup_by_usage_ret, (char *) &ret) == -1) + goto done; + + rv = get_nonnull_secret (conn, ret.secret); + xdr_free ((xdrproc_t) xdr_remote_secret_lookup_by_usage_ret, + (char *) &ret); + +done: + remoteDriverUnlock (priv); + return rv; +} + +static virSecretPtr remoteSecretDefineXML (virConnectPtr conn, const char *xml, unsigned int flags) { virSecretPtr rv = NULL; @@ -7733,7 +7761,7 @@ get_nonnull_node_device (virConnectPtr conn, remote_nonnull_node_device dev) static virSecretPtr get_nonnull_secret (virConnectPtr conn, remote_nonnull_secret secret) { - return virGetSecret(conn, BAD_CAST secret.uuid); + return virGetSecret(conn, BAD_CAST secret.uuid, secret.usageType, secret.usageID); } /* Make remote_nonnull_domain and remote_nonnull_network. */ @@ -7779,6 +7807,8 @@ static void make_nonnull_secret (remote_nonnull_secret *secret_dst, virSecretPtr secret_src) { memcpy (secret_dst->uuid, secret_src->uuid, VIR_UUID_BUFLEN); + secret_dst->usageType = secret_src->usageType; + secret_dst->usageID = secret_src->usageID; } /*----------------------------------------------------------------------*/ @@ -7941,6 +7971,7 @@ static virSecretDriver secret_driver = { .numOfSecrets = remoteSecretNumOfSecrets, .listSecrets = remoteSecretListSecrets, .lookupByUUID = remoteSecretLookupByUUID, + .lookupByUsage = remoteSecretLookupByUsage, .defineXML = remoteSecretDefineXML, .getXMLDesc = remoteSecretGetXMLDesc, .setValue = remoteSecretSetValue, diff --git a/src/secret_conf.c b/src/secret_conf.c index 51ac13d..21215b2 100644 --- a/src/secret_conf.c +++ b/src/secret_conf.c @@ -35,7 +35,7 @@ #define VIR_FROM_THIS VIR_FROM_SECRET -VIR_ENUM_IMPL(virSecretUsageType, VIR_SECRET_USAGE_TYPE_LAST, "none", "volume") +VIR_ENUM_IMPL(virSecretUsageType, VIR_SECRET_USAGE_TYPE_VOLUME + 1, "none", "volume") void virSecretDefFree(virSecretDefPtr def) @@ -88,6 +88,11 @@ virSecretDefParseUsage(virConnectPtr conn, xmlXPathContextPtr ctxt, case VIR_SECRET_USAGE_TYPE_VOLUME: def->usage.volume = virXPathString(conn, "string(./usage/volume)", ctxt); + if (!def->usage.volume) { + virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("volume usage specified, but volume path is missing")); + return -1; + } break; default: diff --git a/src/secret_conf.h b/src/secret_conf.h index 556f5a4..1ecf419 100644 --- a/src/secret_conf.h +++ b/src/secret_conf.h @@ -30,12 +30,6 @@ virReportErrorHelper(conn, VIR_FROM_SECRET, code, __FILE__, \ __FUNCTION__, __LINE__, fmt) -enum virSecretUsageType { - VIR_SECRET_USAGE_TYPE_NONE = 0, /* default when zero-initialized */ - VIR_SECRET_USAGE_TYPE_VOLUME, - - VIR_SECRET_USAGE_TYPE_LAST -}; VIR_ENUM_DECL(virSecretUsageType) typedef struct _virSecretDef virSecretDef; diff --git a/src/secret_driver.c b/src/secret_driver.c index c1e0c67..a60b1ca 100644 --- a/src/secret_driver.c +++ b/src/secret_driver.c @@ -110,57 +110,45 @@ secretFree(virSecretEntryPtr secret) VIR_FREE(secret); } -static virSecretEntryPtr * -secretFind(virSecretDriverStatePtr driver, const unsigned char *uuid) +static virSecretEntryPtr +secretFindByUUID(virSecretDriverStatePtr driver, const unsigned char *uuid) { virSecretEntryPtr *pptr, s; for (pptr = &driver->secrets; *pptr != NULL; pptr = &s->next) { s = *pptr; if (memcmp(s->def->uuid, uuid, VIR_UUID_BUFLEN) == 0) - return pptr; + return s; } return NULL; } static virSecretEntryPtr -secretCreate(virConnectPtr conn, virSecretDriverStatePtr driver, - const unsigned char *uuid) +secretFindByUsage(virSecretDriverStatePtr driver, int usageType, const char *usageID) { - virSecretEntryPtr secret = NULL; + virSecretEntryPtr *pptr, s; - if (VIR_ALLOC(secret) < 0 || VIR_ALLOC(secret->def)) - goto no_memory; - memcpy(secret->def->uuid, uuid, VIR_UUID_BUFLEN); - listInsert(&driver->secrets, secret); - return secret; + for (pptr = &driver->secrets; *pptr != NULL; pptr = &s->next) { + s = *pptr; - no_memory: - virReportOOMError(conn); - secretFree(secret); - return NULL; -} + if (s->def->usage_type != usageType) + continue; -static virSecretEntryPtr -secretFindOrCreate(virConnectPtr conn, virSecretDriverStatePtr driver, - const unsigned char *uuid, bool *created_new) -{ - virSecretEntryPtr *pptr, secret; + switch (usageType) { + case VIR_SECRET_USAGE_TYPE_NONE: + /* never match this */ + break; - pptr = secretFind(driver, uuid); - if (pptr != NULL) { - if (created_new != NULL) - *created_new = false; - return *pptr; + case VIR_SECRET_USAGE_TYPE_VOLUME: + if (STREQ(s->def->usage.volume, usageID)) + return s; + break; + } } - - secret = secretCreate(conn, driver, uuid); - if (secret != NULL && created_new != NULL) - *created_new = true; - return secret; + return NULL; } - /* Permament secret storage */ +/* Permament secret storage */ /* Secrets are stored in virSecretDriverStatePtr->directory. Each secret has virSecretDef stored as XML in "$basename.xml". If a value of the @@ -609,17 +597,33 @@ cleanup: return -1; } + +static const char * +secretUsageIDForDef(virSecretDefPtr def) +{ + switch (def->usage_type) { + case VIR_SECRET_USAGE_TYPE_NONE: + return "none"; + + case VIR_SECRET_USAGE_TYPE_VOLUME: + return def->usage.volume; + + default: + return NULL; + } +} + static virSecretPtr secretLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { virSecretDriverStatePtr driver = conn->secretPrivateData; virSecretPtr ret = NULL; - virSecretEntryPtr *pptr; + virSecretEntryPtr secret; secretDriverLock(driver); - pptr = secretFind(driver, uuid); - if (pptr == NULL) { + secret = secretFindByUUID(driver, uuid); + if (secret == NULL) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(uuid, uuidstr); virSecretReportError(conn, VIR_ERR_NO_SECRET, @@ -627,7 +631,10 @@ secretLookupByUUID(virConnectPtr conn, const unsigned char *uuid) goto cleanup; } - ret = virGetSecret(conn, (*pptr)->def->uuid); + ret = virGetSecret(conn, + secret->def->uuid, + secret->def->usage_type, + secretUsageIDForDef(secret->def)); cleanup: if (1) { @@ -641,14 +648,41 @@ cleanup: static virSecretPtr +secretLookupByUsage(virConnectPtr conn, int usageType, const char *usageID) +{ + virSecretDriverStatePtr driver = conn->secretPrivateData; + virSecretPtr ret = NULL; + virSecretEntryPtr secret; + + secretDriverLock(driver); + + secret = secretFindByUsage(driver, usageType, usageID); + if (secret == NULL) { + virSecretReportError(conn, VIR_ERR_NO_SECRET, + _("no secret with matching usage '%s'"), usageID); + goto cleanup; + } + + ret = virGetSecret(conn, + secret->def->uuid, + secret->def->usage_type, + secretUsageIDForDef(secret->def)); + +cleanup: + secretDriverUnlock(driver); + return ret; +} + + +static virSecretPtr secretDefineXML(virConnectPtr conn, const char *xml, unsigned int flags ATTRIBUTE_UNUSED) { virSecretDriverStatePtr driver = conn->secretPrivateData; virSecretPtr ret = NULL; virSecretEntryPtr secret; - virSecretDefPtr backup, new_attrs; - bool secret_is_new; + virSecretDefPtr backup = NULL; + virSecretDefPtr new_attrs; new_attrs = virSecretDefParseString(conn, xml); if (new_attrs == NULL) @@ -656,28 +690,58 @@ secretDefineXML(virConnectPtr conn, const char *xml, secretDriverLock(driver); - secret = secretFindOrCreate(conn, driver, new_attrs->uuid, - &secret_is_new); - if (secret == NULL) - goto cleanup; + secret = secretFindByUUID(driver, new_attrs->uuid); + if (secret == NULL) { + /* No existing secret with same UUID, try look for matching usage instead */ + const char *usageID = secretUsageIDForDef(new_attrs); + secret = secretFindByUsage(driver, new_attrs->usage_type, usageID); + if (secret) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(secret->def->uuid, uuidstr); + virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("a secret with UUID %s already defined for use with %s"), + uuidstr, usageID); + goto cleanup; + } - /* Save old values of the attributes */ - backup = secret->def; + /* No existing secret at all, create one */ + if (VIR_ALLOC(secret) < 0) { + virReportOOMError(conn); + goto cleanup; + } - if (backup->private && !new_attrs->private) { - virSecretReportError(conn, VIR_ERR_OPERATION_DENIED, "%s", - virErrorMsg(VIR_ERR_OPERATION_DENIED, NULL)); - goto cleanup; + listInsert(&driver->secrets, secret); + secret->def = new_attrs; + } else { + const char *newUsageID = secretUsageIDForDef(new_attrs); + const char *oldUsageID = secretUsageIDForDef(secret->def); + if (STRNEQ(oldUsageID, newUsageID)) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(secret->def->uuid, uuidstr); + virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("a secret with UUID %s is already defined for use with %s"), + uuidstr, oldUsageID); + goto cleanup; + } + + if (secret->def->private && !new_attrs->private) { + virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot change private flag on existing secret")); + goto cleanup; + } + + /* Got an existing secret matches attrs, so reuse that */ + backup = secret->def; + secret->def = new_attrs; } - secret->def = new_attrs; if (!new_attrs->ephemeral) { - if (backup->ephemeral) { + if (backup && backup->ephemeral) { if (secretSaveValue(conn, driver, secret) < 0) goto restore_backup; } if (secretSaveDef(conn, driver, secret) < 0) { - if (backup->ephemeral) { + if (backup && backup->ephemeral) { char *filename; /* Undo the secretSaveValue() above; ignore errors */ @@ -688,7 +752,7 @@ secretDefineXML(virConnectPtr conn, const char *xml, } goto restore_backup; } - } else if (!backup->ephemeral) { + } else if (backup && !backup->ephemeral) { if (secretDeleteSaved(conn, driver, secret) < 0) goto restore_backup; } @@ -696,13 +760,17 @@ secretDefineXML(virConnectPtr conn, const char *xml, new_attrs = NULL; virSecretDefFree(backup); - ret = virGetSecret(conn, secret->def->uuid); + ret = virGetSecret(conn, + secret->def->uuid, + secret->def->usage_type, + secretUsageIDForDef(secret->def)); goto cleanup; restore_backup: - /* Error - restore previous state and free new attributes */ - secret->def = backup; - if (secret_is_new) { + if (backup) { + /* Error - restore previous state and free new attributes */ + secret->def = backup; + } else { /* "secret" was added to the head of the list above */ if (listUnlink(&driverState->secrets) != secret) virSecretReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", @@ -723,12 +791,12 @@ secretGetXMLDesc(virSecretPtr obj, unsigned int flags ATTRIBUTE_UNUSED) { virSecretDriverStatePtr driver = obj->conn->secretPrivateData; char *ret = NULL; - virSecretEntryPtr *pptr; + virSecretEntryPtr secret; secretDriverLock(driver); - pptr = secretFind(driver, obj->uuid); - if (pptr == NULL) { + secret = secretFindByUUID(driver, obj->uuid); + if (secret == NULL) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(obj->uuid, uuidstr); virSecretReportError(obj->conn, VIR_ERR_NO_SECRET, @@ -736,7 +804,7 @@ secretGetXMLDesc(virSecretPtr obj, unsigned int flags ATTRIBUTE_UNUSED) goto cleanup; } - ret = virSecretDefFormat(obj->conn, (*pptr)->def); + ret = virSecretDefFormat(obj->conn, secret->def); cleanup: secretDriverUnlock(driver); @@ -752,7 +820,7 @@ secretSetValue(virSecretPtr obj, const unsigned char *value, int ret = -1; unsigned char *old_value, *new_value; size_t old_value_size; - virSecretEntryPtr secret, *pptr; + virSecretEntryPtr secret; if (VIR_ALLOC_N(new_value, value_size) < 0) { virReportOOMError(obj->conn); @@ -761,15 +829,14 @@ secretSetValue(virSecretPtr obj, const unsigned char *value, secretDriverLock(driver); - pptr = secretFind(driver, obj->uuid); - if (pptr == NULL) { + secret = secretFindByUUID(driver, obj->uuid); + if (secret == NULL) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(obj->uuid, uuidstr); virSecretReportError(obj->conn, VIR_ERR_NO_SECRET, _("no secret with matching uuid '%s'"), uuidstr); goto cleanup; } - secret = *pptr; old_value = secret->value; old_value_size = secret->value_size; @@ -810,19 +877,19 @@ secretGetValue(virSecretPtr obj, size_t *value_size, unsigned int flags) { virSecretDriverStatePtr driver = obj->conn->secretPrivateData; unsigned char *ret = NULL; - virSecretEntryPtr *pptr, secret; + virSecretEntryPtr secret; secretDriverLock(driver); - pptr = secretFind(driver, obj->uuid); - if (pptr == NULL) { + secret = secretFindByUUID(driver, obj->uuid); + if (secret == NULL) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(obj->uuid, uuidstr); virSecretReportError(obj->conn, VIR_ERR_NO_SECRET, _("no secret with matching uuid '%s'"), uuidstr); goto cleanup; } - secret = *pptr; + if (secret->value == NULL) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(obj->uuid, uuidstr); @@ -856,12 +923,12 @@ secretUndefine(virSecretPtr obj) { virSecretDriverStatePtr driver = obj->conn->secretPrivateData; int ret = -1; - virSecretEntryPtr *pptr, secret; + virSecretEntryPtr secret; secretDriverLock(driver); - pptr = secretFind(driver, obj->uuid); - if (pptr == NULL) { + secret = secretFindByUUID(driver, obj->uuid); + if (secret == NULL) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(obj->uuid, uuidstr); virSecretReportError(obj->conn, VIR_ERR_NO_SECRET, @@ -869,19 +936,22 @@ secretUndefine(virSecretPtr obj) goto cleanup; } - secret = listUnlink(pptr); - if (!secret->def->ephemeral) { - if (secretDeleteSaved(obj->conn, driver, secret) < 0) - goto restore_backup; + if (!secret->def->ephemeral && + secretDeleteSaved(obj->conn, driver, secret) < 0) + goto cleanup; + + if (driver->secrets == secret) { + driver->secrets = secret->next; + } else { + virSecretEntryPtr tmp = driver->secrets; + while (tmp && tmp->next != secret) + tmp = tmp->next; + if (tmp) + tmp->next = secret->next; } secretFree(secret); ret = 0; - goto cleanup; - -restore_backup: - /* This may change the order of secrets in the list. We don't care. */ - listInsert(&driver->secrets, secret); cleanup: secretDriverUnlock(driver); @@ -1000,6 +1070,7 @@ static virSecretDriver secretDriver = { .numOfSecrets = secretNumOfSecrets, .listSecrets = secretListSecrets, .lookupByUUID = secretLookupByUUID, + .lookupByUsage = secretLookupByUsage, .defineXML = secretDefineXML, .getXMLDesc = secretGetXMLDesc, .setValue = secretSetValue, diff --git a/src/virsh.c b/src/virsh.c index 74147da..4825f1c 100644 --- a/src/virsh.c +++ b/src/virsh.c @@ -5527,11 +5527,33 @@ cmdSecretList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) qsort(uuids, maxuuids, sizeof(char *), namesorter); - vshPrintExtra(ctl, "%s\n", _("UUID")); - vshPrintExtra(ctl, "-----------------------------------------\n"); + vshPrintExtra(ctl, "%-36s %s\n", _("UUID"), _("Usage")); + vshPrintExtra(ctl, "-----------------------------------------------------------\n"); for (i = 0; i < maxuuids; i++) { - vshPrint(ctl, "%-36s\n", uuids[i]); + virSecretPtr sec = virSecretLookupByUUIDString(ctl->conn, uuids[i]); + const char *usageType = NULL; + + if (!sec) { + free(uuids[i]); + continue; + } + + switch (virSecretGetUsageType(sec)) { + case VIR_SECRET_USAGE_TYPE_VOLUME: + usageType = _("Volume"); + break; + } + + if (usageType) { + vshPrint(ctl, "%-36s %s %s\n", + uuids[i], usageType, + virSecretGetUsageID(sec)); + } else { + vshPrint(ctl, "%-36s %s\n", + uuids[i], _("Unused")); + } + virSecretFree(sec); free(uuids[i]); } free(uuids); -- 1.6.2.5
On Fri, Sep 11, 2009 at 03:19:18PM +0100, Daniel P. Berrange wrote:
* include/libvirt/libvirt.h, include/libvirt/libvirt.h.in: Add virSecretGetUsageType, virSecretGetUsageID and virLookupSecretByUsage * python/generator.py: Mark virSecretGetUsageType, virSecretGetUsageID as not throwing exceptions * qemud/remote.c: Implement dispatch for virLookupSecretByUsage * qemud/remote_protocol.x: Add usage type & ID as attributes of remote_nonnull_secret. Add RPC calls for new public APIs * qemud/remote_dispatch_args.h, qemud/remote_dispatch_prototypes.h, qemud/remote_dispatch_ret.h, qemud/remote_dispatch_table.h, qemud/remote_protocol.c, qemud/remote_protocol.h: Re-generate * src/datatypes.c, src/datatypes.h: Add usageType and usageID as properties of virSecretPtr * src/driver.h: Add virLookupSecretByUsage driver entry point * src/libvirt.c: Implement virSecretGetUsageType, virSecretGetUsageID and virLookupSecretByUsage * src/libvirt_public.syms: Export virSecretGetUsageType, virSecretGetUsageID and virLookupSecretByUsage * src/remote_internal.c: Implement virLookupSecretByUsage entry * src/secret_conf.c, src/secret_conf.h: Remove the virSecretUsageType enum, now in public API. Make volume path mandatory when parsing XML * src/secret_driver.c: Enforce usage uniqueness when defining secrets. Implement virSecretLookupByUsage api method * src/virsh.c: Include usage for secret-list command [...] /** + * virSecretLookupByUsage: + * @conn: pointer to the hypervisor connection + * @usageType: the type of secret usage + * @usageID: identifier of the object using the secret + * + * Try to lookup a secret on the given hypervisor based on its usage + * + * Returns a new secret object or NULL in case of failure. If the + * secret cannot be found, then VIR_ERR_NO_SECRET error is raised. + */ +virSecretPtr +virSecretLookupByUsage(virConnectPtr conn, + int usageType, + const char *usageID) +{ + DEBUG("conn=%p, usageType=%d usageID=%s", conn, usageType, NULLSTR(usageID)); + + virResetLastError(); + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__); + return (NULL); + } + if (usageID == NULL) { + virLibConnError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + if (conn->secretDriver && + conn->secretDriver->lookupByUsage) { + virSecretPtr ret; + ret = conn->secretDriver->lookupByUsage (conn, usageType, usageID); + if (!ret) + goto error; + return ret; + } + + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + /* Copy to connection error object for back compatability */ + virSetConnError(conn); + return NULL; +} + + +/** * virSecretDefineXML: * @conn: virConnect connection * @xml: XML describing the secret. @@ -9036,6 +9083,53 @@ error: return -1; }
+/** + * virSecretGetUsageType: + * @secret: a secret object + * + * Get the type of object which uses this secret + * + * Returns a positive integer identifying the type of object, + * or -1 upon error. + */ +int +virSecretGetUsageType(virSecretPtr secret) +{ + DEBUG("secret=%p", secret); + + virResetLastError(); + + if (!VIR_IS_SECRET(secret)) { + virLibSecretError(NULL, VIR_ERR_INVALID_SECRET, __FUNCTION__); + return (-1); + } + return (secret->usageType); +} + +/** + * virSecretGetUsageID: + * @secret: a secret object + * + * Get the unique identifier of the object with which this + * secret is to be used + * + * Returns a string identifying the object using the secret, + * or NULL upon error + */ +const char * +virSecretGetUsageID(virSecretPtr secret) +{ + DEBUG("secret=%p", secret); + + virResetLastError(); + + if (!VIR_IS_SECRET(secret)) { + virLibSecretError(NULL, VIR_ERR_INVALID_SECRET, __FUNCTION__); + return (NULL); + } + return (secret->usageID); +} +
Looking from the outside I find that hard to graps especially the last one. virSecretGetUsageID return value supposed to be an UUID , the comment let the user expect this but it seems to be actually free form. I'm not against the patch but I think this needs some example or improved comments to really set properly the mechanism and expectations for the user. This may come after as a set of documentations, but if the function description could be clarified a bit this would be nice. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Fri, Sep 11, 2009 at 06:17:40PM +0200, Daniel Veillard wrote:
On Fri, Sep 11, 2009 at 03:19:18PM +0100, Daniel P. Berrange wrote:
+/** + * virSecretGetUsageType: + * @secret: a secret object + * + * Get the type of object which uses this secret + * + * Returns a positive integer identifying the type of object, + * or -1 upon error. + */ +int +virSecretGetUsageType(virSecretPtr secret) +{ + DEBUG("secret=%p", secret); + + virResetLastError(); + + if (!VIR_IS_SECRET(secret)) { + virLibSecretError(NULL, VIR_ERR_INVALID_SECRET, __FUNCTION__); + return (-1); + } + return (secret->usageType); +} + +/** + * virSecretGetUsageID: + * @secret: a secret object + * + * Get the unique identifier of the object with which this + * secret is to be used + * + * Returns a string identifying the object using the secret, + * or NULL upon error + */ +const char * +virSecretGetUsageID(virSecretPtr secret) +{ + DEBUG("secret=%p", secret); + + virResetLastError(); + + if (!VIR_IS_SECRET(secret)) { + virLibSecretError(NULL, VIR_ERR_INVALID_SECRET, __FUNCTION__); + return (NULL); + } + return (secret->usageID); +} +
Looking from the outside I find that hard to graps especially the last one. virSecretGetUsageID return value supposed to be an UUID , the comment let the user expect this but it seems to be actually free form.
I'm not against the patch but I think this needs some example or improved comments to really set properly the mechanism and expectations for the user. This may come after as a set of documentations, but if the function description could be clarified a bit this would be nice.
I'll expand the API docs for this method. It is not returning a UUID. I named it 'ID' to imply a arbitrary type of unique identifier since the format of the identifier returned by 'virSecretGetUsageID' will vary depending on what 'virSecretGetUsageType' shows. For a usage type of VIR_SECRET_USAGE_TYPE_VOLUME, the identifier will be the full volume file path, eg /var/lib/libvirt/images/encrypted1.img As and when we add new usage types (eg perhaps for VNC passwords, or SPICE credentials), then we'll define appropriate formats for their identifiers. As an example, with virsh, using the virSecretGetUsageID method, it will now show $ virsh secret-list UUID Usage ----------------------------------------------------------- 0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f Volume /home/berrange/VirtualMachines/encrypted2.img 80cfa88e-baa3-3e6f-a2aa-12d0576bc7c5 Volume /foo/bar e3a9758f-b0c6-7a3a-ebb9-71a69c930289 Volume /home/berrange/VirtualMachines/encrypted1.img e3b61a0f-714a-0b62-78c2-81eeb1d7d7a5 Unused The path bit of the usage there comes directly from the virSecretGetUsageType method, and also happens to match te <usage><volume>..</volume></usage> XML element contents. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Fri, Sep 11, 2009 at 06:31:02PM +0100, Daniel P. Berrange wrote:
On Fri, Sep 11, 2009 at 06:17:40PM +0200, Daniel Veillard wrote:
On Fri, Sep 11, 2009 at 03:19:18PM +0100, Daniel P. Berrange wrote:
+/** + * virSecretGetUsageType: + * @secret: a secret object + * + * Get the type of object which uses this secret + * + * Returns a positive integer identifying the type of object, + * or -1 upon error. + */ +int +virSecretGetUsageType(virSecretPtr secret) +{ + DEBUG("secret=%p", secret); + + virResetLastError(); + + if (!VIR_IS_SECRET(secret)) { + virLibSecretError(NULL, VIR_ERR_INVALID_SECRET, __FUNCTION__); + return (-1); + } + return (secret->usageType); +} + +/** + * virSecretGetUsageID: + * @secret: a secret object + * + * Get the unique identifier of the object with which this + * secret is to be used + * + * Returns a string identifying the object using the secret, + * or NULL upon error + */ +const char * +virSecretGetUsageID(virSecretPtr secret) +{ + DEBUG("secret=%p", secret); + + virResetLastError(); + + if (!VIR_IS_SECRET(secret)) { + virLibSecretError(NULL, VIR_ERR_INVALID_SECRET, __FUNCTION__); + return (NULL); + } + return (secret->usageID); +} +
Looking from the outside I find that hard to graps especially the last one. virSecretGetUsageID return value supposed to be an UUID , the comment let the user expect this but it seems to be actually free form.
I'm not against the patch but I think this needs some example or improved comments to really set properly the mechanism and expectations for the user. This may come after as a set of documentations, but if the function description could be clarified a bit this would be nice.
I'll expand the API docs for this method. It is not returning a UUID. I named it 'ID' to imply a arbitrary type of unique identifier since the format of the identifier returned by 'virSecretGetUsageID' will vary depending on what 'virSecretGetUsageType' shows. For a usage type of VIR_SECRET_USAGE_TYPE_VOLUME, the identifier will be the full volume file path, eg /var/lib/libvirt/images/encrypted1.img
As and when we add new usage types (eg perhaps for VNC passwords, or SPICE credentials), then we'll define appropriate formats for their identifiers.
As an example, with virsh, using the virSecretGetUsageID method, it will now show
$ virsh secret-list UUID Usage ----------------------------------------------------------- 0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f Volume /home/berrange/VirtualMachines/encrypted2.img 80cfa88e-baa3-3e6f-a2aa-12d0576bc7c5 Volume /foo/bar e3a9758f-b0c6-7a3a-ebb9-71a69c930289 Volume /home/berrange/VirtualMachines/encrypted1.img e3b61a0f-714a-0b62-78c2-81eeb1d7d7a5 Unused
The path bit of the usage there comes directly from the virSecretGetUsageType method, and also happens to match te <usage><volume>..</volume></usage> XML element contents.
Okay, that clears things up ! ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
* src/storage_backend_fs.c: Lookup & fill in secret passphrase UUID for storage volumes using encryption --- src/storage_backend_fs.c | 41 ++++++++++++++++++++++++++++++++++++++++- 1 files changed, 40 insertions(+), 1 deletions(-) diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c index 5ff0ed8..01cb171 100644 --- a/src/storage_backend_fs.c +++ b/src/storage_backend_fs.c @@ -430,6 +430,11 @@ static int virStorageBackendProbeTarget(virConnectPtr conn, } enc->format = VIR_STORAGE_ENCRYPTION_FORMAT_QCOW; *encryption = enc; + /* XXX ideally we'd fill in secret UUID here + * but we cannot guarentee 'conn' is non-NULL + * at this point in time :-( So we only fill + * in secrets when someone first queries a vol + */ } return 0; } @@ -1230,8 +1235,42 @@ virStorageBackendFileSystemVolRefresh(virConnectPtr conn, virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virStorageVolDefPtr vol) { + int ret; + /* Refresh allocation / permissions info in case its changed */ - return virStorageBackendUpdateVolInfo(conn, vol, 0); + ret = virStorageBackendUpdateVolInfo(conn, vol, 0); + if (ret < 0) + return ret; + + /* Load any secrets if posible */ + if (vol->target.encryption && + vol->target.encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_QCOW && + vol->target.encryption->nsecrets == 0) { + virSecretPtr sec; + virStorageEncryptionSecretPtr encsec = NULL; + + sec = virSecretLookupByUsage(conn, + VIR_SECRET_USAGE_TYPE_VOLUME, + vol->target.path); + if (sec) { + if (VIR_ALLOC_N(vol->target.encryption->secrets, 1) < 0 || + VIR_ALLOC(encsec) < 0) { + VIR_FREE(vol->target.encryption->secrets); + virReportOOMError(conn); + virSecretFree(sec); + return -1; + } + + vol->target.encryption->nsecrets = 1; + vol->target.encryption->secrets[0] = encsec; + + encsec->type = VIR_STORAGE_ENCRYPTION_SECRET_TYPE_PASSPHRASE; + virSecretGetUUID(sec, encsec->uuid); + virSecretFree(sec); + } + } + + return 0; } virStorageBackend virStorageBackendDirectory = { -- 1.6.2.5
On Fri, Sep 11, 2009 at 03:19:19PM +0100, Daniel P. Berrange wrote:
* src/storage_backend_fs.c: Lookup & fill in secret passphrase UUID for storage volumes using encryption --- src/storage_backend_fs.c | 41 ++++++++++++++++++++++++++++++++++++++++- 1 files changed, 40 insertions(+), 1 deletions(-)
diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c index 5ff0ed8..01cb171 100644 --- a/src/storage_backend_fs.c +++ b/src/storage_backend_fs.c @@ -430,6 +430,11 @@ static int virStorageBackendProbeTarget(virConnectPtr conn, } enc->format = VIR_STORAGE_ENCRYPTION_FORMAT_QCOW; *encryption = enc; + /* XXX ideally we'd fill in secret UUID here + * but we cannot guarentee 'conn' is non-NULL + * at this point in time :-( So we only fill + * in secrets when someone first queries a vol + */ } return 0; } @@ -1230,8 +1235,42 @@ virStorageBackendFileSystemVolRefresh(virConnectPtr conn, virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virStorageVolDefPtr vol) { + int ret; + /* Refresh allocation / permissions info in case its changed */ - return virStorageBackendUpdateVolInfo(conn, vol, 0); + ret = virStorageBackendUpdateVolInfo(conn, vol, 0); + if (ret < 0) + return ret; + + /* Load any secrets if posible */ + if (vol->target.encryption && + vol->target.encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_QCOW && + vol->target.encryption->nsecrets == 0) { + virSecretPtr sec; + virStorageEncryptionSecretPtr encsec = NULL; + + sec = virSecretLookupByUsage(conn, + VIR_SECRET_USAGE_TYPE_VOLUME, + vol->target.path); + if (sec) { + if (VIR_ALLOC_N(vol->target.encryption->secrets, 1) < 0 || + VIR_ALLOC(encsec) < 0) { + VIR_FREE(vol->target.encryption->secrets); + virReportOOMError(conn); + virSecretFree(sec); + return -1; + } + + vol->target.encryption->nsecrets = 1; + vol->target.encryption->secrets[0] = encsec; + + encsec->type = VIR_STORAGE_ENCRYPTION_SECRET_TYPE_PASSPHRASE; + virSecretGetUUID(sec, encsec->uuid); + virSecretFree(sec); + } + } + + return 0; }
virStorageBackend virStorageBackendDirectory = {
ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
* docs/schemas/secret.rng: Require volume element to be an absolute path. Fix whitespace indentation --- docs/schemas/secret.rng | 68 +++++++++++++++++++++++++--------------------- 1 files changed, 37 insertions(+), 31 deletions(-) diff --git a/docs/schemas/secret.rng b/docs/schemas/secret.rng index 2aab1db..40e2b7f 100644 --- a/docs/schemas/secret.rng +++ b/docs/schemas/secret.rng @@ -7,40 +7,40 @@ <define name='secret'> <element name='secret'> <optional> - <attribute name='ephemeral'> - <choice> - <value>yes</value> - <value>no</value> - </choice> - </attribute> + <attribute name='ephemeral'> + <choice> + <value>yes</value> + <value>no</value> + </choice> + </attribute> </optional> <optional> - <attribute name='private'> - <choice> - <value>yes</value> - <value>no</value> - </choice> - </attribute> + <attribute name='private'> + <choice> + <value>yes</value> + <value>no</value> + </choice> + </attribute> </optional> <interleave> - <optional> - <element name='uuid'> - <ref name='UUID'/> - </element> - </optional> - <optional> - <element name='description'> - <text/> - </element> - </optional> - <optional> - <element name='usage'> - <choice> - <ref name='usagevolume'> - </choice> - <text/> - </element> - </optional> + <optional> + <element name='uuid'> + <ref name='UUID'/> + </element> + </optional> + <optional> + <element name='description'> + <text/> + </element> + </optional> + <optional> + <element name='usage'> + <choice> + <ref name='usagevolume'> + <!-- More choices later --> + </choice> + </element> + </optional> </interleave> </element> </define> @@ -50,7 +50,7 @@ <value>volume</value> </attribute> <element name='volume'> - <text/> + <ref name='absFilePath'/> </element> </define> @@ -65,4 +65,10 @@ </choice> </define> + <define name="absFilePath"> + <data type="string"> + <param name="pattern">/[a-zA-Z0-9_\.\+\-&/%]+</param> + </data> + </define> + </grammar> -- 1.6.2.5
On Fri, Sep 11, 2009 at 03:19:20PM +0100, Daniel P. Berrange wrote:
* docs/schemas/secret.rng: Require volume element to be an absolute path. Fix whitespace indentation --- docs/schemas/secret.rng | 68 +++++++++++++++++++++++++--------------------- 1 files changed, 37 insertions(+), 31 deletions(-)
diff --git a/docs/schemas/secret.rng b/docs/schemas/secret.rng index 2aab1db..40e2b7f 100644 --- a/docs/schemas/secret.rng +++ b/docs/schemas/secret.rng @@ -7,40 +7,40 @@ <define name='secret'> <element name='secret'> <optional> - <attribute name='ephemeral'> - <choice> - <value>yes</value> - <value>no</value> - </choice> - </attribute> + <attribute name='ephemeral'> + <choice> + <value>yes</value> + <value>no</value> + </choice> + </attribute> </optional> <optional> - <attribute name='private'> - <choice> - <value>yes</value> - <value>no</value> - </choice> - </attribute> + <attribute name='private'> + <choice> + <value>yes</value> + <value>no</value> + </choice> + </attribute> </optional> <interleave> - <optional> - <element name='uuid'> - <ref name='UUID'/> - </element> - </optional> - <optional> - <element name='description'> - <text/> - </element> - </optional> - <optional> - <element name='usage'> - <choice> - <ref name='usagevolume'> - </choice> - <text/> - </element> - </optional> + <optional> + <element name='uuid'> + <ref name='UUID'/> + </element> + </optional> + <optional> + <element name='description'> + <text/> + </element> + </optional> + <optional> + <element name='usage'> + <choice> + <ref name='usagevolume'> + <!-- More choices later --> + </choice> + </element> + </optional> </interleave> </element> </define> @@ -50,7 +50,7 @@ <value>volume</value> </attribute> <element name='volume'> - <text/> + <ref name='absFilePath'/> </element> </define>
@@ -65,4 +65,10 @@ </choice> </define>
+ <define name="absFilePath"> + <data type="string"> + <param name="pattern">/[a-zA-Z0-9_\.\+\-&/%]+</param> + </data> + </define> + </grammar>
ACK thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Miloslav Trmac