[libvirt] [PATCH 0/3] Add hotplug/unplug for RBD disk with AES secret
Since AES secrets were setup for RBD during qemuDomainSecretDiskPrepare, the secret object needs to be added to the command line when hotplugging. Additionally, when unplugging, if there's an AES secret, we need to remove it. John Ferlan (3): qemu: Remove type from qemuBuildSecretInfoProps qemu: Make qemuBuildSecretInfoProps global qemu: Add secinfo for hotplug virtio disk src/qemu/qemu_command.c | 11 +++-------- src/qemu/qemu_command.h | 4 ++++ src/qemu/qemu_hotplug.c | 44 ++++++++++++++++++++++++++++++++++++++------ 3 files changed, 45 insertions(+), 14 deletions(-) -- 2.5.5
It's just a constant "secret" string anyway Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6944129..18d7ba2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -510,7 +510,6 @@ 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 * * Build the JSON properties for the secret info type. @@ -520,14 +519,11 @@ qemuNetworkDriveGetPort(int protocol, */ static int qemuBuildSecretInfoProps(qemuDomainSecretInfoPtr secinfo, - const char **type, virJSONValuePtr *propsret) { int ret = -1; char *keyid = NULL; - *type = "secret"; - if (!(keyid = qemuDomainGetMasterKeyAlias())) return -1; @@ -565,13 +561,12 @@ qemuBuildObjectSecretCommandLine(virCommandPtr cmd, { int ret = -1; virJSONValuePtr props = NULL; - const char *type; char *tmp = NULL; - if (qemuBuildSecretInfoProps(secinfo, &type, &props) < 0) + if (qemuBuildSecretInfoProps(secinfo, &props) < 0) return -1; - if (!(tmp = virQEMUBuildObjectCommandlineFromJSON(type, + if (!(tmp = virQEMUBuildObjectCommandlineFromJSON("secret", secinfo->s.aes.alias, props))) goto cleanup; -- 2.5.5
On Wed, Jun 22, 2016 at 07:46:31 -0400, John Ferlan wrote:
It's just a constant "secret" string anyway
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)
ACK
Need to create the object for a hotplug disk Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_command.h | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 18d7ba2..6ae545c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -517,7 +517,7 @@ qemuNetworkDriveGetPort(int protocol, * Returns 0 on success with the filled in JSON property; otherwise, * returns -1 on failure error message set. */ -static int +int qemuBuildSecretInfoProps(qemuDomainSecretInfoPtr secinfo, virJSONValuePtr *propsret) { diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 9ff4edb..c4d0567 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -61,6 +61,10 @@ virCommandPtr qemuBuildCommandLine(virQEMUDriverPtr driver, const char *domainLibDir) ATTRIBUTE_NONNULL(15); +/* Generate the object properties for a secret */ +int qemuBuildSecretInfoProps(qemuDomainSecretInfoPtr secinfo, + virJSONValuePtr *propsret); + /* Generate '-device' string for chardev device */ int qemuBuildChrDeviceStr(char **deviceStr, -- 2.5.5
On Wed, Jun 22, 2016 at 07:46:32 -0400, John Ferlan wrote:
Need to create the object for a hotplug disk
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_command.h | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-)
ACK
Commit id 'a1344f70a' added AES secret processing for RBD when starting up a guest. As such, when the hotplug code calls qemuDomainSecretDiskPrepare an AES secret could be added to the disk about to be hotplugged. If an AES secret was added, then the hotplug code would need to generate the secret object because qemuBuildDriveStr would add the "password-secret=" to the returned 'driveStr' rather than the base64 encoded password. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_hotplug.c | 44 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f695903..a85467f 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -310,6 +310,9 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, bool releaseaddr = false; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); const char *src = virDomainDiskGetSource(disk); + virJSONValuePtr objProps = NULL; + qemuDomainDiskPrivatePtr diskPriv; + qemuDomainSecretInfoPtr secinfo; if (!disk->info.type) { if (qemuDomainMachineIsS390CCW(vm->def) && @@ -342,6 +345,13 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, if (qemuDomainSecretDiskPrepare(conn, priv, disk) < 0) goto error; + diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + secinfo = diskPriv->secinfo; + if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) { + if (qemuBuildSecretInfoProps(secinfo, &objProps) < 0) + goto error; + } + if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps))) goto error; @@ -354,9 +364,15 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0) goto error; - /* Attach the device - 2 step process */ + /* Attach the device - possible 3 step process */ qemuDomainObjEnterMonitor(driver, vm); + if (objProps && qemuMonitorAddObject(priv->mon, "secret", + secinfo->s.aes.alias, + objProps) < 0) + goto failaddsecret; + objProps = NULL; + if (qemuMonitorAddDrive(priv->mon, drivestr) < 0) goto failadddrive; @@ -374,6 +390,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, ret = 0; cleanup: + virJSONValueFree(objProps); qemuDomainSecretDiskDestroy(disk); VIR_FREE(devstr); VIR_FREE(drivestr); @@ -393,8 +410,13 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, } failadddrive: + if (objProps) + ignore_value(qemuMonitorDelObject(priv->mon, secinfo->s.aes.alias)); + + failaddsecret: if (qemuDomainObjExitMonitor(driver, vm) < 0) releaseaddr = false; + objProps = NULL; /* qemuMonitorAddObject consumes props on failure too */ failexitmonitor: virDomainAuditDisk(vm, NULL, disk->src, "attach", false); @@ -3389,6 +3411,8 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, { int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; + qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(detach); + qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo; if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) { virReportError(VIR_ERR_OPERATION_FAILED, @@ -3422,12 +3446,14 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, qemuDomainMarkDeviceForRemoval(vm, &detach->info); qemuDomainObjEnterMonitor(driver, vm); - if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { - if (qemuDomainObjExitMonitor(driver, vm) < 0) - goto cleanup; - virDomainAuditDisk(vm, detach->src, NULL, "detach", false); - goto cleanup; + if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) { + if (qemuMonitorDelObject(priv->mon, secinfo->s.aes.alias) < 0) + goto faildel; } + + if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) + goto faildel; + if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup; @@ -3437,6 +3463,12 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, cleanup: qemuDomainResetDeviceRemoval(vm); return ret; + + faildel: + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; + virDomainAuditDisk(vm, detach->src, NULL, "detach", false); + goto cleanup; } static int -- 2.5.5
On Wed, Jun 22, 2016 at 07:46:33 -0400, John Ferlan wrote:
Commit id 'a1344f70a' added AES secret processing for RBD when starting up a guest. As such, when the hotplug code calls qemuDomainSecretDiskPrepare an AES secret could be added to the disk about to be hotplugged. If an AES secret was added, then the hotplug code would need to generate the secret object because qemuBuildDriveStr would add the "password-secret=" to the returned 'driveStr' rather than the base64 encoded password.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_hotplug.c | 44 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 38 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f695903..a85467f 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c
[...]
@@ -3422,12 +3446,14 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, qemuDomainMarkDeviceForRemoval(vm, &detach->info);
qemuDomainObjEnterMonitor(driver, vm); - if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { - if (qemuDomainObjExitMonitor(driver, vm) < 0) - goto cleanup; - virDomainAuditDisk(vm, detach->src, NULL, "detach", false); - goto cleanup; + if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) {
This won't be initialized if you restart the daemon and thus the secret object would not be deleted in such case. To make it a bit worse, you can't call qemuDomainSecretPrepare since the secrets may be missing and are not really needed at this point. You need though generate the correct alias and use it in such case.
+ if (qemuMonitorDelObject(priv->mon, secinfo->s.aes.alias) < 0) + goto faildel; } + + if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) + goto faildel; + if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup;
Peter
participants (2)
-
John Ferlan -
Peter Krempa