Re: [libvirt] [PATCH 00/10] Convert secret driver to use hashed object
On 03/02/2016 01:54 PM, John Ferlan wrote:
> This series replaces the usage of linked list in secret driver with a
> hashed object list as was suggested in the review in the last patch of
> the previous secret driver changes series:
>
> http://www.redhat.com/archives/libvir-list/2016-February/msg01274.html
>
> Patch 1 - starts the ball rolling
> Patch 2 - is yet another code optimization (as found in bridge_driver)
> Patches 3-8 - Add the new API's "slowly" (for review purposes)
> Patch 9 - replaces the usage of linked lists with the hashed object
> Patch 10 - moves the secretLoadAllConfigs to secret_conf
>
> The changes are loosely modeled after both virdomainobj and network_conf
> functionality. The real meat and potato(e)s is found in patches 5 and 9.
> Other functions should be relatively straightforward.
>
> I've done testing of various virsh secret-* commands (both valid and invalid
> options) as well as performing driver start, stop, and reload testing.
> Each patch was built using 'check' and 'syntax-check'.
>
> The end result is much more code in secret_conf and much less in
> secret_driver.
>
> John Ferlan (10):
> secret: Move virSecretObj to secret_conf.h
> Add secretObjFromSecret
> secret: Add hashed virSecretObj and virSecretObjList
> secret: Introduce virSecretObjListFindBy{UUID|Usage} support
> secret: Introduce virSecretObjListAdd* and virSecretObjListRemove
> secret: Introduce virSecretObjListNumOfSecrets
> secret: Introduce virSecretObjListExport
> secret: Introduce virSecretObjListGetUUIDs
> secret: Use the hashed virSecretObjList
> secret: Move and rename secretLoadAllConfigs
>
> src/conf/secret_conf.c | 819
> -
> src/conf/secret_conf.h | 76 -
> src/libvirt_private.syms | 11 +
> src/secret/secret_driver.c | 647 +++
> 4 files changed, 957 insertions(+), 596 deletions(-)
>
Ping - any chance these can be finished up? Would it be better to
repost the entire series. It should apply cleanly as there hasn't been
changes in this space.
Tks -
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 03/12] qemu: Introduce qemuDomainHostdevPrivatePtr
Modeled after the qemuDomainDiskPrivatePtr logic, create a privateData
pointer in the _virDomainHostdevDef to allow storage of private data
for a hypervisor in order to at least temporarily store auth/secrets
data for usage during qemuBuildCommandLine.
NB: Since the qemu_parse_command (qemuParseCommandLine) code is not
expecting to restore the auth/secret data, there's no need to add
code to handle this new structure there.
Updated copyrights for modules touched. Some didn't have updates in a
couple years even though changes have been made.
Signed-off-by: John Ferlan
---
src/conf/domain_conf.c| 33 +---
src/conf/domain_conf.h| 5 -
src/lxc/lxc_native.c | 4 ++--
src/qemu/qemu_domain.c| 44 +++
src/qemu/qemu_domain.h| 13 +
src/qemu/qemu_parse_command.c | 4 ++--
src/vbox/vbox_common.c| 4 ++--
src/xenconfig/xen_common.c| 4 ++--
src/xenconfig/xen_sxpr.c | 4 ++--
tests/virhostdevtest.c| 3 ++-
10 files changed, 99 insertions(+), 19 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 28248c8..07f5b26 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2121,16 +2121,32 @@ void virDomainVideoDefFree(virDomainVideoDefPtr def)
VIR_FREE(def);
}
-virDomainHostdevDefPtr virDomainHostdevDefAlloc(void)
+
+virDomainHostdevDefPtr
+virDomainHostdevDefAlloc(virDomainXMLOptionPtr xmlopt)
{
virDomainHostdevDefPtr def = NULL;
if (VIR_ALLOC(def) < 0 ||
-VIR_ALLOC(def->info) < 0)
+VIR_ALLOC(def->info) < 0) {
VIR_FREE(def);
+return NULL;
+}
+
+if (xmlopt &&
+xmlopt->privateData.hostdevNew &&
+!(def->privateData = xmlopt->privateData.hostdevNew()))
+goto error;
+
return def;
+
+ error:
+VIR_FREE(def->info);
+VIR_FREE(def);
+return NULL;
}
+
static void
virDomainHostdevSubsysSCSIiSCSIClear(virDomainHostdevSubsysSCSIiSCSIPtr
iscsisrc)
{
@@ -12247,7 +12263,8 @@ virDomainVideoDefParseXML(xmlNodePtr node,
}
static virDomainHostdevDefPtr
-virDomainHostdevDefParseXML(xmlNodePtr node,
+virDomainHostdevDefParseXML(virDomainXMLOptionPtr xmlopt,
+xmlNodePtr node,
xmlXPathContextPtr ctxt,
virHashTablePtr bootHash,
unsigned int flags)
@@ -12259,7 +12276,7 @@ virDomainHostdevDefParseXML(xmlNodePtr node,
ctxt->node = node;
-if (!(def = virDomainHostdevDefAlloc()))
+if (!(def = virDomainHostdevDefAlloc(xmlopt)))
goto error;
if (mode) {
@@ -12909,8 +12926,9 @@ virDomainDeviceDefParse(const char *xmlStr,
goto error;
break;
case VIR_DOMAIN_DEVICE_HOSTDEV:
-if (!(dev->data.hostdev = virDomainHostdevDefParseXML(node, ctxt,
- NULL, flags)))
+if (!(dev->data.hostdev = virDomainHostdevDefParseXML(xmlopt, node,
+ ctxt, NULL,
+ flags)))
goto error;
break;
case VIR_DOMAIN_DEVICE_CONTROLLER:
@@ -16430,7 +16448,8 @@ virDomainDefParseXML(xmlDocPtr xml,
for (i = 0; i < n; i++) {
virDomainHostdevDefPtr hostdev;
-hostdev = virDomainHostdevDefParseXML(nodes[i], ctxt, bootHash, flags);
+hostdev = virDomainHostdevDefParseXML(xmlopt, nodes[i], ctxt,
+ bootHash, flags);
if (!hostdev)
goto error;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 1986f53..fb6a02b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -543,6 +543,8 @@ struct _virDomainHostdevCaps {
/* basic device for direct passthrough */
struct _virDomainHostdevDef {
virDomainDeviceDef parent; /* higher level Def containing this */
+virObjectPtr privateData;
+
int mode; /* enum virDomainHostdevMode */
int startupPolicy; /* enum virDomainStartupPolicy */
bool managed;
@@ -2495,6 +2497,7 @@ struct _virDomainXMLPrivateDataCallbacks {
virDomainXMLPrivateDataAllocFunc alloc;
virDomainXMLPrivateDataFreeFunc free;
virDomainXMLPrivateDataNewFuncdiskNew;
+virDomainXMLPrivateDataNewFunchostdevNew;
virDomainXMLPrivateDataFormatFunc format;
virDomainXMLPrivateDataParseFunc parse;
};
@@ -2572,7 +2575,7 @@ void virDomainMemballoonDefFree(virDomainMemballoonDefPtr
def);
void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def);
void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def);
void virDomainVideoDefFree(virDomainVideoDefPtr def);
-virDomainHostdevDefPtr virDomainHostdevDefAlloc(void);
+virDomainHostdevDefPtr virDomainHostdevDefAlloc(virDomainXMLOptionPtr xmlopt);
void virDomainHostdevDefClear(virDom
[libvirt] [PATCH v2 11/12] qemu: Introduce qemuDomainSecretIV
Add the data structure and infrastructure to support an initialization
vector (IV) secret. The IV secret generation will need to have access
to the domain private master key, so let's make sure the prepare disk
and hostdev functions can accept that now.
Anywhere that needs to make a decision over which secret type to use
in order to fill in or use the IV secret has a switch added. In particular,
when building the command line for the network URI, create a couple of
helper functions which make the decision to add the secret info.
Signed-off-by: John Ferlan
---
src/qemu/qemu_command.c | 106 ++--
src/qemu/qemu_domain.c | 34 ++--
src/qemu/qemu_domain.h | 22 --
src/qemu/qemu_hotplug.c | 6 +--
4 files changed, 138 insertions(+), 30 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8629c15..a8cacb3 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -606,6 +606,85 @@ qemuNetworkDriveGetPort(int protocol,
return -1;
}
+
+/* qemuBuildGeneralSecinfoURI:
+ * @uri: Pointer to the URI structure to add to
+ * @secinfo: Pointer to the secret info data (if present)
+ *
+ * If we have a secinfo, then build the command line options for
+ * the secret info for the "general" case (somewhat a misnomer since
+ * an iscsi disk is the only one with a secinfo).
+ *
+ * Returns 0 on success or if no secinfo,
+ * -1 and error message if fail to add secret information
+ */
+static int
+qemuBuildGeneralSecinfoURI(virURIPtr uri,
+ qemuDomainSecretInfoPtr secinfo)
+{
+if (!secinfo)
+return 0;
+
+switch ((qemuDomainSecretInfoType) secinfo->type) {
+case VIR_DOMAIN_SECRET_INFO_PLAIN:
+if (secinfo->s.plain.secret) {
+if (virAsprintf(&uri->user, "%s:%s",
+secinfo->s.plain.username,
+secinfo->s.plain.secret) < 0)
+return -1;
+} else {
+if (VIR_STRDUP(uri->user, secinfo->s.plain.username) < 0)
+return -1;
+}
+break;
+
+case VIR_DOMAIN_SECRET_INFO_IV:
+case VIR_DOMAIN_SECRET_INFO_LAST:
+return -1;
+}
+
+return 0;
+}
+
+
+/* qemuBuildRBDSecinfoURI:
+ * @uri: Pointer to the URI structure to add to
+ * @secinfo: Pointer to the secret info data (if present)
+ *
+ * If we have a secinfo, then build the command line options for
+ * the secret info for the RBD network storage. Assumption for this
+ * is both username and secret exist for plaintext
+ *
+ * Returns 0 on success or if no secinfo,
+ * -1 and error message if fail to add secret information
+ */
+static int
+qemuBuildRBDSecinfoURI(virBufferPtr buf,
+ qemuDomainSecretInfoPtr secinfo)
+{
+if (!secinfo) {
+virBufferAddLit(buf, ":auth_supported=none");
+return 0;
+}
+
+switch ((qemuDomainSecretInfoType) secinfo->type) {
+case VIR_DOMAIN_SECRET_INFO_PLAIN:
+virBufferEscape(buf, '\\', ":", ":id=%s",
+secinfo->s.plain.username);
+virBufferEscape(buf, '\\', ":",
+":key=%s:auth_supported=cephx\\;none",
+secinfo->s.plain.secret);
+break;
+
+case VIR_DOMAIN_SECRET_INFO_IV:
+case VIR_DOMAIN_SECRET_INFO_LAST:
+return -1;
+}
+
+return 0;
+}
+
+
#define QEMU_DEFAULT_NBD_PORT "10809"
static char *
@@ -701,7 +780,8 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
goto cleanup;
}
-if ((uri->port = qemuNetworkDriveGetPort(src->protocol,
src->hosts->port)) < 0)
+if ((uri->port = qemuNetworkDriveGetPort(src->protocol,
+ src->hosts->port)) < 0)
goto cleanup;
if (src->path) {
@@ -721,17 +801,8 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
virAsprintf(&uri->query, "socket=%s", src->hosts->socket) < 0)
goto cleanup;
-if (secinfo) {
-if (secinfo->s.plain.secret) {
-if (virAsprintf(&uri->user, "%s:%s",
-secinfo->s.plain.username,
-secinfo->s.plain.secret) < 0)
-goto cleanup;
-} else {
-if (VIR_STRDUP(uri->user, secinfo->s.plain.username) < 0)
-goto cleanup;
-}
-}
+if (qemuBuildGeneralSecinfoURI(uri, secinfo) < 0)
+goto cleanup;
if (VIR_STRDUP(uri->server, src->hosts->name) < 0)
goto cleanup;
@@ -777,15 +848,8 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
if (src->snapshot)
virBufferEscape(&buf, '\\', ":", "@%s", src->snapshot);
-if (secinfo) {
-virBufferEs
[libvirt] [PATCH v2 06/12] qemu: hotplug: Assume support for -device for attach virtio disk
Since support for QEMU_CAPS_DEVICE is not assumed, let's drop the legacy
code to make life easier going forward.
Signed-off-by: John Ferlan
---
src/qemu/qemu_hotplug.c | 79 +
1 file changed, 33 insertions(+), 46 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 94c0222..11efd7b 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -317,7 +317,6 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
virDomainDiskDefPtr disk)
{
int ret = -1;
-const char* type = virDomainDiskBusTypeToString(disk->bus);
qemuDomainObjPrivatePtr priv = vm->privateData;
char *devstr = NULL;
char *drivestr = NULL;
@@ -341,62 +340,50 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
if (qemuDomainPrepareDisk(driver, vm, disk, NULL, false) < 0)
goto cleanup;
-if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
-if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
-if (virDomainCCWAddressAssign(&disk->info, priv->ccwaddrs,
- !disk->info.addr.ccw.assigned) < 0)
-goto error;
-} else if (!disk->info.type ||
-disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
-if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &disk->info) < 0)
-goto error;
-}
-releaseaddr = true;
-if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0)
+if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
+if (virDomainCCWAddressAssign(&disk->info, priv->ccwaddrs,
+ !disk->info.addr.ccw.assigned) < 0)
goto error;
-
-if (qemuDomainSecretDiskPrepare(conn, disk) < 0)
+} else if (!disk->info.type ||
+disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &disk->info) < 0)
goto error;
+}
+releaseaddr = true;
+if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0)
+goto error;
-if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps)))
-goto error;
+if (qemuDomainSecretDiskPrepare(conn, disk) < 0)
+goto error;
-if (!(drivealias = qemuDeviceDriveHostAlias(disk, priv->qemuCaps)))
-goto error;
+if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps)))
+goto error;
-if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps)))
-goto error;
-}
+if (!(drivealias = qemuDeviceDriveHostAlias(disk, priv->qemuCaps)))
+goto error;
+
+if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps)))
+goto error;
if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0)
goto error;
qemuDomainObjEnterMonitor(driver, vm);
-if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
-ret = qemuMonitorAddDrive(priv->mon, drivestr);
-if (ret == 0) {
-ret = qemuMonitorAddDevice(priv->mon, devstr);
-if (ret < 0) {
-virErrorPtr orig_err = virSaveLastError();
-if (!drivealias ||
-qemuMonitorDriveDel(priv->mon, drivealias) < 0) {
-VIR_WARN("Unable to remove drive %s (%s) after failed "
- "qemuMonitorAddDevice",
- NULLSTR(drivealias), drivestr);
-}
-if (orig_err) {
-virSetError(orig_err);
-virFreeError(orig_err);
-}
+ret = qemuMonitorAddDrive(priv->mon, drivestr);
+if (ret == 0) {
+ret = qemuMonitorAddDevice(priv->mon, devstr);
+if (ret < 0) {
+virErrorPtr orig_err = virSaveLastError();
+if (!drivealias ||
+qemuMonitorDriveDel(priv->mon, drivealias) < 0) {
+VIR_WARN("Unable to remove drive %s (%s) after failed "
+ "qemuMonitorAddDevice",
+ NULLSTR(drivealias), drivestr);
+}
+if (orig_err) {
+virSetError(orig_err);
+virFreeError(orig_err);
}
-}
-} else if (!disk->info.type ||
-disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
-virDevicePCIAddress guestAddr = disk->info.addr.pci;
-ret = qemuMonitorAddPCIDisk(priv->mon, src, type, &guestAddr);
-if (ret == 0) {
-disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
-memcpy(&disk->info.addr.pci, &guestAddr, sizeof(guestAddr));
}
}
if (qemuDomainObjExitMonitor(driver, vm) < 0) {
--
2.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/l
[libvirt] [PATCH v2 08/12] qemu: hotplug: Adjust error path for attach virtio disk
Adjust error path logic to make it clearer how to undo the failed add.
Signed-off-by: John Ferlan
---
src/qemu/qemu_hotplug.c | 55 -
1 file changed, 32 insertions(+), 23 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index ae314be..17a70a3 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -318,6 +318,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
{
int ret = -1;
qemuDomainObjPrivatePtr priv = vm->privateData;
+virErrorPtr orig_err;
char *devstr = NULL;
char *drivestr = NULL;
char *drivealias = NULL;
@@ -368,36 +369,24 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0)
goto error;
+/* Attach the device - 2 step process */
qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorAddDrive(priv->mon, drivestr);
-if (ret == 0) {
-ret = qemuMonitorAddDevice(priv->mon, devstr);
-if (ret < 0) {
-virErrorPtr orig_err = virSaveLastError();
-if (!drivealias ||
-qemuMonitorDriveDel(priv->mon, drivealias) < 0) {
-VIR_WARN("Unable to remove drive %s (%s) after failed "
- "qemuMonitorAddDevice",
- NULLSTR(drivealias), drivestr);
-}
-if (orig_err) {
-virSetError(orig_err);
-virFreeError(orig_err);
-}
-}
-}
+
+if (qemuMonitorAddDrive(priv->mon, drivestr) < 0)
+goto failadddrive;
+
+if (qemuMonitorAddDevice(priv->mon, devstr) < 0)
+goto failadddevice;
+
if (qemuDomainObjExitMonitor(driver, vm) < 0) {
releaseaddr = false;
-ret = -1;
-goto error;
+goto failexitmonitor;
}
-virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0);
-
-if (ret < 0)
-goto error;
+virDomainAuditDisk(vm, NULL, disk->src, "attach", true);
virDomainDiskInsertPreAlloced(vm->def, disk);
+ret = 0;
cleanup:
qemuDomainSecretDiskDestroy(disk);
@@ -407,6 +396,26 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
virObjectUnref(cfg);
return ret;
+ failadddevice:
+orig_err = virSaveLastError();
+if (!drivealias ||
+qemuMonitorDriveDel(priv->mon, drivealias) < 0) {
+VIR_WARN("Unable to remove drive %s (%s) after failed "
+ "qemuMonitorAddDevice",
+ NULLSTR(drivealias), drivestr);
+}
+if (orig_err) {
+virSetError(orig_err);
+virFreeError(orig_err);
+}
+
+ failadddrive:
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+releaseaddr = false;
+
+ failexitmonitor:
+virDomainAuditDisk(vm, NULL, disk->src, "attach", false);
+
error:
if (releaseaddr)
qemuDomainReleaseDeviceAddress(vm, &disk->info, src);
--
2.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 12/12] qemu: Utilize qemu secret objects for SCSI/RBD auth/secret
https://bugzilla.redhat.com/show_bug.cgi?id=1182074
If they're available and we need to pass secrets to qemu, then use the
qemu domain secret object in order to pass the secrets for iSCSI and
RBD volumes instead of passing plaintext or base64 encoded secrets on
the command line.
New APIs:
qemuDomainGetIVKeyAlias:
Generate/return the secret object alias for an initialization
vector (IV) secret info type. This will be saved in the secret
info block.
qemuDomainSecretInfoGetAlias:
Return a pointer to the alias to the specific secret info as
long as the secret object is supported (future patches may add
a new secret info type with a different pointer to return).
qemuDomainSecretHaveEncrypt:
Boolean function to determine whether the underly encryption
API is available. This function will utilize a similar mechanism
as the 'gnutls_rnd' did in configure.ac. This function creates
the encrypted secret based upon the domain master key, an
initialization vector (16 byte random value), and the stored secret.
qemuDomainSecretIVSetup: (private)
This API handles the details of the generation of the IV secret
and saves the pieces that need to be passed to qemu in order for
the secret to be decrypted. The requirement from qemu is the IV
and encrypted secret are to be base64 encoded. They can be passed
either directly or within a file. This implementation chooses
to pass directly rather than a file (file passing is shown below).
qemuBuildiSCSICommandLine: (private)
qemuBuildDiskiSCSICommandLine
qemuBuildHostdeviSCSICommandLine
These API's will handle adding the IV secret onto an '-iscsi'
command line option.
qemuBuildSecretInfoProps: (private)
Generate/return a JSON properties object for the IV secret to
be used by both the command building and eventually the hotplug
code in order to add the secret object.
Changes for qemuDomainSecret{Disk|Hostdev}Prepare:
If both the encryption API exists and the secret object exist, then
setup the IV secret (qemuDomainSecretIVSetup) as the default means
for the disk/hostdev to provide the secret to qemu. Prior to command
line generation and during hotplug, these prepare API's are called
allowing for code after the API to perform the right steps.
Command Building:
Adjust the qemuBuild{General|RBD}SecinfoURI API's in order to generate
the specific command options for an IV secret, such as:
For iSCSI:
-object secret,id=sec0,keyid=$masterKey,filename=/path/to/example.pw
or
-object secret,id=sec0,keyid=$masterKey,data=$base64encodedencrypted,
format=base64
-iscsi -initiator-name=$iqn,user=user,password-secret=sec0
-drive file=iscsi://example.com/$iqn/1,...
For RBD:
-object secret,id=secret0,keyid=$masterKey,file=/path/to/poolkey.b64,
format=base64
or
-object secret,id=secret0,keyid=$masterKey,data=$base64encodedencrypted,
format=base64
-drive file=rbd:pool/image:id=myname:auth_supported=cephx\;none:\
mon_host=mon1.example.org\:6321,password-secret=secret0,...
where for both 'id=' value is the secret object alias, the 'keyid=
$masterKey' is the master key shared with qemu, and the -drive
syntax will reference that alias as the 'password-secret'. For
the iSCSI object 'user=' replaces the URI generated 'user:secret@'
prepended to the iSCSI 'host' name (example.com). For the RBD -drive
syntax, the 'id=myname' is kept to define the username, while the
'key=$base64 encoded secret' is removed.
While according to the syntax described for qemu commits 'b189346eb'
(iSCSI) and '60390a21' (RBD) or as seen in the email archive:
https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg04083.html
it is possible to pass a plaintext password via a file, the qemu
commit 'ac1d8878' describes the more feature rich 'keyid=' option
based upon the shared masterKey.
Tests:
Add mock's for virRandomBytes and gnutls_rnd in order to return a
constant stream of '0xff' in the bytes for a non random key in order
to generate "constant" values for the secrets so that the tests can
use those results to compare results.
Hotplug:
Since the hotplug code doesn't add command line arguments, passing
the encoded/plaintext secrets directly to the monitor will suffice.
Besides, it's passing the IV secret via '-iscsi' won't be possible.
Perhaps when the -drive command is modified to accept not only the
initiator-name, but -user and -password-secret arguments, then the
IV code can be utilized for hotplug secrets.
The goal is to make this the default and have no user interaction
required in order to allow using the IV mechanism. If the mechanism
is not available, then fall back to the current mechanism.
Signed-off-by: John Ferlan
---
configure.ac | 1 +
src/qemu/qemu_alias.c | 23 ++
[libvirt] [PATCH v2 04/12] qemu: Introduce qemuDomainSecretHostdevPrepare and Destroy
Similar to the qemuDomainSecretDiskPrepare, generate the secret
for the Hostdev's prior to call qemuProcessLaunch which calls
qemuBuildCommandLine. Additionally, since the secret is not longer
added as part of building the command, the hotplug code will need
to make the call to add the secret in the hostdevPriv.
Since this then is the last requirement to pass a virConnectPtr
to qemuBuildCommandLine, we now can remove that as part of these
changes. That removal has cascading effects through various callers.
Signed-off-by: John Ferlan
---
src/qemu/qemu_command.c | 41 +--
src/qemu/qemu_command.h | 8 ++
src/qemu/qemu_domain.c | 73 +
src/qemu/qemu_domain.h | 7 +
src/qemu/qemu_driver.c | 3 +-
src/qemu/qemu_hotplug.c | 31 +
src/qemu/qemu_hotplug.h | 3 +-
src/qemu/qemu_process.c | 5 ++--
8 files changed, 120 insertions(+), 51 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 24ed8ed..9ecb7b6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -50,7 +50,6 @@
#include "secret_conf.h"
#include "network/bridge_driver.h"
#include "virnetdevtap.h"
-#include "secret_util.h"
#include "device_conf.h"
#include "virstoragefile.h"
#include "virtpm.h"
@@ -4460,29 +4459,24 @@ qemuBuildSCSIHostHostdevDrvStr(virDomainHostdevDefPtr
dev)
}
static char *
-qemuBuildSCSIiSCSIHostdevDrvStr(virConnectPtr conn,
-virDomainHostdevDefPtr dev)
+qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev)
{
char *source = NULL;
char *secret = NULL;
char *username = NULL;
virStorageSource src;
+qemuDomainHostdevPrivatePtr hostdevPriv = QEMU_DOMAIN_HOSTDEV_PRIVATE(dev);
memset(&src, 0, sizeof(src));
virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi;
virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi;
-if (conn && iscsisrc->auth) {
-const char *protocol =
-virStorageNetProtocolTypeToString(VIR_STORAGE_NET_PROTOCOL_ISCSI);
-bool encode = false;
-int secretType = VIR_SECRET_USAGE_TYPE_ISCSI;
+if (hostdevPriv->secinfo) {
+qemuDomainSecretInfoPtr secinfo = hostdevPriv->secinfo;
-username = iscsisrc->auth->username;
-if (!(secret = virSecretGetSecretString(conn, protocol, encode,
-iscsisrc->auth, secretType)))
-goto cleanup;
+username = secinfo->s.plain.username;
+secret = secinfo->s.plain.secret;
}
src.protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI;
@@ -4493,14 +4487,11 @@ qemuBuildSCSIiSCSIHostdevDrvStr(virConnectPtr conn,
/* Rather than pull what we think we want - use the network disk code */
source = qemuBuildNetworkDriveURI(&src, username, secret);
- cleanup:
-VIR_FREE(secret);
return source;
}
char *
-qemuBuildSCSIHostdevDrvStr(virConnectPtr conn,
- virDomainHostdevDefPtr dev,
+qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
virQEMUCapsPtr qemuCaps)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -4508,7 +4499,7 @@ qemuBuildSCSIHostdevDrvStr(virConnectPtr conn,
virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi;
if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) {
-if (!(source = qemuBuildSCSIiSCSIHostdevDrvStr(conn, dev)))
+if (!(source = qemuBuildSCSIiSCSIHostdevDrvStr(dev)))
goto error;
virBufferAsprintf(&buf, "file=%s,if=none,format=raw", source);
} else {
@@ -4806,7 +4797,6 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
static int
qemuBuildHostdevCommandLine(virCommandPtr cmd,
-virConnectPtr conn,
const virDomainDef *def,
virQEMUCapsPtr qemuCaps,
unsigned int *bootHostdevNet)
@@ -4955,8 +4945,7 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd,
char *drvstr;
virCommandAddArg(cmd, "-drive");
-if (!(drvstr = qemuBuildSCSIHostdevDrvStr(conn, hostdev,
- qemuCaps)))
+if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev, qemuCaps)))
return -1;
virCommandAddArg(cmd, drvstr);
VIR_FREE(drvstr);
@@ -9193,13 +9182,9 @@ qemuBuildCommandLineValidate(virQEMUDriverPtr driver,
/*
* Constructs a argv suitable for launching qemu with config defined
* for a given virtual machine.
- *
- * XXX 'conn' is only required to resolve network -> bridge name
- * figure out how to remove this requirement some day
*/
virCommandPtr
-qemuBuildCommandLine(virConnectPtr conn,
- virQEMUDriverPtr driver,
+qemuBuildCommandLine(virQEMUDriverPt
[libvirt] [PATCH v2 05/12] qemu: Use qemuDomainSecretInfoPtr in qemuBuildNetworkDriveURI
Rather than take username and password as parameters, now take
a qemuDomainSecretInfoPtr and decode within the function.
NB: Having secinfo implies having the username for a plain type
from a successful virSecretGetSecretString
Signed-off-by: John Ferlan
---
src/qemu/qemu_command.c | 40 +---
1 file changed, 13 insertions(+), 27 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9ecb7b6..8629c15 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -610,8 +610,7 @@ qemuNetworkDriveGetPort(int protocol,
static char *
qemuBuildNetworkDriveURI(virStorageSourcePtr src,
- const char *username,
- const char *secret)
+ qemuDomainSecretInfoPtr secinfo)
{
char *ret = NULL;
virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -722,12 +721,14 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
virAsprintf(&uri->query, "socket=%s", src->hosts->socket) < 0)
goto cleanup;
-if (username) {
-if (secret) {
-if (virAsprintf(&uri->user, "%s:%s", username, secret) < 0)
+if (secinfo) {
+if (secinfo->s.plain.secret) {
+if (virAsprintf(&uri->user, "%s:%s",
+secinfo->s.plain.username,
+secinfo->s.plain.secret) < 0)
goto cleanup;
} else {
-if (VIR_STRDUP(uri->user, username) < 0)
+if (VIR_STRDUP(uri->user, secinfo->s.plain.username) < 0)
goto cleanup;
}
}
@@ -776,11 +777,12 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
if (src->snapshot)
virBufferEscape(&buf, '\\', ":", "@%s", src->snapshot);
-if (username) {
-virBufferEscape(&buf, '\\', ":", ":id=%s", username);
+if (secinfo) {
+virBufferEscape(&buf, '\\', ":", ":id=%s",
+secinfo->s.plain.username);
virBufferEscape(&buf, '\\', ":",
":key=%s:auth_supported=cephx\\;none",
-secret);
+secinfo->s.plain.secret);
} else {
virBufferAddLit(&buf, ":auth_supported=none");
}
@@ -835,8 +837,6 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
char **source)
{
int actualType = virStorageSourceGetActualType(src);
-char *secret = NULL;
-char *username = NULL;
int ret = -1;
*source = NULL;
@@ -855,12 +855,7 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
break;
case VIR_STORAGE_TYPE_NETWORK:
-if (secinfo) {
-username = secinfo->s.plain.username;
-secret = secinfo->s.plain.secret;
-}
-
-if (!(*source = qemuBuildNetworkDriveURI(src, username, secret)))
+if (!(*source = qemuBuildNetworkDriveURI(src, secinfo)))
goto cleanup;
break;
@@ -4462,8 +4457,6 @@ static char *
qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev)
{
char *source = NULL;
-char *secret = NULL;
-char *username = NULL;
virStorageSource src;
qemuDomainHostdevPrivatePtr hostdevPriv = QEMU_DOMAIN_HOSTDEV_PRIVATE(dev);
@@ -4472,20 +4465,13 @@ qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr
dev)
virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi;
virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi;
-if (hostdevPriv->secinfo) {
-qemuDomainSecretInfoPtr secinfo = hostdevPriv->secinfo;
-
-username = secinfo->s.plain.username;
-secret = secinfo->s.plain.secret;
-}
-
src.protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI;
src.path = iscsisrc->path;
src.hosts = iscsisrc->hosts;
src.nhosts = iscsisrc->nhosts;
/* Rather than pull what we think we want - use the network disk code */
-source = qemuBuildNetworkDriveURI(&src, username, secret);
+source = qemuBuildNetworkDriveURI(&src, hostdevPriv->secinfo);
return source;
}
--
2.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 09/12] qemu: hotplug: Adjust error path for attach hostdev scsi disk
Adjust error path logic to make it clearer how to undo the failed add.
Signed-off-by: John Ferlan
---
src/qemu/qemu_hotplug.c | 52 ++---
1 file changed, 32 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 17a70a3..c206369 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1925,6 +1925,7 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
{
int ret = -1;
qemuDomainObjPrivatePtr priv = vm->privateData;
+virErrorPtr orig_err;
virDomainControllerDefPtr cont = NULL;
char *devstr = NULL;
char *drvstr = NULL;
@@ -1984,32 +1985,24 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0)
goto cleanup;
+/* Attach the device - 2 step process */
qemuDomainObjEnterMonitor(driver, vm);
-if ((ret = qemuMonitorAddDrive(priv->mon, drvstr)) == 0) {
-if ((ret = qemuMonitorAddDevice(priv->mon, devstr)) < 0) {
-virErrorPtr orig_err = virSaveLastError();
-if (qemuMonitorDriveDel(priv->mon, drvstr) < 0)
-VIR_WARN("Unable to remove drive %s (%s) after failed "
- "qemuMonitorAddDevice",
- drvstr, devstr);
-if (orig_err) {
-virSetError(orig_err);
-virFreeError(orig_err);
-}
-}
-}
-if (qemuDomainObjExitMonitor(driver, vm) < 0) {
-ret = -1;
-goto cleanup;
-}
-virDomainAuditHostdev(vm, hostdev, "attach", ret == 0);
-if (ret < 0)
-goto cleanup;
+if (qemuMonitorAddDrive(priv->mon, drvstr) < 0)
+goto failadddrive;
+
+if (qemuMonitorAddDevice(priv->mon, devstr) < 0)
+goto failadddevice;
+
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+goto failexitmonitor;
+
+virDomainAuditHostdev(vm, hostdev, "attach", true);
vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
ret = 0;
+
cleanup:
qemuDomainSecretHostdevDestroy(hostdev);
if (ret < 0) {
@@ -2024,6 +2017,25 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
VIR_FREE(drvstr);
VIR_FREE(devstr);
return ret;
+
+ failadddevice:
+orig_err = virSaveLastError();
+if (qemuMonitorDriveDel(priv->mon, drvstr) < 0)
+VIR_WARN("Unable to remove drive %s (%s) after failed "
+ "qemuMonitorAddDevice",
+ drvstr, devstr);
+if (orig_err) {
+virSetError(orig_err);
+virFreeError(orig_err);
+}
+
+ failadddrive:
+ignore_value(qemuDomainObjExitMonitor(driver, vm));
+
+ failexitmonitor:
+virDomainAuditHostdev(vm, hostdev, "attach", false);
+
+goto cleanup;
}
--
2.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 07/12] qemu: hotplug: Adjust error path for attach scsi disk
Adjust error path logic to make it clearer how to undo the failed add.
Signed-off-by: John Ferlan
---
src/qemu/qemu_hotplug.c | 35 +++
1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 11efd7b..ae314be 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -584,29 +584,22 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0)
goto error;
+/* Attach the device - 2 step process */
qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorAddDrive(priv->mon, drivestr);
-if (ret == 0) {
-ret = qemuMonitorAddDevice(priv->mon, devstr);
-if (ret < 0) {
-VIR_WARN("qemuMonitorAddDevice failed on %s (%s)",
- drivestr, devstr);
-/* XXX should call 'drive_del' on error but this does not exist
yet */
-}
-}
+if (qemuMonitorAddDrive(priv->mon, drivestr) < 0)
+goto failadddrive;
-if (qemuDomainObjExitMonitor(driver, vm) < 0) {
-ret = -1;
-goto error;
-}
+if (qemuMonitorAddDevice(priv->mon, devstr) < 0)
+goto failadddevice;
-virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0);
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+goto failexitmonitor;
-if (ret < 0)
-goto error;
+virDomainAuditDisk(vm, NULL, disk->src, "attach", true);
virDomainDiskInsertPreAlloced(vm->def, disk);
+ret = 0;
cleanup:
qemuDomainSecretDiskDestroy(disk);
@@ -615,6 +608,16 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
virObjectUnref(cfg);
return ret;
+ failadddevice:
+/* XXX should call 'drive_del' on error but this does not exist yet */
+VIR_WARN("qemuMonitorAddDevice failed on %s (%s)", drivestr, devstr);
+
+ failadddrive:
+ignore_value(qemuDomainObjExitMonitor(driver, vm));
+
+ failexitmonitor:
+virDomainAuditDisk(vm, NULL, disk->src, "attach", false);
+
error:
ignore_value(qemuDomainPrepareDisk(driver, vm, disk, NULL, true));
goto cleanup;
--
2.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 02/12] qemu: Introduce qemuDomainSecretPrepare and Destroy
Rather than needing to pass the conn parameter to various command
line building API's, add qemuDomainSecretPrepare just prior to the
qemuProcessLaunch which calls qemuBuilCommandLine. The function
must be called after qemuProcessPrepareHost since it's expected
to eventually need the domain masterKey generated during the prepare
host call. Additionally, future patches may require device aliases
(assigned during the prepare domain call) in order to associate
the secret objects.
The qemuDomainSecretDestroy is called after the qemuProcessLaunch
finishes in order to clear and free memory used by the secrets
that were recently prepared, so they are not kept around in memory
too long.
Placing the setup here is beneficial for future patches which will
need the domain masterKey in order to generate an encrypted secret
along with an initialization vector to be saved and passed (since
the masterKey shouldn't be passed around).
Finally, since the secret is not added during command line build,
the hotplug code will need to get the secret into the private disk data.
Signed-off-by: John Ferlan
---
src/qemu/qemu_command.c | 45 ---
src/qemu/qemu_command.h | 5 +-
src/qemu/qemu_domain.c | 150 ++--
src/qemu/qemu_domain.h | 15 -
src/qemu/qemu_driver.c | 10 ++--
src/qemu/qemu_hotplug.c | 26 +
src/qemu/qemu_hotplug.h | 1 -
src/qemu/qemu_process.c | 8 +++
8 files changed, 202 insertions(+), 58 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 26c19ff..24ed8ed 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -832,7 +832,7 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
int
qemuGetDriveSourceString(virStorageSourcePtr src,
- virConnectPtr conn,
+ qemuDomainSecretInfoPtr secinfo,
char **source)
{
int actualType = virStorageSourceGetActualType(src);
@@ -846,31 +846,6 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
if (virStorageSourceIsEmpty(src))
return 1;
-if (conn) {
-if (actualType == VIR_STORAGE_TYPE_NETWORK &&
-src->auth &&
-(src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI ||
- src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) {
-bool encode = false;
-int secretType = VIR_SECRET_USAGE_TYPE_ISCSI;
-const char *protocol =
virStorageNetProtocolTypeToString(src->protocol);
-username = src->auth->username;
-
-if (src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD) {
-/* qemu requires the secret to be encoded for RBD */
-encode = true;
-secretType = VIR_SECRET_USAGE_TYPE_CEPH;
-}
-
-if (!(secret = virSecretGetSecretString(conn,
-protocol,
-encode,
-src->auth,
-secretType)))
-goto cleanup;
-}
-}
-
switch ((virStorageType) actualType) {
case VIR_STORAGE_TYPE_BLOCK:
case VIR_STORAGE_TYPE_FILE:
@@ -881,6 +856,11 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
break;
case VIR_STORAGE_TYPE_NETWORK:
+if (secinfo) {
+username = secinfo->s.plain.username;
+secret = secinfo->s.plain.secret;
+}
+
if (!(*source = qemuBuildNetworkDriveURI(src, username, secret)))
goto cleanup;
break;
@@ -894,7 +874,6 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
ret = 0;
cleanup:
-VIR_FREE(secret);
return ret;
}
@@ -1033,8 +1012,7 @@ qemuCheckFips(void)
char *
-qemuBuildDriveStr(virConnectPtr conn,
- virDomainDiskDefPtr disk,
+qemuBuildDriveStr(virDomainDiskDefPtr disk,
bool bootable,
virQEMUCapsPtr qemuCaps)
{
@@ -1046,6 +1024,7 @@ qemuBuildDriveStr(virConnectPtr conn,
int busid = -1, unitid = -1;
char *source = NULL;
int actualType = virStorageSourceGetActualType(disk->src);
+qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
if (idx < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -1127,7 +1106,7 @@ qemuBuildDriveStr(virConnectPtr conn,
break;
}
-if (qemuGetDriveSourceString(disk->src, conn, &source) < 0)
+if (qemuGetDriveSourceString(disk->src, diskPriv->secinfo, &source) < 0)
goto error;
if (source &&
@@ -1816,7 +1795,6 @@ qemuBuildDriveDevStr(const virDomainDef *def,
static int
qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
- virConnectPtr conn,
const virDomainDef *def,
virQEMUCapsPtr qemuCaps,
[libvirt] [PATCH v2 10/12] qemu: hotplug: Fix possible memory leak of props
If we failed to build the aliases or attach the chardev, then the props
would be leaked - fix that.
Signed-off-by: John Ferlan
---
src/qemu/qemu_hotplug.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index c206369..c05f88c 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1686,6 +1686,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
if (qemuMonitorAddObject(priv->mon, type, objAlias, props) < 0)
goto failbackend;
+props = NULL;
if (qemuMonitorAddDevice(priv->mon, devstr) < 0)
goto failfrontend;
@@ -1703,6 +1704,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
audit:
virDomainAuditRNG(vm, NULL, rng, "attach", ret == 0);
cleanup:
+virJSONValueFree(props);
if (ret < 0 && vm)
qemuDomainReleaseDeviceAddress(vm, &rng->info, NULL);
VIR_FREE(charAlias);
@@ -1716,6 +1718,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
failbackend:
if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD)
ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias));
+props = NULL; /* qemuMonitorAddObject consumes on failure */
failchardev:
if (qemuDomainObjExitMonitor(driver, vm) < 0) {
vm = NULL;
--
2.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 01/12] qemu: Introduce qemuDomainSecretInfo
Introduce a new private structure to hold qemu domain auth/secret data.
This will be stored in the qemuDomainDiskPrivate as a means to store the
auth and fetched secret data rather than generating during building of
the command line.
The initial changes will handle the current username and secret values
for rbd and iscsi disks (in their various forms). The rbd secret is
stored as a base64 encoded value, while the iscsi secret is stored as
a plain text value. Future changes will store encoded/encrypted secret
data as well as an initialization vector needed to be given to qemu
in order to decrypt the encoded password along with the domain masterKey.
The inital assumption will be that VIR_DOMAIN_SECRET_INFO_PLAIN is
being used.
Although it's expected that the cleanup of the secret data will be
done immediately after command line generation, reintroduce the object
dispose function qemuDomainDiskPrivateDispose to handle removing
memory associated with the structure for "normal" cleanup paths.
Signed-off-by: John Ferlan
---
src/qemu/qemu_domain.c | 32 +++-
src/qemu/qemu_domain.h | 27 +++
2 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ed1e0e5..55cb30f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -729,7 +729,28 @@ qemuDomainMasterKeyCreate(virQEMUDriverPtr driver,
}
+static void
+qemuDomainSecretPlainFree(qemuDomainSecretPlain secret)
+{
+VIR_FREE(secret.username);
+memset(secret.secret, 0, strlen(secret.secret));
+VIR_FREE(secret.secret);
+}
+
+
+static void
+qemuDomainSecretInfoFree(qemuDomainSecretInfoPtr *secinfo)
+{
+if (!*secinfo)
+return;
+
+qemuDomainSecretPlainFree((*secinfo)->s.plain);
+VIR_FREE(*secinfo);
+}
+
+
static virClassPtr qemuDomainDiskPrivateClass;
+static void qemuDomainDiskPrivateDispose(void *obj);
static int
qemuDomainDiskPrivateOnceInit(void)
@@ -737,7 +758,7 @@ qemuDomainDiskPrivateOnceInit(void)
qemuDomainDiskPrivateClass = virClassNew(virClassForObject(),
"qemuDomainDiskPrivate",
sizeof(qemuDomainDiskPrivate),
- NULL);
+ qemuDomainDiskPrivateDispose);
if (!qemuDomainDiskPrivateClass)
return -1;
else
@@ -761,6 +782,15 @@ qemuDomainDiskPrivateNew(void)
}
+static void
+qemuDomainDiskPrivateDispose(void *obj)
+{
+qemuDomainDiskPrivatePtr priv = obj;
+
+qemuDomainSecretInfoFree(&priv->secinfo);
+}
+
+
/* This is the old way of setting up per-domain directories */
static int
qemuDomainSetPrivatePathsOld(virQEMUDriverPtr driver,
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 7d2c4fd..9cfe3e4 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -239,6 +239,29 @@ struct _qemuDomainObjPrivate {
size_t masterKeyLen;
};
+/* Type of domain secret */
+typedef enum {
+VIR_DOMAIN_SECRET_INFO_PLAIN = 0,
+
+VIR_DOMAIN_SECRET_INFO_LAST
+} qemuDomainSecretInfoType;
+
+typedef struct _qemuDomainSecretPlain qemuDomainSecretPlain;
+typedef struct _qemuDomainSecretPlain *qemuDomainSecretPlainPtr;
+struct _qemuDomainSecretPlain {
+char *username;
+char *secret;
+};
+
+typedef struct _qemuDomainSecretInfo qemuDomainSecretInfo;
+typedef qemuDomainSecretInfo *qemuDomainSecretInfoPtr;
+struct _qemuDomainSecretInfo {
+int type; /* qemuDomainSecretInfoType */
+union {
+qemuDomainSecretPlain plain;
+} s;
+};
+
# define QEMU_DOMAIN_DISK_PRIVATE(disk)\
((qemuDomainDiskPrivatePtr) (disk)->privateData)
@@ -258,6 +281,10 @@ struct _qemuDomainDiskPrivate {
bool blockJobSync; /* the block job needs synchronized termination */
bool migrating; /* the disk is being migrated */
+
+/* for storage devices using auth/secret
+ * NB: *not* to be written to qemu domain object XML */
+qemuDomainSecretInfoPtr secinfo;
};
typedef enum {
--
2.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 00/12] Add IV Secret Object support
v1: http://www.redhat.com/archives/libvir-list/2016-April/msg00596.html Differences since v1: - Add qemuBuildiSCSICommandLine (and BuildDiskiSCSI && BuildHostdeviSCSI) These will do the magic necessary in order to support IV secret objects for the impending iSCSI -drive argument. This API doesn't require any qemu patches in order to work AFAICT. I also determined that the "id=" *isn't* required for an '-iscsi ...' argument, which made using the complete 'path' string for 'initiator-name' possible. The other option was to break it up and pass the "iqn.*" string as the initiator-name and a "modified" remaining string as the "id=" parameter. The modified would be to ensure only alphanumeric, '-', '.', and '_' characters are in the 'id=' string. - Fix up some logic found while actually working through the tests. Some of it related to what was found for the 'iscsi' options. A couple of other minor nits. - Add tests and mocks for virRandomBytes and gnutls_rnd (note: the former could be used to "randomly" (hah!) generate a UUID of all '0xff'). A mock of 'gnutls_encrypt' is not necessary since, it can only be called if the function gnutls_encrypt exists *and* we have a secret object capability. Not having a mock function allows us to validate that gnutls_encrypt actually generates a value we expect based on some less than stellar and totally non random key's! - Remove the hotplug IV code (I've saved it off for future expansion). Although not needing to do hotplug probably means patches 6-9 are not required, but still I think better than the existing so I kept them even though they have nothing to do with IV secrets (they'd need to go in after patches 1-5 anyways). - Ran the changes through the coverity checker... John Ferlan (12): qemu: Introduce qemuDomainSecretInfo qemu: Introduce qemuDomainSecretPrepare and Destroy qemu: Introduce qemuDomainHostdevPrivatePtr qemu: Introduce qemuDomainSecretHostdevPrepare and Destroy qemu: Use qemuDomainSecretInfoPtr in qemuBuildNetworkDriveURI qemu: hotplug: Assume support for -device for attach virtio disk qemu: hotplug: Adjust error path for attach scsi disk qemu: hotplug: Adjust error path for attach virtio disk qemu: hotplug: Adjust error path for attach hostdev scsi disk qemu: hotplug: Fix possible memory leak of props qemu: Introduce qemuDomainSecretIV qemu: Utilize qemu secret objects for SCSI/RBD auth/secret configure.ac | 1 + src/conf/domain_conf.c | 33 +- src/conf/domain_conf.h | 5 +- src/lxc/lxc_native.c | 4 +- src/qemu/qemu_alias.c | 23 + src/qemu/qemu_alias.h | 2 + src/qemu/qemu_command.c| 445 ++ src/qemu/qemu_command.h| 13 +- src/qemu/qemu_domain.c | 516 - src/qemu/qemu_domain.h | 81 +++- src/qemu/qemu_driver.c | 13 +- src/qemu/qemu_hotplug.c| 247 +- src/qemu/qemu_hotplug.h| 4 +- src/qemu/qemu_parse_command.c | 4 +- src/qemu/qemu_process.c| 13 +- src/vbox/vbox_common.c | 4 +- src/xenconfig/xen_common.c | 4 +- src/xenconfig/xen_sxpr.c | 4 +- ...uxml2argv-disk-drive-network-iscsi-auth-IV.args | 39 ++ ...muxml2argv-disk-drive-network-iscsi-auth-IV.xml | 43 ++ ...emuxml2argv-disk-drive-network-rbd-auth-IV.args | 31 ++ ...qemuxml2argv-disk-drive-network-rbd-auth-IV.xml | 42 ++ ...emuxml2argv-hostdev-scsi-lsi-iscsi-auth-IV.args | 41 ++ ...qemuxml2argv-hostdev-scsi-lsi-iscsi-auth-IV.xml | 48 ++ ...xml2argv-hostdev-scsi-virtio-iscsi-auth-IV.args | 43 ++ ...uxml2argv-hostdev-scsi-virtio-iscsi-auth-IV.xml | 48 ++ tests/qemuxml2argvmock.c | 31 +- tests/qemuxml2argvtest.c | 19 + tests/virhostdevtest.c | 3 +- 29 files changed, 1557 insertions(+), 247 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-IV.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth-IV.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-IV.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth-IV.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth-IV.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth-IV.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-au
Re: [libvirt] [PATCH] qemu: reject min_guarantee at parse time
On Fri, Apr 15, 2016 at 05:37:17PM -0400, Cole Robinson wrote:
min_guarantee isn't implemented for qemu, and an explicit check was
added in june 2014 to reject the VM at qemu startup time. It's a weird
place to do XML validation, so move it to the post parse area where
we have similar checks.
NACK, it's done precisely there because if there is any domain that has
that parameter already in, it will disappear after upgrade to libvirt
with this patch. That's what we have that validation function for and
the reason why it is called when the domain is being started.
We have bunch of similar issues that I wanted to address globally, but
it doesn't look like it will work in near future.
Also drop a validation check against min_guarantee in the command line
building; it will never see min_guarantee there.
---
src/qemu/qemu_command.c | 1 -
src/qemu/qemu_domain.c | 7 +++
src/qemu/qemu_process.c | 7 ---
3 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 26c19ff..2fb967a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9143,7 +9143,6 @@ qemuBuildCommandLineValidate(virQEMUDriverPtr driver,
if (virMemoryLimitIsSet(def->mem.hard_limit) ||
virMemoryLimitIsSet(def->mem.soft_limit) ||
-def->mem.min_guarantee ||
virMemoryLimitIsSet(def->mem.swap_hard_limit)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Memory tuning is not available in session mode"));
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 73187ce..f5fafc8 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1690,6 +1690,13 @@ qemuDomainDefPostParse(virDomainDefPtr def,
return ret;
}
+if (def->mem.min_guarantee) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Parameter 'min_guarantee' "
+ "not supported by QEMU."));
+return -1;
+}
+
if (def->os.loader &&
def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH &&
def->os.loader->readonly == VIR_TRISTATE_SWITCH_ON &&
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 81d86c2..387aff5 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4550,13 +4550,6 @@ qemuProcessStartValidate(virQEMUDriverPtr driver,
virDomainDefCheckDuplicateDiskInfo(vm->def) < 0)
return -1;
-if (vm->def->mem.min_guarantee) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Parameter 'min_guarantee' "
- "not supported by QEMU."));
-return -1;
-}
-
VIR_DEBUG("Checking for any possible (non-fatal) issues");
/*
--
2.7.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] tests: Fix syntax in iSCSI auth/secret tests
While working on the tests for the secret initialization vector, I found that the existing iSCSI tests were lacking in how they defined the IQN. Many had IQN's of just 'iqn.1992-01.com.example' for one disk while using 'iqn.1992-01.com.example/1' for the second disk (same for hostdevs - guess how they were copied/generated). Typically (and documented this way), IQN's would include be of the form 'iqn.1992-01.com.example:storage/1' indicating an IQN using "storage" for naming authority specific string and "/1" for the iSCSI LUN. So modify the input XML's to use the more proper format - this of course has a ripple effect on the output XML and the args. Also note that the "%3A" is generated by the virURIFormat/xmlSaveUri to represent the colon. Signed-off-by: John Ferlan --- .../qemuargv2xml-disk-drive-network-iscsi-auth.args | 6 +++--- .../qemuargv2xml-disk-drive-network-iscsi-auth.xml| 4 ++-- .../qemuxml2argv-disk-drive-network-iscsi-auth.args | 8 +--- .../qemuxml2argv-disk-drive-network-iscsi-auth.xml| 7 +-- .../qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args | 4 ++-- .../qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.xml | 4 ++-- .../qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.args | 4 ++-- .../qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.xml | 4 ++-- .../qemuxml2xmlout-disk-drive-network-iscsi-auth.xml | 7 +-- .../qemuxml2xmlout-hostdev-scsi-lsi-iscsi-auth.xml| 4 ++-- .../qemuxml2xmlout-hostdev-scsi-virtio-iscsi-auth.xml | 4 ++-- 11 files changed, 32 insertions(+), 24 deletions(-) diff --git a/tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-iscsi-auth.args b/tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-iscsi-auth.args index 5bebcae..44c9506 100644 --- a/tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-iscsi-auth.args +++ b/tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-iscsi-auth.args @@ -17,9 +17,9 @@ QEMU_AUDIO_DRV=none \ -boot c \ -usb \ -drive file=iscsi://myname:aqcvn5ho6hzfahaaq0ncv8jtjcice+hoblm...@example.org:\ -6000/iqn.1992-01.com.example,format=raw,if=virtio \ --drive file=iscsi://example.org:6000/iqn.1992-01.com.example/1,format=raw,\ -if=virtio \ +6000/iqn.1992-01.com.example%3Astorage/1,format=raw,if=virtio \ +-drive file=iscsi://example.org:6000/iqn.1992-01.com.example%3Astorage/2,\ +format=raw,if=virtio \ -net none \ -serial none \ -parallel none diff --git a/tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-iscsi-auth.xml b/tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-iscsi-auth.xml index 35b3abc..b5f948b 100644 --- a/tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-iscsi-auth.xml +++ b/tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-iscsi-auth.xml @@ -19,14 +19,14 @@ - + - + diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth.args index 735a0ae..66e2497 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth.args @@ -18,10 +18,12 @@ QEMU_AUDIO_DRV=none \ -boot c \ -usb \ -drive file=iscsi://myname:aqcvn5ho6hzfahaaq0ncv8jtjcice+hoblm...@example.org:\ -6000/iqn.1992-01.com.example,format=raw,if=none,id=drive-virtio-disk0 \ +6000/iqn.1992-01.com.example%3Astorage/1,format=raw,if=none,\ +id=drive-virtio-disk0 \ -device virtio-blk-pci,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,\ id=virtio-disk0 \ --drive file=iscsi://example.org:6000/iqn.1992-01.com.example/1,format=raw,\ -if=none,id=drive-virtio-disk1 \ +-drive file=iscsi://myname:aqcvn5ho6hzfahaaq0ncv8jtjcice+hoblm...@example.org:\ +6000/iqn.1992-01.com.example%3Astorage/2,format=raw,if=none,\ +id=drive-virtio-disk1 \ -device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk1,\ id=virtio-disk1 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth.xml index 5ac4abf..1f80d3b 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-auth.xml @@ -19,14 +19,17 @@ - + - + + + + diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args index 2bd98e5..ff6655f 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args @@ -21,9 +21,9 @@ QEMU_AUDIO_DRV=
Re: [libvirt] RFC: virtio-rng and /dev/urandom
> Also, I do not believe /dev/urandom is FIPS compliant. Finally, the refill > policy is different, so it is not really true the algorithm is the same. > > All in all, other than a seed value it really doesn't make any sense. Of > course, none of this matters on newer Intel hardware ;) Right, but there's always the point about people that use heterogeneous hosts and cannot pass rdrand/rdseed to the guest. For these, we should add a QEMU driver that uses rdrand/rdseed, and thus decouples virtio-rng from the host /dev/* completely. >From the libvirt POV there are various possibilities: - Libvirt can have a libvirt.conf parameter that says "ignore whatever is specified in the guest XML if rdrand/rdseed is available, and instead use rdrand/rdseed". - Libvirt can allow specifying rdrand/rdseed _and_ an additional backend, like this: /dev/random and fallback to the second if rdrand/rdseed are not available. Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
