[libvirt] [PATCH v2 0/6] Move secret object command line building helpers
v1: http://www.redhat.com/archives/libvir-list/2016-May/msg02017.html Changes in v2: Patch 1 - Move function to a new src/util/virqemu.c with adjustments to other affected areas Patches 2-4 - Similarly adjust the flow/name of the patches with an adjustment so that patch 2 is just the new code in virqemu.c and patch 3 shows how the master secret code would use the same function to build it's command line. Patch 4 just follows suit... Patch 5 was reviewed/ACK'd - this patch just shows the changes to remove the "virSecretPtr secret = NULL;" and "virObjectUnref(secret);" Patch 6 is unchanged John Ferlan (6): qemu: Move and rename qemuBuildObjectCommandlineFromJSON util: Introduce virQEMUBuildSecretObjectProps qemu: Use virQEMUBuildSecretObjectProps to build secret objects qemu: Rework secinfo command line building storage: Use virSecretGetSecretString secret: Move virStorageSecretType to secret_util and rename cfg.mk | 2 +- po/POTFILES.in | 1 + src/Makefile.am | 2 + src/libvirt_private.syms | 5 + src/libxl/libxl_conf.c | 2 +- src/qemu/qemu_command.c | 219 +++++---------------- src/qemu/qemu_command.h | 4 - src/qemu/qemu_domain.c | 4 +- src/secret/secret_util.c | 18 +- src/secret/secret_util.h | 22 ++- src/storage/storage_backend_iscsi.c | 55 +----- src/storage/storage_backend_rbd.c | 49 +---- src/util/virqemu.c | 211 ++++++++++++++++++++ src/util/virqemu.h | 42 ++++ src/util/virstoragefile.c | 33 ++-- src/util/virstoragefile.h | 17 +- tests/qemuargv2xmltest.c | 4 +- tests/qemucommandutiltest.c | 9 +- ...muxml2argv-disk-drive-network-rbd-auth-AES.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-master-key.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-name-escape.args | 3 +- 21 files changed, 380 insertions(+), 328 deletions(-) create mode 100644 src/util/virqemu.c create mode 100644 src/util/virqemu.h -- 2.5.5
Move the module from qemu_command.c to a new module virqemu.c and rename the API to virQEMUBuildObjectCommandline. This API will then be shareable with qemu-img and the need to build a security object for luks support. Signed-off-by: John Ferlan <jferlan@redhat.com> --- po/POTFILES.in | 1 + src/Makefile.am | 1 + src/libvirt_private.syms | 4 ++ src/qemu/qemu_command.c | 126 +++------------------------------------ src/qemu/qemu_command.h | 4 -- src/util/virqemu.c | 142 ++++++++++++++++++++++++++++++++++++++++++++ src/util/virqemu.h | 34 +++++++++++ tests/qemucommandutiltest.c | 9 ++- 8 files changed, 195 insertions(+), 126 deletions(-) create mode 100644 src/util/virqemu.c create mode 100644 src/util/virqemu.h diff --git a/po/POTFILES.in b/po/POTFILES.in index 0d92448..2a6fae4 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -225,6 +225,7 @@ src/util/virpidfile.c src/util/virpolkit.c src/util/virportallocator.c src/util/virprocess.c +src/util/virqemu.c src/util/virrandom.c src/util/virrotatingfile.c src/util/virscsi.c diff --git a/src/Makefile.am b/src/Makefile.am index 12b66c2..f3c9a14 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -153,6 +153,7 @@ UTIL_SOURCES = \ util/virportallocator.c util/virportallocator.h \ util/virprobe.h \ util/virprocess.c util/virprocess.h \ + util/virqemu.c util/virqemu.h \ util/virrandom.h util/virrandom.c \ util/virrotatingfile.h util/virrotatingfile.c \ util/virscsi.c util/virscsi.h \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e325168..d25baae 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2142,6 +2142,10 @@ virProcessTranslateStatus; virProcessWait; +# util/virqemu.h +virQEMUBuildObjectCommandlineFromJSON; + + # util/virrandom.h virRandom; virRandomBits; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 368bd87..1455c0d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -37,6 +37,7 @@ #include "virfile.h" #include "virnetdev.h" #include "virnetdevbridge.h" +#include "virqemu.h" #include "virstring.h" #include "virtime.h" #include "viruuid.h" @@ -286,116 +287,6 @@ qemuVirCommandGetDevSet(virCommandPtr cmd, int fd) } -static int -qemuBuildObjectCommandLinePropsInternal(const char *key, - const virJSONValue *value, - virBufferPtr buf, - bool nested) -{ - virJSONValuePtr elem; - virBitmapPtr bitmap = NULL; - ssize_t pos = -1; - ssize_t end; - size_t i; - - switch ((virJSONType) value->type) { - case VIR_JSON_TYPE_STRING: - virBufferAsprintf(buf, ",%s=%s", key, value->data.string); - break; - - case VIR_JSON_TYPE_NUMBER: - virBufferAsprintf(buf, ",%s=%s", key, value->data.number); - break; - - case VIR_JSON_TYPE_BOOLEAN: - if (value->data.boolean) - virBufferAsprintf(buf, ",%s=yes", key); - else - virBufferAsprintf(buf, ",%s=no", key); - - break; - - case VIR_JSON_TYPE_ARRAY: - if (nested) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("nested -object property arrays are not supported")); - return -1; - } - - if (virJSONValueGetArrayAsBitmap(value, &bitmap) == 0) { - while ((pos = virBitmapNextSetBit(bitmap, pos)) > -1) { - if ((end = virBitmapNextClearBit(bitmap, pos)) < 0) - end = virBitmapLastSetBit(bitmap) + 1; - - if (end - 1 > pos) { - virBufferAsprintf(buf, ",%s=%zd-%zd", key, pos, end - 1); - pos = end; - } else { - virBufferAsprintf(buf, ",%s=%zd", key, pos); - } - } - } else { - /* fallback, treat the array as a non-bitmap, adding the key - * for each member */ - for (i = 0; i < virJSONValueArraySize(value); i++) { - elem = virJSONValueArrayGet((virJSONValuePtr)value, i); - - /* recurse to avoid duplicating code */ - if (qemuBuildObjectCommandLinePropsInternal(key, elem, buf, - true) < 0) - return -1; - } - } - break; - - case VIR_JSON_TYPE_OBJECT: - case VIR_JSON_TYPE_NULL: - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("NULL and OBJECT JSON types can't be converted to " - "commandline string")); - return -1; - } - - virBitmapFree(bitmap); - return 0; -} - - -static int -qemuBuildObjectCommandLineProps(const char *key, - const virJSONValue *value, - void *opaque) -{ - return qemuBuildObjectCommandLinePropsInternal(key, value, opaque, false); -} - - -char * -qemuBuildObjectCommandlineFromJSON(const char *type, - const char *alias, - virJSONValuePtr props) -{ - virBuffer buf = VIR_BUFFER_INITIALIZER; - char *ret = NULL; - - virBufferAsprintf(&buf, "%s,id=%s", type, alias); - - if (virJSONValueObjectForeachKeyValue(props, - qemuBuildObjectCommandLineProps, - &buf) < 0) - goto cleanup; - - if (virBufferCheckError(&buf) < 0) - goto cleanup; - - ret = virBufferContentAndReset(&buf); - - cleanup: - virBufferFreeAndReset(&buf); - return ret; -} - - char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk) { char *ret; @@ -680,8 +571,9 @@ qemuBuildObjectSecretCommandLine(virCommandPtr cmd, if (qemuBuildSecretInfoProps(secinfo, &type, &props) < 0) return -1; - if (!(tmp = qemuBuildObjectCommandlineFromJSON(type, secinfo->s.aes.alias, - props))) + if (!(tmp = virQEMUBuildObjectCommandlineFromJSON(type, + secinfo->s.aes.alias, + props))) goto cleanup; virCommandAddArgList(cmd, "-object", tmp, NULL); @@ -3226,9 +3118,9 @@ qemuBuildMemoryCellBackendStr(virDomainDefPtr def, &props, false)) < 0) goto cleanup; - if (!(*backendStr = qemuBuildObjectCommandlineFromJSON(backendType, - alias, - props))) + if (!(*backendStr = virQEMUBuildObjectCommandlineFromJSON(backendType, + alias, + props))) goto cleanup; ret = rc; @@ -3268,7 +3160,7 @@ qemuBuildMemoryDimmBackendStr(virDomainMemoryDefPtr mem, &backendType, &props, true) < 0) goto cleanup; - ret = qemuBuildObjectCommandlineFromJSON(backendType, alias, props); + ret = virQEMUBuildObjectCommandlineFromJSON(backendType, alias, props); cleanup: VIR_FREE(alias); @@ -5411,7 +5303,7 @@ qemuBuildRNGBackendStr(virDomainRNGDefPtr rng, if (qemuBuildRNGBackendProps(rng, qemuCaps, &type, &props) < 0) goto cleanup; - ret = qemuBuildObjectCommandlineFromJSON(type, alias, props); + ret = virQEMUBuildObjectCommandlineFromJSON(type, alias, props); cleanup: VIR_FREE(alias); diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 1c22705..d2b7fa7 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -44,10 +44,6 @@ VIR_ENUM_DECL(qemuVideo) -char *qemuBuildObjectCommandlineFromJSON(const char *type, - const char *alias, - virJSONValuePtr props); - virCommandPtr qemuBuildCommandLine(virQEMUDriverPtr driver, virLogManagerPtr logManager, virDomainDefPtr def, diff --git a/src/util/virqemu.c b/src/util/virqemu.c new file mode 100644 index 0000000..f87e20b --- /dev/null +++ b/src/util/virqemu.c @@ -0,0 +1,142 @@ +/* + * virqemu.c: QEMU object parsing/formatting + * + * Copyright (C) 2016 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + */ + + +#include <config.h> + +#include "virbuffer.h" +#include "virerror.h" +#include "virlog.h" +#include "virqemu.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +VIR_LOG_INIT("util.qemu"); + + +static int +virQEMUBuildObjectCommandLinePropsInternal(const char *key, + const virJSONValue *value, + virBufferPtr buf, + bool nested) +{ + virJSONValuePtr elem; + virBitmapPtr bitmap = NULL; + ssize_t pos = -1; + ssize_t end; + size_t i; + + switch ((virJSONType) value->type) { + case VIR_JSON_TYPE_STRING: + virBufferAsprintf(buf, ",%s=%s", key, value->data.string); + break; + + case VIR_JSON_TYPE_NUMBER: + virBufferAsprintf(buf, ",%s=%s", key, value->data.number); + break; + + case VIR_JSON_TYPE_BOOLEAN: + if (value->data.boolean) + virBufferAsprintf(buf, ",%s=yes", key); + else + virBufferAsprintf(buf, ",%s=no", key); + + break; + + case VIR_JSON_TYPE_ARRAY: + if (nested) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("nested -object property arrays are not supported")); + return -1; + } + + if (virJSONValueGetArrayAsBitmap(value, &bitmap) == 0) { + while ((pos = virBitmapNextSetBit(bitmap, pos)) > -1) { + if ((end = virBitmapNextClearBit(bitmap, pos)) < 0) + end = virBitmapLastSetBit(bitmap) + 1; + + if (end - 1 > pos) { + virBufferAsprintf(buf, ",%s=%zd-%zd", key, pos, end - 1); + pos = end; + } else { + virBufferAsprintf(buf, ",%s=%zd", key, pos); + } + } + } else { + /* fallback, treat the array as a non-bitmap, adding the key + * for each member */ + for (i = 0; i < virJSONValueArraySize(value); i++) { + elem = virJSONValueArrayGet((virJSONValuePtr)value, i); + + /* recurse to avoid duplicating code */ + if (virQEMUBuildObjectCommandLinePropsInternal(key, elem, buf, + true) < 0) + return -1; + } + } + break; + + case VIR_JSON_TYPE_OBJECT: + case VIR_JSON_TYPE_NULL: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("NULL and OBJECT JSON types can't be converted to " + "commandline string")); + return -1; + } + + virBitmapFree(bitmap); + return 0; +} + + +static int +virQEMUBuildObjectCommandLineProps(const char *key, + const virJSONValue *value, + void *opaque) +{ + return virQEMUBuildObjectCommandLinePropsInternal(key, value, opaque, false); +} + + +char * +virQEMUBuildObjectCommandlineFromJSON(const char *type, + const char *alias, + virJSONValuePtr props) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + char *ret = NULL; + + virBufferAsprintf(&buf, "%s,id=%s", type, alias); + + if (virJSONValueObjectForeachKeyValue(props, + virQEMUBuildObjectCommandLineProps, + &buf) < 0) + goto cleanup; + + if (virBufferCheckError(&buf) < 0) + goto cleanup; + + ret = virBufferContentAndReset(&buf); + + cleanup: + virBufferFreeAndReset(&buf); + return ret; +} diff --git a/src/util/virqemu.h b/src/util/virqemu.h new file mode 100644 index 0000000..0a72202 --- /dev/null +++ b/src/util/virqemu.h @@ -0,0 +1,34 @@ +/* + * virqemu.h: qemu specific object parsing/formatting + * + * Copyright (C) 2009, 2012-2016 Red Hat, Inc. + * Copyright (C) 2009 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + */ + + +#ifndef __VIR_QEMU_H_ +# define __VIR_QEMU_H_ + +# include "internal.h" +# include "virjson.h" + +char *virQEMUBuildObjectCommandlineFromJSON(const char *type, + const char *alias, + virJSONValuePtr props); + +#endif /* __VIR_QEMU_H_ */ diff --git a/tests/qemucommandutiltest.c b/tests/qemucommandutiltest.c index bd457f8..5072a62 100644 --- a/tests/qemucommandutiltest.c +++ b/tests/qemucommandutiltest.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2015 Red Hat, Inc. + * Copyright (C) 2015-2016 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -19,8 +19,8 @@ #include <config.h> -#include "qemu/qemu_command.h" #include "util/virjson.h" +#include "util/virqemu.h" #include "testutils.h" #include "testutilsqemu.h" @@ -51,9 +51,8 @@ testQemuCommandBuildObjectFromJSON(const void *opaque) data->expectprops ? data->expectprops : "") < 0) return -1; - result = qemuBuildObjectCommandlineFromJSON("testobject", - "testalias", - val); + result = virQEMUBuildObjectCommandlineFromJSON("testobject", + "testalias", val); if (STRNEQ_NULLABLE(expect, result)) { fprintf(stderr, "\nFailed to create object string. " -- 2.5.5
A common way to build a qemu secret object to be used by qemu_command.c in the short term and a bit longer term by storage_backend.c for qemu-img. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virqemu.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virqemu.h | 8 ++++++ 3 files changed, 78 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d25baae..e46172b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2144,6 +2144,7 @@ virProcessWait; # util/virqemu.h virQEMUBuildObjectCommandlineFromJSON; +virQEMUBuildSecretObjectProps; # util/virrandom.h diff --git a/src/util/virqemu.c b/src/util/virqemu.c index f87e20b..243fcbe 100644 --- a/src/util/virqemu.c +++ b/src/util/virqemu.c @@ -140,3 +140,72 @@ virQEMUBuildObjectCommandlineFromJSON(const char *type, virBufferFreeAndReset(&buf); return ret; } + + +/** + * virQEMUBuildSecretObjectProps + * @data: Pointer to data string + * @isfile: Boolean to indicate whether data is raw data or a filepath string + * @fmt: Format for the data/file (may be NULL) + * @keyid: Master key alias id (may be NULL) + * @iv: Initialization vector (may be NULL) + * @propsret: location to store the created/built property object + * + * There's many ways to build a secret object for qemu depending on need, + * + * -object secret,id=$alias,data=$data,format=base64 + * -object secret,id=$alias,file=$file[,format=base64] + * -object secret,id=$alias,data=$data,keyid=$keyid,[iv=$iv],format=base64 + * + * When a keyid and/or iv are provided, they are assumed to be base64 encoded + * + * Build the JSON object property thusly and return + * + * Returns 0 on success, -1 on failure w/ error set + */ +int +virQEMUBuildSecretObjectProps(const char *data, + bool isfile, + const char *fmt, + const char *keyid, + const char *iv, + virJSONValuePtr *propsret) +{ + /* Don't allow a construct such as: + * -object secret,id=$alias,data=$data + * It could provide a raw, text secret on the command line + */ + if (!isfile && STREQ_NULLABLE(fmt, "raw")) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot provide a raw data secret")); + return -1; + } + + if (!(*propsret = virJSONValueNewObject())) + return -1; + + if (isfile) { + if (virJSONValueObjectAdd(*propsret, "s:file", data, NULL) < 0) + goto error; + } else { + if (virJSONValueObjectAdd(*propsret, "s:data", data, NULL) < 0) + goto error; + } + + if (keyid && virJSONValueObjectAdd(*propsret, "s:keyid", keyid, NULL) < 0) + goto error; + + if (iv && virJSONValueObjectAdd(*propsret, "s:iv", iv, NULL) < 0) + goto error; + + /* NB: QEMU will assume "raw" when fmt not provided! */ + if (fmt && virJSONValueObjectAdd(*propsret, "s:format", fmt, NULL) < 0) + goto error; + + return 0; + + error: + virJSONValueFree(*propsret); + + return -1; +} diff --git a/src/util/virqemu.h b/src/util/virqemu.h index 0a72202..dedde3c 100644 --- a/src/util/virqemu.h +++ b/src/util/virqemu.h @@ -31,4 +31,12 @@ char *virQEMUBuildObjectCommandlineFromJSON(const char *type, const char *alias, virJSONValuePtr props); +int virQEMUBuildSecretObjectProps(const char *data, + bool isfile, + const char *fmt, + const char *keyid, + const char *iv, + virJSONValuePtr *propsret) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; + #endif /* __VIR_QEMU_H_ */ -- 2.5.5
In qemuBuildMasterKeyCommandLine, rather than inline code build the command use virQEMUBuildSecretObjectProps to generate the JSON secret object and then virQEMUBuildObjectCommandlineFromJSON to build the object. This is just like qemuBuildSecretInfoProps which will now also use the common function. Adjust the tests to remove the "format=raw,", since it's really not necessary as qemu will assume "raw" unless "format=base64" is provided. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 27 +++++++++++++++------- ...muxml2argv-disk-drive-network-rbd-auth-AES.args | 3 +-- .../qemuxml2argvdata/qemuxml2argv-master-key.args | 3 +-- .../qemuxml2argvdata/qemuxml2argv-name-escape.args | 3 +-- 4 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1455c0d..da624d0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -198,6 +198,9 @@ qemuBuildMasterKeyCommandLine(virCommandPtr cmd, char *alias = NULL; char *path = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; + const char *type = "secret"; + virJSONValuePtr props = NULL; + char *tmp = NULL; /* If the -object secret does not exist, then just return. This just * means the domain won't be able to use a secret master key and is @@ -219,10 +222,19 @@ qemuBuildMasterKeyCommandLine(virCommandPtr cmd, if (!(path = qemuDomainGetMasterKeyFilePath(domainLibDir))) goto cleanup; - virCommandAddArg(cmd, "-object"); - virBufferAsprintf(&buf, "secret,id=%s,format=raw,file=", alias); qemuBufferEscapeComma(&buf, path); - virCommandAddArgBuffer(cmd, &buf); + if (virBufferCheckError(&buf) < 0) + goto cleanup; + VIR_FREE(path); + path = virBufferContentAndReset(&buf); + + if (virQEMUBuildSecretObjectProps(path, true, NULL, NULL, NULL, &props) < 0) + goto cleanup; + + if (!(tmp = virQEMUBuildObjectCommandlineFromJSON(type, alias, props))) + goto cleanup; + + virCommandAddArgList(cmd, "-object", tmp, NULL); ret = 0; @@ -230,6 +242,7 @@ qemuBuildMasterKeyCommandLine(virCommandPtr cmd, virBufferFreeAndReset(&buf); VIR_FREE(alias); VIR_FREE(path); + VIR_FREE(tmp); return ret; } @@ -531,11 +544,9 @@ qemuBuildSecretInfoProps(qemuDomainSecretInfoPtr secinfo, if (!(keyid = qemuDomainGetMasterKeyAlias())) return -1; - if (virJSONValueObjectCreate(propsret, - "s:data", secinfo->s.aes.ciphertext, - "s:keyid", keyid, - "s:iv", secinfo->s.aes.iv, - "s:format", "base64", NULL) < 0) + if (virQEMUBuildSecretObjectProps(secinfo->s.aes.ciphertext, false, + "base64", keyid, secinfo->s.aes.iv, + propsret) < 0) goto cleanup; ret = 0; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-AES.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-AES.args index 7100d2d..77801bd 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-AES.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-AES.args @@ -7,8 +7,7 @@ QEMU_AUDIO_DRV=none \ /usr/bin/qemu \ -name QEMUGuest1 \ -S \ --object secret,id=masterKey0,format=raw,\ -file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-object secret,id=masterKey0,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -M pc \ -m 214 \ -smp 1 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-master-key.args b/tests/qemuxml2argvdata/qemuxml2argv-master-key.args index de030eb..3b1b672 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-master-key.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-master-key.args @@ -7,8 +7,7 @@ QEMU_AUDIO_DRV=none \ /usr/bin/qemu \ -name QEMUGuest1 \ -S \ --object secret,id=masterKey0,format=raw,\ -file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-object secret,id=masterKey0,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -M pc \ -m 214 \ -smp 2 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args b/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args index b59706c..659379a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args @@ -7,8 +7,7 @@ QEMU_AUDIO_DRV=none \ /usr/bin/qemu \ -name guest=foo=1,,bar=2,debug-threads=on \ -S \ --object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-foo=1,,\ -bar=2/master-key.aes \ +-object secret,id=masterKey0,file=/tmp/lib/domain--1-foo=1,,bar=2/master-key.aes \ -M pc \ -m 214 \ -smp 1 \ -- 2.5.5
Rework the logic to remove the various levels of indirection between qemuBuildSecretInfoProps and qemuBuildObjectSecretCommandLine. Merge them into qemuBuildObjectSecinfoCommandLine which will do all the work of building the secinfo secret object command line. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 74 +++++++++++++------------------------------------ 1 file changed, 20 insertions(+), 54 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index da624d0..eac36ab 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -520,79 +520,44 @@ qemuNetworkDriveGetPort(int protocol, } -/** - * qemuBuildSecretInfoProps: - * @secinfo: pointer to the secret info object - * @type: returns a pointer to a character string for object name - * @props: json properties to return +/* qemuBuildSecinfoCommandLine: + * @cmd: Pointer to the command string + * @secinfo: Pointer to a possible secinfo * - * Build the JSON properties for the secret info type. + * Build the command line string using the secinfo structure and + * common json object building principles. * - * Returns 0 on success with the filled in JSON property; otherwise, - * returns -1 on failure error message set. + * Returns 0 on success, -1 on failure w/ error message set */ static int -qemuBuildSecretInfoProps(qemuDomainSecretInfoPtr secinfo, - const char **type, - virJSONValuePtr *propsret) +qemuBuildObjectSecinfoCommandLine(virCommandPtr cmd, + qemuDomainSecretInfoPtr secinfo) { int ret = -1; + virJSONValuePtr props = NULL; char *keyid = NULL; - - *type = "secret"; + char *commandStr = NULL; if (!(keyid = qemuDomainGetMasterKeyAlias())) return -1; if (virQEMUBuildSecretObjectProps(secinfo->s.aes.ciphertext, false, "base64", keyid, secinfo->s.aes.iv, - propsret) < 0) + &props) < 0) goto cleanup; - ret = 0; - - cleanup: - VIR_FREE(keyid); - - return ret; -} - - -/** - * qemuBuildObjectSecretCommandLine: - * @cmd: the command to modify - * @secinfo: pointer to the secret info object - * - * If the secinfo is available and associated with an AES secret, - * then format the command line for the secret object. This object - * will be referenced by the device that needs/uses it, so it needs - * to be in place first. - * - * Returns 0 on success, -1 w/ error message on failure - */ -static int -qemuBuildObjectSecretCommandLine(virCommandPtr cmd, - qemuDomainSecretInfoPtr secinfo) -{ - int ret = -1; - virJSONValuePtr props = NULL; - const char *type; - char *tmp = NULL; - - if (qemuBuildSecretInfoProps(secinfo, &type, &props) < 0) - return -1; - - if (!(tmp = virQEMUBuildObjectCommandlineFromJSON(type, - secinfo->s.aes.alias, - props))) + if (!(commandStr = + virQEMUBuildObjectCommandlineFromJSON("secret", secinfo->s.aes.alias, + props))) goto cleanup; - virCommandAddArgList(cmd, "-object", tmp, NULL); + virCommandAddArgList(cmd, "-object", commandStr, NULL); ret = 0; cleanup: + VIR_FREE(keyid); virJSONValueFree(props); - VIR_FREE(tmp); + VIR_FREE(commandStr); return ret; } @@ -602,7 +567,8 @@ qemuBuildObjectSecretCommandLine(virCommandPtr cmd, * @cmd: Pointer to the command string * @secinfo: Pointer to a possible secinfo * - * Add the secret object for the disks that will be using it to perform + * If the secinfo is available and associated with an AES secret, then + * add the secret object for the disks that will be using it to perform * their authentication. * * Returns 0 on success, -1 w/ error on some sort of failure. @@ -615,7 +581,7 @@ qemuBuildDiskSecinfoCommandLine(virCommandPtr cmd, if (!secinfo || secinfo->type != VIR_DOMAIN_SECRET_INFO_TYPE_AES) return 0; - return qemuBuildObjectSecretCommandLine(cmd, secinfo); + return qemuBuildObjectSecinfoCommandLine(cmd, secinfo); } -- 2.5.5
Rather than inline code secret lookup for rbd/iscsi, use the common function. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/Makefile.am | 1 + src/storage/storage_backend_iscsi.c | 50 +++++-------------------------------- src/storage/storage_backend_rbd.c | 48 +++-------------------------------- 3 files changed, 10 insertions(+), 89 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index f3c9a14..019242b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1615,6 +1615,7 @@ libvirt_driver_storage_impl_la_SOURCES = libvirt_driver_storage_impl_la_CFLAGS = \ -I$(srcdir)/access \ -I$(srcdir)/conf \ + -I$(srcdir)/secret \ $(AM_CFLAGS) libvirt_driver_storage_impl_la_LDFLAGS = $(AM_LDFLAGS) libvirt_driver_storage_impl_la_LIBADD = diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index bccfba3..a45c525 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -1,7 +1,7 @@ /* * storage_backend_iscsi.c: storage backend for iSCSI handling * - * Copyright (C) 2007-2014 Red Hat, Inc. + * Copyright (C) 2007-2016 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -43,7 +43,7 @@ #include "virobject.h" #include "virstring.h" #include "viruuid.h" - +#include "secret_util.h" #define VIR_FROM_THIS VIR_FROM_STORAGE VIR_LOG_INIT("storage.storage_backend_iscsi"); @@ -277,11 +277,10 @@ virStorageBackendISCSISetAuth(const char *portal, virConnectPtr conn, virStoragePoolSourcePtr source) { - virSecretPtr secret = NULL; unsigned char *secret_value = NULL; + size_t secret_size; virStorageAuthDefPtr authdef = source->auth; int ret = -1; - char uuidStr[VIR_UUID_STRING_BUFLEN]; if (!authdef || authdef->authType == VIR_STORAGE_AUTH_TYPE_NONE) return 0; @@ -301,45 +300,9 @@ virStorageBackendISCSISetAuth(const char *portal, return -1; } - if (authdef->secretType == VIR_STORAGE_SECRET_TYPE_UUID) - secret = virSecretLookupByUUID(conn, authdef->secret.uuid); - else - secret = virSecretLookupByUsage(conn, VIR_SECRET_USAGE_TYPE_ISCSI, - authdef->secret.usage); - - if (secret) { - size_t secret_size; - secret_value = - conn->secretDriver->secretGetValue(secret, &secret_size, 0, - VIR_SECRET_GET_VALUE_INTERNAL_CALL); - if (!secret_value) { - if (authdef->secretType == VIR_STORAGE_SECRET_TYPE_UUID) { - virUUIDFormat(authdef->secret.uuid, uuidStr); - virReportError(VIR_ERR_INTERNAL_ERROR, - _("could not get the value of the secret " - "for username %s using uuid '%s'"), - authdef->username, uuidStr); - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("could not get the value of the secret " - "for username %s using usage value '%s'"), - authdef->username, authdef->secret.usage); - } - goto cleanup; - } - } else { - if (authdef->secretType == VIR_STORAGE_SECRET_TYPE_UUID) { - virUUIDFormat(authdef->secret.uuid, uuidStr); - virReportError(VIR_ERR_NO_SECRET, - _("no secret matches uuid '%s'"), - uuidStr); - } else { - virReportError(VIR_ERR_NO_SECRET, - _("no secret matches usage value '%s'"), - authdef->secret.usage); - } + if (virSecretGetSecretString(conn, authdef, VIR_SECRET_USAGE_TYPE_ISCSI, + &secret_value, &secret_size) < 0) goto cleanup; - } if (virISCSINodeUpdate(portal, source->devices[0].path, @@ -358,8 +321,7 @@ virStorageBackendISCSISetAuth(const char *portal, ret = 0; cleanup: - virObjectUnref(secret); - VIR_FREE(secret_value); + VIR_DISPOSE_N(secret_value, secret_size); return ret; } diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index ac46b9d..64ec545 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -36,6 +36,7 @@ #include "virrandom.h" #include "rados/librados.h" #include "rbd/librbd.h" +#include "secret_util.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -62,8 +63,6 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, size_t secret_value_size = 0; char *rados_key = NULL; virBuffer mon_host = VIR_BUFFER_INITIALIZER; - virSecretPtr secret = NULL; - char secretUuid[VIR_UUID_STRING_BUFLEN]; size_t i; char *mon_buff = NULL; const char *client_mount_timeout = "30"; @@ -86,48 +85,9 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, return -1; } - if (authdef->secretType == VIR_STORAGE_SECRET_TYPE_UUID) { - virUUIDFormat(authdef->secret.uuid, secretUuid); - VIR_DEBUG("Looking up secret by UUID: %s", secretUuid); - secret = virSecretLookupByUUIDString(conn, secretUuid); - } else if (authdef->secret.usage != NULL) { - VIR_DEBUG("Looking up secret by usage: %s", - authdef->secret.usage); - secret = virSecretLookupByUsage(conn, VIR_SECRET_USAGE_TYPE_CEPH, - authdef->secret.usage); - } - - if (secret == NULL) { - if (authdef->secretType == VIR_STORAGE_SECRET_TYPE_UUID) { - virReportError(VIR_ERR_NO_SECRET, - _("no secret matches uuid '%s'"), - secretUuid); - } else { - virReportError(VIR_ERR_NO_SECRET, - _("no secret matches usage value '%s'"), - authdef->secret.usage); - } + if (virSecretGetSecretString(conn, authdef, VIR_SECRET_USAGE_TYPE_CEPH, + &secret_value, &secret_value_size) < 0) goto cleanup; - } - - secret_value = conn->secretDriver->secretGetValue(secret, - &secret_value_size, 0, - VIR_SECRET_GET_VALUE_INTERNAL_CALL); - - if (!secret_value) { - if (authdef->secretType == VIR_STORAGE_SECRET_TYPE_UUID) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("could not get the value of the secret " - "for username '%s' using uuid '%s'"), - authdef->username, secretUuid); - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("could not get the value of the secret " - "for username '%s' using usage value '%s'"), - authdef->username, authdef->secret.usage); - } - goto cleanup; - } if (!(rados_key = virStringEncodeBase64(secret_value, secret_value_size))) goto cleanup; @@ -227,8 +187,6 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, VIR_DISPOSE_N(secret_value, secret_value_size); VIR_DISPOSE_STRING(rados_key); - virObjectUnref(secret); - virBufferFreeAndReset(&mon_host); VIR_FREE(mon_buff); return ret; -- 2.5.5
Move the enum into secret_util, rename it to be just virSecretLookupType. This includes quite a bit of collateral damage, but the goal is to remove the "virStorage*" and replace with the virSecretLookupType so that it's easier to to add new lookups that aren't necessarily storage pool related. Signed-off-by: John Ferlan <jferlan@redhat.com> --- cfg.mk | 2 +- src/libxl/libxl_conf.c | 2 +- src/qemu/qemu_domain.c | 4 ++-- src/secret/secret_util.c | 18 +++++++++--------- src/secret/secret_util.h | 22 ++++++++++++++++++++-- src/storage/storage_backend_iscsi.c | 7 ++++--- src/storage/storage_backend_rbd.c | 3 ++- src/util/virstoragefile.c | 33 +++++++++++++++++---------------- src/util/virstoragefile.h | 17 +++-------------- tests/qemuargv2xmltest.c | 4 ++-- 10 files changed, 61 insertions(+), 51 deletions(-) diff --git a/cfg.mk b/cfg.mk index a7b7266..0529a4e 100644 --- a/cfg.mk +++ b/cfg.mk @@ -780,7 +780,7 @@ mid_dirs=access|conf|cpu|locking|logging|network|node_device|rpc|security|storag sc_prohibit_cross_inclusion: @for dir in $(cross_dirs); do \ case $$dir in \ - util/) safe="util";; \ + util/) safe="(util|secret)";; \ access/ | conf/) safe="($$dir|conf|util)";; \ locking/) safe="($$dir|util|conf|rpc)";; \ cpu/| network/| node_device/| rpc/| security/| storage/) \ diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index a64b4c1..b3f78f0 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1033,7 +1033,7 @@ libxlMakeNetworkDiskSrc(virStorageSourcePtr src, char **srcstr) if (!(conn = virConnectOpen("xen:///system"))) goto cleanup; - if (virSecretGetSecretString(conn, src->auth, + if (virSecretGetSecretString(conn, &src->auth->secdef, VIR_SECRET_USAGE_TYPE_CEPH, &secret, &secretlen) < 0) goto cleanup; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c21465d..a871d3e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -829,7 +829,7 @@ qemuDomainSecretPlainSetup(virConnectPtr conn, if (protocol == VIR_STORAGE_NET_PROTOCOL_RBD) secretType = VIR_SECRET_USAGE_TYPE_CEPH; - return virSecretGetSecretString(conn, authdef, secretType, + return virSecretGetSecretString(conn, &authdef->secdef, secretType, &secinfo->s.plain.secret, &secinfo->s.plain.secretlen); } @@ -902,7 +902,7 @@ qemuDomainSecretAESSetup(virConnectPtr conn, goto cleanup; /* Grab the unencoded secret */ - if (virSecretGetSecretString(conn, authdef, secretType, + if (virSecretGetSecretString(conn, &authdef->secdef, secretType, &secret, &secretlen) < 0) goto cleanup; diff --git a/src/secret/secret_util.c b/src/secret/secret_util.c index 5602401..7bfe635 100644 --- a/src/secret/secret_util.c +++ b/src/secret/secret_util.c @@ -36,12 +36,12 @@ VIR_LOG_INIT("secret.secret_util"); /* virSecretGetSecretString: * @conn: Pointer to the connection driver to make secret driver call - * @authdef: Pointer to the disk storage authentication - * @secretUsageType: Type of secret usage for authdef lookup + * @secdef: Pointer to a storage type def for uuid/usage lookup + * @secretUsageType: Type of secret usage for usage lookup * @secret: returned secret as a sized stream of unsigned chars * @secret_size: Return size of the secret - either raw text or base64 * - * Lookup the secret for the authdef usage type and return it as raw text. + * Lookup the secret for the usage type and return it as raw text. * It is up to the caller to encode the secret further. * * Returns 0 on success, -1 on failure. On success the memory in secret @@ -49,7 +49,7 @@ VIR_LOG_INIT("secret.secret_util"); */ int virSecretGetSecretString(virConnectPtr conn, - virStorageAuthDefPtr authdef, + virSecretLookupTypeDefPtr secdef, virSecretUsageType secretUsageType, uint8_t **secret, size_t *secret_size) @@ -57,14 +57,14 @@ virSecretGetSecretString(virConnectPtr conn, virSecretPtr sec = NULL; int ret = -1; - switch (authdef->secretType) { - case VIR_STORAGE_SECRET_TYPE_UUID: - sec = conn->secretDriver->secretLookupByUUID(conn, authdef->secret.uuid); + switch (secdef->type) { + case VIR_SECRET_LOOKUP_TYPE_UUID: + sec = conn->secretDriver->secretLookupByUUID(conn, secdef->u.uuid); break; - case VIR_STORAGE_SECRET_TYPE_USAGE: + case VIR_SECRET_LOOKUP_TYPE_USAGE: sec = conn->secretDriver->secretLookupByUsage(conn, secretUsageType, - authdef->secret.usage); + secdef->u.usage); break; } diff --git a/src/secret/secret_util.h b/src/secret/secret_util.h index a039662..18ac86a 100644 --- a/src/secret/secret_util.h +++ b/src/secret/secret_util.h @@ -23,10 +23,28 @@ # define __VIR_SECRET_H__ # include "internal.h" -# include "virstoragefile.h" + +typedef enum { + VIR_SECRET_LOOKUP_TYPE_NONE, + VIR_SECRET_LOOKUP_TYPE_UUID, + VIR_SECRET_LOOKUP_TYPE_USAGE, + + VIR_SECRET_LOOKUP_TYPE_LAST +} virSecretLookupType; + +typedef struct _virSecretLookupTypeDef virSecretLookupTypeDef; +typedef virSecretLookupTypeDef *virSecretLookupTypeDefPtr; +struct _virSecretLookupTypeDef { + int type; /* virSecretLookupType */ + union { + unsigned char uuid[VIR_UUID_BUFLEN]; + char *usage; + } u; + +}; int virSecretGetSecretString(virConnectPtr conn, - virStorageAuthDefPtr authdef, + virSecretLookupTypeDefPtr secdef, virSecretUsageType secretUsageType, uint8_t **ret_secret, size_t *ret_secret_size) diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index a45c525..81872ea 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -285,8 +285,8 @@ virStorageBackendISCSISetAuth(const char *portal, if (!authdef || authdef->authType == VIR_STORAGE_AUTH_TYPE_NONE) return 0; - VIR_DEBUG("username='%s' authType=%d secretType=%d", - authdef->username, authdef->authType, authdef->secretType); + VIR_DEBUG("username='%s' authType=%d secdef.type=%d", + authdef->username, authdef->authType, authdef->secdef.type); if (authdef->authType != VIR_STORAGE_AUTH_TYPE_CHAP) { virReportError(VIR_ERR_XML_ERROR, "%s", _("iscsi pool only supports 'chap' auth type")); @@ -300,7 +300,8 @@ virStorageBackendISCSISetAuth(const char *portal, return -1; } - if (virSecretGetSecretString(conn, authdef, VIR_SECRET_USAGE_TYPE_ISCSI, + if (virSecretGetSecretString(conn, &authdef->secdef, + VIR_SECRET_USAGE_TYPE_ISCSI, &secret_value, &secret_size) < 0) goto cleanup; diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 64ec545..bf77c54 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -85,7 +85,8 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, return -1; } - if (virSecretGetSecretString(conn, authdef, VIR_SECRET_USAGE_TYPE_CEPH, + if (virSecretGetSecretString(conn, &authdef->secdef, + VIR_SECRET_USAGE_TYPE_CEPH, &secret_value, &secret_value_size) < 0) goto cleanup; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index d2da9e7..54940a0 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1506,8 +1506,8 @@ virStorageAuthDefFree(virStorageAuthDefPtr authdef) VIR_FREE(authdef->username); VIR_FREE(authdef->secrettype); - if (authdef->secretType == VIR_STORAGE_SECRET_TYPE_USAGE) - VIR_FREE(authdef->secret.usage); + if (authdef->secdef.type == VIR_SECRET_LOOKUP_TYPE_USAGE) + VIR_FREE(authdef->secdef.u.usage); VIR_FREE(authdef); } @@ -1526,11 +1526,12 @@ virStorageAuthDefCopy(const virStorageAuthDef *src) if (VIR_STRDUP(ret->secrettype, src->secrettype) < 0) goto error; ret->authType = src->authType; - ret->secretType = src->secretType; - if (ret->secretType == VIR_STORAGE_SECRET_TYPE_UUID) { - memcpy(ret->secret.uuid, src->secret.uuid, sizeof(ret->secret.uuid)); - } else if (ret->secretType == VIR_STORAGE_SECRET_TYPE_USAGE) { - if (VIR_STRDUP(ret->secret.usage, src->secret.usage) < 0) + ret->secdef.type = src->secdef.type; + if (ret->secdef.type == VIR_SECRET_LOOKUP_TYPE_UUID) { + memcpy(ret->secdef.u.uuid, src->secdef.u.uuid, + sizeof(ret->secdef.u.uuid)); + } else if (ret->secdef.type == VIR_SECRET_LOOKUP_TYPE_USAGE) { + if (VIR_STRDUP(ret->secdef.u.usage, src->secdef.u.usage) < 0) goto error; } return ret; @@ -1573,16 +1574,16 @@ virStorageAuthDefParseSecret(xmlXPathContextPtr ctxt, } if (uuid) { - if (virUUIDParse(uuid, authdef->secret.uuid) < 0) { + if (virUUIDParse(uuid, authdef->secdef.u.uuid) < 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("invalid auth secret uuid")); goto cleanup; } - authdef->secretType = VIR_STORAGE_SECRET_TYPE_UUID; + authdef->secdef.type = VIR_SECRET_LOOKUP_TYPE_UUID; } else { - authdef->secret.usage = usage; + authdef->secdef.u.usage = usage; usage = NULL; - authdef->secretType = VIR_STORAGE_SECRET_TYPE_USAGE; + authdef->secdef.type = VIR_SECRET_LOOKUP_TYPE_USAGE; } ret = 0; @@ -1625,7 +1626,7 @@ virStorageAuthDefParseXML(xmlXPathContextPtr ctxt) VIR_FREE(authtype); } - authdef->secretType = VIR_STORAGE_SECRET_TYPE_NONE; + authdef->secdef.type = VIR_SECRET_LOOKUP_TYPE_NONE; if (virStorageAuthDefParseSecret(ctxt, authdef) < 0) goto error; @@ -1680,12 +1681,12 @@ virStorageAuthDefFormat(virBufferPtr buf, else virBufferAddLit(buf, "<secret"); - if (authdef->secretType == VIR_STORAGE_SECRET_TYPE_UUID) { - virUUIDFormat(authdef->secret.uuid, uuidstr); + if (authdef->secdef.type == VIR_SECRET_LOOKUP_TYPE_UUID) { + virUUIDFormat(authdef->secdef.u.uuid, uuidstr); virBufferAsprintf(buf, " uuid='%s'/>\n", uuidstr); - } else if (authdef->secretType == VIR_STORAGE_SECRET_TYPE_USAGE) { + } else if (authdef->secdef.type == VIR_SECRET_LOOKUP_TYPE_USAGE) { virBufferEscapeString(buf, " usage='%s'/>\n", - authdef->secret.usage); + authdef->secdef.u.usage); } else { virBufferAddLit(buf, "/>\n"); } diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index b88e715..b4ad42e 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -1,7 +1,7 @@ /* * virstoragefile.h: file utility functions for FS storage backend * - * Copyright (C) 2007-2009, 2012-2014 Red Hat, Inc. + * Copyright (C) 2007-2009, 2012-2016 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -28,6 +28,7 @@ # include "virseclabel.h" # include "virstorageencryption.h" # include "virutil.h" +# include "secret/secret_util.h" /* Minimum header size required to probe all known formats with * virStorageFileProbeFormat, or obtain metadata from a known format. @@ -201,25 +202,13 @@ typedef enum { } virStorageAuthType; VIR_ENUM_DECL(virStorageAuth) -typedef enum { - VIR_STORAGE_SECRET_TYPE_NONE, - VIR_STORAGE_SECRET_TYPE_UUID, - VIR_STORAGE_SECRET_TYPE_USAGE, - - VIR_STORAGE_SECRET_TYPE_LAST -} virStorageSecretType; - typedef struct _virStorageAuthDef virStorageAuthDef; typedef virStorageAuthDef *virStorageAuthDefPtr; struct _virStorageAuthDef { char *username; char *secrettype; /* <secret type='%s' for disk source */ int authType; /* virStorageAuthType */ - int secretType; /* virStorageSecretType */ - union { - unsigned char uuid[VIR_UUID_BUFLEN]; - char *usage; - } secret; + virSecretLookupTypeDef secdef; }; typedef struct _virStorageDriverData virStorageDriverData; diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 0fe6b9b..212e6e8 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -36,8 +36,8 @@ static int testSanitizeDef(virDomainDefPtr vmdef) virDomainDiskDefPtr disk = vmdef->disks[i]; if (disk->src->auth) { - disk->src->auth->secretType = VIR_STORAGE_SECRET_TYPE_USAGE; - if (VIR_STRDUP(disk->src->auth->secret.usage, + disk->src->auth->secdef.type = VIR_SECRET_LOOKUP_TYPE_USAGE; + if (VIR_STRDUP(disk->src->auth->secdef.u.usage, "qemuargv2xml_usage") < 0) goto fail; } -- 2.5.5
On 05/31/2016 06:39 PM, John Ferlan wrote:
v1: http://www.redhat.com/archives/libvir-list/2016-May/msg02017.html
Changes in v2:
Patch 1 - Move function to a new src/util/virqemu.c with adjustments to other affected areas
Patches 2-4 - Similarly adjust the flow/name of the patches with an adjustment so that patch 2 is just the new code in virqemu.c and patch 3 shows how the master secret code would use the same function to build it's command line. Patch 4 just follows suit...
Based on the ongoing discussion in v1: http://www.redhat.com/archives/libvir-list/2016-June/msg00014.html I'll drop patches 2-4 (although I may add one to use virJSONValueObjectCreate rather than the current open code in qemuBuildMasterKeyCommandLine). John
Patch 5 was reviewed/ACK'd - this patch just shows the changes to remove the "virSecretPtr secret = NULL;" and "virObjectUnref(secret);"
Patch 6 is unchanged
John Ferlan (6): qemu: Move and rename qemuBuildObjectCommandlineFromJSON util: Introduce virQEMUBuildSecretObjectProps qemu: Use virQEMUBuildSecretObjectProps to build secret objects qemu: Rework secinfo command line building storage: Use virSecretGetSecretString secret: Move virStorageSecretType to secret_util and rename
cfg.mk | 2 +- po/POTFILES.in | 1 + src/Makefile.am | 2 + src/libvirt_private.syms | 5 + src/libxl/libxl_conf.c | 2 +- src/qemu/qemu_command.c | 219 +++++---------------- src/qemu/qemu_command.h | 4 - src/qemu/qemu_domain.c | 4 +- src/secret/secret_util.c | 18 +- src/secret/secret_util.h | 22 ++- src/storage/storage_backend_iscsi.c | 55 +----- src/storage/storage_backend_rbd.c | 49 +---- src/util/virqemu.c | 211 ++++++++++++++++++++ src/util/virqemu.h | 42 ++++ src/util/virstoragefile.c | 33 ++-- src/util/virstoragefile.h | 17 +- tests/qemuargv2xmltest.c | 4 +- tests/qemucommandutiltest.c | 9 +- ...muxml2argv-disk-drive-network-rbd-auth-AES.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-master-key.args | 3 +- .../qemuxml2argvdata/qemuxml2argv-name-escape.args | 3 +- 21 files changed, 380 insertions(+), 328 deletions(-) create mode 100644 src/util/virqemu.c create mode 100644 src/util/virqemu.h
participants (1)
-
John Ferlan