On Tue, Nov 03, 2009 at 02:50:00PM -0500, Daniel P. Berrange wrote:
> Decouple the monitor code from the virDomainDefPtr structure
> by moving the disk encryption lookup code back into the
> qemu_driver.c file. Instead provide a function callback to
> the monitor code which can be invoked to retrieve encryption
> data as required.
>
> * src/qemu/qemu_driver.c: Add findDomainDiskEncryption,
> and findVolumeQcowPassphrase. Pass address of the method
> findVolumeQcowPassphrase into qemuMonitorOpen()
> * src/qemu/qemu_monitor.c: Associate a disk
> encryption function callback with the qemuMonitorPtr
> object.
> * src/qemu/qemu_monitor_text.c: Remove findDomainDiskEncryption
> and findVolumeQcowPassphrase.
> ---
> src/qemu/qemu_driver.c | 110 ++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_monitor.c | 22 ++++++++
> src/qemu/qemu_monitor.h | 20 ++++++++
> src/qemu/qemu_monitor_text.c | 103 ++-------------------------------------
> 4 files changed, 157 insertions(+), 98 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index e985543..30cc66e 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -348,6 +348,113 @@ qemuHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
> }
>
>
> +static virStorageEncryptionPtr
> +findDomainDiskEncryption(virConnectPtr conn, virDomainObjPtr vm,
> + const char *path)
> +{
> + bool seen_volume;
> + int i;
> +
> + seen_volume = false;
> + for (i = 0; i < vm->def->ndisks; i++) {
> + virDomainDiskDefPtr disk;
> +
> + disk = vm->def->disks[i];
> + if (disk->src != NULL && STREQ(disk->src, path)) {
> + seen_volume = true;
> + if (disk->encryption != NULL)
> + return disk->encryption;
> + }
> + }
> + if (seen_volume)
> + qemudReportError(conn, NULL, NULL, VIR_ERR_INVALID_DOMAIN,
> + _("missing <encryption> for volume %s"), path);
> + else
> + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
> + _("unexpected passphrase request for volume %s"),
> + path);
> + return NULL;
> +}
> +
> +
> +static int
> +findVolumeQcowPassphrase(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
> + virConnectPtr conn,
> + virDomainObjPtr vm,
> + const char *path,
> + char **secretRet,
> + size_t *secretLen)
> +{
> + virStorageEncryptionPtr enc;
> + virSecretPtr secret;
> + char *passphrase;
> + unsigned char *data;
> + size_t size;
> +
> + if (!conn) {
> + qemudReportError(NULL, NULL, NULL, VIR_ERR_NO_SUPPORT,
> + "%s", _("cannot find secrets without a connection"));
> + return -1;
> + }
> +
> + if (conn->secretDriver == NULL ||
> + conn->secretDriver->lookupByUUID == NULL ||
> + conn->secretDriver->getValue == NULL) {
> + qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, "%s",
> + _("secret storage not supported"));
> + return -1;
> + }
> +
> + enc = findDomainDiskEncryption(conn, vm, path);
> + if (enc == NULL)
> + return -1;
> +
> + if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_QCOW ||
> + enc->nsecrets != 1 ||
> + enc->secrets[0]->type !=
> + VIR_STORAGE_ENCRYPTION_SECRET_TYPE_PASSPHRASE) {
> + qemudReportError(conn, NULL, NULL, VIR_ERR_INVALID_DOMAIN,
> + _("invalid <encryption> for volume %s"), path);
> + return -1;
> + }
> +
> + secret = conn->secretDriver->lookupByUUID(conn,
> + enc->secrets[0]->uuid);
> + if (secret == NULL)
> + return -1;
> + data = conn->secretDriver->getValue(secret, &size,
> + VIR_SECRET_GET_VALUE_INTERNAL_CALL);
> + virUnrefSecret(secret);
> + if (data == NULL)
> + return -1;
> +
> + if (memchr(data, '\0', size) != NULL) {
> + memset(data, 0, size);
> + VIR_FREE(data);
> + qemudReportError(conn, NULL, NULL, VIR_ERR_INVALID_SECRET,
> + _("format='qcow' passphrase for %s must not contain a "
> + "'\\0'"), path);
> + return -1;
> + }
> +
> + if (VIR_ALLOC_N(passphrase, size + 1) < 0) {
> + memset(data, 0, size);
> + VIR_FREE(data);
> + virReportOOMError(conn);
> + return -1;
> + }
> + memcpy(passphrase, data, size);
> + passphrase[size] = '\0';
> +
> + memset(data, 0, size);
> + VIR_FREE(data);
> +
> + *secretRet = passphrase;
> + *secretLen = size;
> +
> + return 0;
> +}
> +
> static int
> qemuConnectMonitor(virDomainObjPtr vm, int reconnect)
> {
> @@ -358,6 +465,9 @@ qemuConnectMonitor(virDomainObjPtr vm, int reconnect)
> return -1;
> }
>
> + qemuMonitorRegisterDiskSecretLookup(priv->mon,
> + findVolumeQcowPassphrase);
> +
> return 0;
> }
>
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 5f7e20c..352328d 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -45,6 +45,7 @@ struct _qemuMonitor {
> virDomainObjPtr vm;
>
> qemuMonitorEOFNotify eofCB;
> + qemuMonitorDiskSecretLookup secretCB;
> };
>
> /* Return -1 for error, 1 to continue reading and 0 for success */
> @@ -322,6 +323,7 @@ qemuMonitorOpen(virDomainObjPtr vm,
> goto cleanup;
> }
>
> +
> return mon;
>
> cleanup:
> @@ -344,6 +346,13 @@ void qemuMonitorClose(qemuMonitorPtr mon)
> }
>
>
> +void qemuMonitorRegisterDiskSecretLookup(qemuMonitorPtr mon,
> + qemuMonitorDiskSecretLookup secretCB)
> +{
> + mon->secretCB = secretCB;
> +}
> +
> +
> int qemuMonitorWrite(qemuMonitorPtr mon,
> const char *data,
> size_t len)
> @@ -410,3 +419,16 @@ retry:
> }
> return 0;
> }
> +
> +
> +int qemuMonitorGetDiskSecret(qemuMonitorPtr mon,
> + virConnectPtr conn,
> + const char *path,
> + char **secret,
> + size_t *secretLen)
> +{
> + *secret = NULL;
> + *secretLen = 0;
> +
> + return mon->secretCB(mon, conn, mon->vm, path, secret, secretLen);
> +}
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index e863e20..6fb99a9 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -36,12 +36,27 @@ typedef void (*qemuMonitorEOFNotify)(qemuMonitorPtr mon,
> virDomainObjPtr vm,
> int withError);
>
> +/* XXX we'd really like to avoid virCOnnectPtr here
> + * It is required so the callback can find the active
> + * secret driver. Need to change this to work like the
> + * security drivers do, to avoid this
> + */
> +typedef int (*qemuMonitorDiskSecretLookup)(qemuMonitorPtr mon,
> + virConnectPtr conn,
> + virDomainObjPtr vm,
> + const char *path,
> + char **secret,
> + size_t *secretLen);
> +
> qemuMonitorPtr qemuMonitorOpen(virDomainObjPtr vm,
> int reconnect,
> qemuMonitorEOFNotify eofCB);
>
> void qemuMonitorClose(qemuMonitorPtr mon);
>
> +void qemuMonitorRegisterDiskSecretLookup(qemuMonitorPtr mon,
> + qemuMonitorDiskSecretLookup secretCB);
> +
> int qemuMonitorWrite(qemuMonitorPtr mon,
> const char *data,
> size_t len);
> @@ -57,5 +72,10 @@ int qemuMonitorRead(qemuMonitorPtr mon,
>
> int qemuMonitorWaitForInput(qemuMonitorPtr mon);
>
> +int qemuMonitorGetDiskSecret(qemuMonitorPtr mon,
> + virConnectPtr conn,
> + const char *path,
> + char **secret,
> + size_t *secretLen);
>
> #endif /* QEMU_MONITOR_H */
> diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
> index fa17971..a0146ae 100644
> --- a/src/qemu/qemu_monitor_text.c
> +++ b/src/qemu/qemu_monitor_text.c
> @@ -334,101 +334,6 @@ qemuMonitorCommand(const virDomainObjPtr vm,
> }
>
>
> -
> -static virStorageEncryptionPtr
> -findDomainDiskEncryption(virConnectPtr conn, virDomainObjPtr vm,
> - const char *path)
> -{
> - bool seen_volume;
> - int i;
> -
> - seen_volume = false;
> - for (i = 0; i < vm->def->ndisks; i++) {
> - virDomainDiskDefPtr disk;
> -
> - disk = vm->def->disks[i];
> - if (disk->src != NULL && STREQ(disk->src, path)) {
> - seen_volume = true;
> - if (disk->encryption != NULL)
> - return disk->encryption;
> - }
> - }
> - if (seen_volume)
> - qemudReportError(conn, NULL, NULL, VIR_ERR_INVALID_DOMAIN,
> - _("missing <encryption> for volume %s"), path);
> - else
> - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
> - _("unexpected passphrase request for volume %s"),
> - path);
> - return NULL;
> -}
> -
> -static char *
> -findVolumeQcowPassphrase(virConnectPtr conn, virDomainObjPtr vm,
> - const char *path, size_t *passphrase_len)
> -{
> - virStorageEncryptionPtr enc;
> - virSecretPtr secret;
> - char *passphrase;
> - unsigned char *data;
> - size_t size;
> -
> - if (conn->secretDriver == NULL ||
> - conn->secretDriver->lookupByUUID == NULL ||
> - conn->secretDriver->getValue == NULL) {
> - qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, "%s",
> - _("secret storage not supported"));
> - return NULL;
> - }
> -
> - enc = findDomainDiskEncryption(conn, vm, path);
> - if (enc == NULL)
> - return NULL;
> -
> - if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_QCOW ||
> - enc->nsecrets != 1 ||
> - enc->secrets[0]->type !=
> - VIR_STORAGE_ENCRYPTION_SECRET_TYPE_PASSPHRASE) {
> - qemudReportError(conn, NULL, NULL, VIR_ERR_INVALID_DOMAIN,
> - _("invalid <encryption> for volume %s"), path);
> - return NULL;
> - }
> -
> - secret = conn->secretDriver->lookupByUUID(conn,
> - enc->secrets[0]->uuid);
> - if (secret == NULL)
> - return NULL;
> - data = conn->secretDriver->getValue(secret, &size,
> - VIR_SECRET_GET_VALUE_INTERNAL_CALL);
> - virUnrefSecret(secret);
> - if (data == NULL)
> - return NULL;
> -
> - if (memchr(data, '\0', size) != NULL) {
> - memset(data, 0, size);
> - VIR_FREE(data);
> - qemudReportError(conn, NULL, NULL, VIR_ERR_INVALID_SECRET,
> - _("format='qcow' passphrase for %s must not contain a "
> - "'\\0'"), path);
> - return NULL;
> - }
> -
> - if (VIR_ALLOC_N(passphrase, size + 1) < 0) {
> - memset(data, 0, size);
> - VIR_FREE(data);
> - virReportOOMError(conn);
> - return NULL;
> - }
> - memcpy(passphrase, data, size);
> - passphrase[size] = '\0';
> -
> - memset(data, 0, size);
> - VIR_FREE(data);
> -
> - *passphrase_len = size;
> - return passphrase;
> -}
> -
> static int
> qemuMonitorSendVolumePassphrase(const virDomainObjPtr vm,
> const char *buf,
> @@ -436,7 +341,8 @@ qemuMonitorSendVolumePassphrase(const virDomainObjPtr vm,
> void *data)
> {
> virConnectPtr conn = data;
> - char *passphrase, *path;
> + qemuDomainObjPrivatePtr priv = vm->privateData;
> + char *passphrase = NULL, *path;
> const char *prompt_path;
> size_t path_len, passphrase_len = 0;
> int res;
> @@ -456,9 +362,10 @@ qemuMonitorSendVolumePassphrase(const virDomainObjPtr vm,
> memcpy(path, prompt_path, path_len);
> path[path_len] = '\0';
>
> - passphrase = findVolumeQcowPassphrase(conn, vm, path, &passphrase_len);
> + res = qemuMonitorGetDiskSecret(priv->mon, conn, path,
> + &passphrase, &passphrase_len);
> VIR_FREE(path);
> - if (passphrase == NULL)
> + if (res < 0)
> return -1;
>
> res = qemuMonitorSend(vm, passphrase, -1);
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list