[libvirt] [PATCH v2 1/3] utils: Implement virCommandPassFDGetFDIndex

Implement virCommandPassFDGetFDIndex to determine the index a given file descriptor will have when passed to the child process. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/libvirt_private.syms | 1 + src/util/vircommand.c | 24 ++++++++++++++++++++++++ src/util/vircommand.h | 3 +++ 3 files changed, 28 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6028002..96cfbf2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1214,6 +1214,7 @@ virCommandNewArgList; virCommandNewArgs; virCommandNonblockingFDs; virCommandPassFD; +virCommandPassFDGetFDIndex; virCommandPassListenFDs; virCommandRawStatus; virCommandRequireHandshake; diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 6527d85..648f5ed 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -1019,6 +1019,30 @@ virCommandPassListenFDs(virCommandPtr cmd) cmd->flags |= VIR_EXEC_LISTEN_FDS; } +/* + * virCommandPassFDGetFDIndex: + * @cmd: pointer to virCommand + * @fd: FD to get index of + * + * Determine the index of the FD in the transfer set. + * + * Returns index >= 0 if @set contains @fd, + * -1 otherwise. + */ +int +virCommandPassFDGetFDIndex(virCommandPtr cmd, int fd) +{ + size_t i = 0; + + while (i < cmd->npassfd) { + if (cmd->passfd[i].fd == fd) + return i; + i++; + } + + return -1; +} + /** * virCommandSetPidFile: * @cmd: the command to modify diff --git a/src/util/vircommand.h b/src/util/vircommand.h index bf65de4..198da2f 100644 --- a/src/util/vircommand.h +++ b/src/util/vircommand.h @@ -62,6 +62,9 @@ void virCommandPassFD(virCommandPtr cmd, void virCommandPassListenFDs(virCommandPtr cmd); +int virCommandPassFDGetFDIndex(virCommandPtr cmd, + int fd); + void virCommandSetPidFile(virCommandPtr cmd, const char *pidfile) ATTRIBUTE_NONNULL(2); -- 1.9.3

Move the TPM command line build code into its own function. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c112619..d6bc294 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8191,6 +8191,30 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd, return ret; } +static int +qemuBuildTPMCommandLine(virDomainDefPtr def, + virCommandPtr cmd, + virQEMUCapsPtr qemuCaps, + const char *emulator) +{ + char *optstr; + + if (!(optstr = qemuBuildTPMBackendStr(def, qemuCaps, emulator))) + return -1; + + virCommandAddArgList(cmd, "-tpmdev", optstr, NULL); + VIR_FREE(optstr); + + if (!(optstr = qemuBuildTPMDevStr(def, qemuCaps, emulator))) + return -1; + + virCommandAddArgList(cmd, "-device", optstr, NULL); + VIR_FREE(optstr); + + return 0; +} + + qemuBuildCommandLineCallbacks buildCommandLineCallbacks = { .qemuGetSCSIDeviceSgName = virSCSIDeviceGetSgName, }; @@ -9600,19 +9624,8 @@ qemuBuildCommandLine(virConnectPtr conn, } if (def->tpm) { - char *optstr; - - if (!(optstr = qemuBuildTPMBackendStr(def, qemuCaps, emulator))) + if (qemuBuildTPMCommandLine(def, cmd, qemuCaps, emulator) < 0) goto error; - - virCommandAddArgList(cmd, "-tpmdev", optstr, NULL); - VIR_FREE(optstr); - - if (!(optstr = qemuBuildTPMDevStr(def, qemuCaps, emulator))) - goto error; - - virCommandAddArgList(cmd, "-device", optstr, NULL); - VIR_FREE(optstr); } for (i = 0; i < def->ninputs; i++) { -- 1.9.3

Pass the TPM file descriptor to QEMU via command line. Instead of passing /dev/tpm0 we now pass /dev/fdset/10 and the additional parameters -add-fd set=10,fd=20. This addresses the use case when QEMU is started with non-root privileges and QEMU cannot open /dev/tpm0 for example. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 117 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d6bc294..4c6b76d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -161,6 +161,58 @@ VIR_ENUM_IMPL(qemuNumaPolicy, VIR_DOMAIN_NUMATUNE_MEM_LAST, "interleave"); /** + * qemuVirCommandGetFDSet: + * @cmd: the command to modify + * @fd: fd to reassign to the child + * + * Get the parameters for the QEMU -add-fd command line option + * for the given file descriptor. The file descriptor must previously + * have been 'transferred' in a virCommandPassFD() call. + * This function for example returns "set=10,fd=20". + */ +static char * +qemuVirCommandGetFDSet(virCommandPtr cmd, int fd) +{ + char *result = NULL; + int idx = virCommandPassFDGetFDIndex(cmd, fd); + + if (idx >= 0) { + ignore_value(virAsprintf(&result, "set=%d,fd=%d", idx, fd)); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("file descriptor %d has not been transferred"), fd); + } + + return result; +} + +/** + * qemuVirCommandGetDevSet: + * @cmd: the command to modify + * @fd: fd to reassign to the child + * + * Get the parameters for the QEMU path= parameter where a file + * descriptor is accessed via a file descriptor set, for example + * /dev/fdset/10. The file descriptor must previously have been + * 'transferred' in a virCommandPassFD() call. + */ +static char * +qemuVirCommandGetDevSet(virCommandPtr cmd, int fd) +{ + char *result = NULL; + int idx = virCommandPassFDGetFDIndex(cmd, fd); + + if (idx >= 0) { + ignore_value(virAsprintf(&result, "/dev/fdset/%d", idx)); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("file descriptor %d has not been transferred"), fd); + } + return result; +} + + +/** * qemuPhysIfaceConnect: * @def: the definition of the VM (needed by 802.1Qbh and audit) * @driver: pointer to the driver instance @@ -6321,15 +6373,20 @@ qemuBuildRNGDevStr(virDomainDefPtr def, static char *qemuBuildTPMBackendStr(const virDomainDef *def, + virCommandPtr cmd, virQEMUCapsPtr qemuCaps, - const char *emulator) + const char *emulator, + int *tpmfd, int *cancelfd) { const virDomainTPMDef *tpm = def->tpm; virBuffer buf = VIR_BUFFER_INITIALIZER; const char *type = virDomainTPMBackendTypeToString(tpm->type); - char *cancel_path; + char *cancel_path = NULL, *devset = NULL; const char *tpmdev; + *tpmfd = -1; + *cancelfd = -1; + virBufferAsprintf(&buf, "%s,id=tpm-%s", type, tpm->info.alias); switch (tpm->type) { @@ -6341,11 +6398,42 @@ static char *qemuBuildTPMBackendStr(const virDomainDef *def, if (!(cancel_path = virTPMCreateCancelPath(tpmdev))) goto error; + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ADD_FD)) { + *tpmfd = open(tpmdev, O_RDWR); + if (*tpmfd < 0) { + virReportSystemError(errno, _("Could not open TPM device %s"), + tpmdev); + goto error; + } + + virCommandPassFD(cmd, *tpmfd, + VIR_COMMAND_PASS_FD_CLOSE_PARENT); + devset = qemuVirCommandGetDevSet(cmd, *tpmfd); + if (devset == NULL) + goto error; + + *cancelfd = open(cancel_path, O_WRONLY); + if (*cancelfd < 0) { + virReportSystemError(errno, + _("Could not open TPM device's cancel " + "path %s"), cancel_path); + goto error; + } + VIR_FREE(cancel_path); + + virCommandPassFD(cmd, *cancelfd, + VIR_COMMAND_PASS_FD_CLOSE_PARENT); + cancel_path = qemuVirCommandGetDevSet(cmd, *cancelfd); + if (cancel_path == NULL) + goto error; + } virBufferAddLit(&buf, ",path="); - virBufferEscape(&buf, ',', ",", "%s", tpmdev); + virBufferEscape(&buf, ',', ",", "%s", devset ? devset : tpmdev); virBufferAddLit(&buf, ",cancel-path="); virBufferEscape(&buf, ',', ",", "%s", cancel_path); + + VIR_FREE(devset); VIR_FREE(cancel_path); break; @@ -6365,6 +6453,9 @@ static char *qemuBuildTPMBackendStr(const virDomainDef *def, emulator, type); error: + VIR_FREE(devset); + VIR_FREE(cancel_path); + virBufferFreeAndReset(&buf); return NULL; } @@ -8198,13 +8289,35 @@ qemuBuildTPMCommandLine(virDomainDefPtr def, const char *emulator) { char *optstr; + int tpmfd = -1; + int cancelfd = -1; + char *fdset; - if (!(optstr = qemuBuildTPMBackendStr(def, qemuCaps, emulator))) + if (!(optstr = qemuBuildTPMBackendStr(def, cmd, qemuCaps, emulator, + &tpmfd, &cancelfd))) return -1; virCommandAddArgList(cmd, "-tpmdev", optstr, NULL); VIR_FREE(optstr); + if (tpmfd >= 0) { + fdset = qemuVirCommandGetFDSet(cmd, tpmfd); + if (!fdset) + return -1; + + virCommandAddArgList(cmd, "-add-fd", fdset, NULL); + VIR_FREE(fdset); + } + + if (cancelfd >= 0) { + fdset = qemuVirCommandGetFDSet(cmd, cancelfd); + if (!fdset) + return -1; + + virCommandAddArgList(cmd, "-add-fd", fdset, NULL); + VIR_FREE(fdset); + } + if (!(optstr = qemuBuildTPMDevStr(def, qemuCaps, emulator))) return -1; -- 1.9.3

On 03.03.2015 15:40, Stefan Berger wrote:
Pass the TPM file descriptor to QEMU via command line. Instead of passing /dev/tpm0 we now pass /dev/fdset/10 and the additional parameters -add-fd set=10,fd=20.
This addresses the use case when QEMU is started with non-root privileges and QEMU cannot open /dev/tpm0 for example.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 117 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d6bc294..4c6b76d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -161,6 +161,58 @@ VIR_ENUM_IMPL(qemuNumaPolicy, VIR_DOMAIN_NUMATUNE_MEM_LAST, "interleave");
/** + * qemuVirCommandGetFDSet: + * @cmd: the command to modify + * @fd: fd to reassign to the child + * + * Get the parameters for the QEMU -add-fd command line option + * for the given file descriptor. The file descriptor must previously + * have been 'transferred' in a virCommandPassFD() call. + * This function for example returns "set=10,fd=20". + */ +static char * +qemuVirCommandGetFDSet(virCommandPtr cmd, int fd) +{ + char *result = NULL; + int idx = virCommandPassFDGetFDIndex(cmd, fd); + + if (idx >= 0) { + ignore_value(virAsprintf(&result, "set=%d,fd=%d", idx, fd)); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("file descriptor %d has not been transferred"), fd); + } + + return result; +} + +/** + * qemuVirCommandGetDevSet: + * @cmd: the command to modify + * @fd: fd to reassign to the child + * + * Get the parameters for the QEMU path= parameter where a file + * descriptor is accessed via a file descriptor set, for example + * /dev/fdset/10. The file descriptor must previously have been + * 'transferred' in a virCommandPassFD() call. + */ +static char * +qemuVirCommandGetDevSet(virCommandPtr cmd, int fd) +{ + char *result = NULL; + int idx = virCommandPassFDGetFDIndex(cmd, fd); + + if (idx >= 0) { + ignore_value(virAsprintf(&result, "/dev/fdset/%d", idx)); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("file descriptor %d has not been transferred"), fd); + } + return result; +} + + +/** * qemuPhysIfaceConnect: * @def: the definition of the VM (needed by 802.1Qbh and audit) * @driver: pointer to the driver instance @@ -6321,15 +6373,20 @@ qemuBuildRNGDevStr(virDomainDefPtr def,
static char *qemuBuildTPMBackendStr(const virDomainDef *def, + virCommandPtr cmd, virQEMUCapsPtr qemuCaps, - const char *emulator) + const char *emulator, + int *tpmfd, int *cancelfd) { const virDomainTPMDef *tpm = def->tpm; virBuffer buf = VIR_BUFFER_INITIALIZER; const char *type = virDomainTPMBackendTypeToString(tpm->type); - char *cancel_path; + char *cancel_path = NULL, *devset = NULL; const char *tpmdev;
+ *tpmfd = -1; + *cancelfd = -1; + virBufferAsprintf(&buf, "%s,id=tpm-%s", type, tpm->info.alias);
switch (tpm->type) { @@ -6341,11 +6398,42 @@ static char *qemuBuildTPMBackendStr(const virDomainDef *def, if (!(cancel_path = virTPMCreateCancelPath(tpmdev))) goto error;
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ADD_FD)) { + *tpmfd = open(tpmdev, O_RDWR); + if (*tpmfd < 0) { + virReportSystemError(errno, _("Could not open TPM device %s"), + tpmdev); + goto error; + } + + virCommandPassFD(cmd, *tpmfd, + VIR_COMMAND_PASS_FD_CLOSE_PARENT); + devset = qemuVirCommandGetDevSet(cmd, *tpmfd); + if (devset == NULL) + goto error; + + *cancelfd = open(cancel_path, O_WRONLY); + if (*cancelfd < 0) { + virReportSystemError(errno, + _("Could not open TPM device's cancel " + "path %s"), cancel_path); + goto error; + } + VIR_FREE(cancel_path); + + virCommandPassFD(cmd, *cancelfd, + VIR_COMMAND_PASS_FD_CLOSE_PARENT); + cancel_path = qemuVirCommandGetDevSet(cmd, *cancelfd); + if (cancel_path == NULL) + goto error; + }
I'm thinking of how to test this code, but those open() complicate the things. Unless we want to mock them in qemuxml2argvmock.c which would not be trivial after all. ACK series. Michal

On 03/05/2015 10:25 AM, Michal Privoznik wrote:
Pass the TPM file descriptor to QEMU via command line. Instead of passing /dev/tpm0 we now pass /dev/fdset/10 and the additional parameters -add-fd set=10,fd=20.
This addresses the use case when QEMU is started with non-root privileges and QEMU cannot open /dev/tpm0 for example.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 117 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d6bc294..4c6b76d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -161,6 +161,58 @@ VIR_ENUM_IMPL(qemuNumaPolicy, VIR_DOMAIN_NUMATUNE_MEM_LAST, "interleave");
/** + * qemuVirCommandGetFDSet: + * @cmd: the command to modify + * @fd: fd to reassign to the child + * + * Get the parameters for the QEMU -add-fd command line option + * for the given file descriptor. The file descriptor must previously + * have been 'transferred' in a virCommandPassFD() call. + * This function for example returns "set=10,fd=20". + */ +static char * +qemuVirCommandGetFDSet(virCommandPtr cmd, int fd) +{ + char *result = NULL; + int idx = virCommandPassFDGetFDIndex(cmd, fd); + + if (idx >= 0) { + ignore_value(virAsprintf(&result, "set=%d,fd=%d", idx, fd)); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("file descriptor %d has not been transferred"), fd); + } + + return result; +} + +/** + * qemuVirCommandGetDevSet: + * @cmd: the command to modify + * @fd: fd to reassign to the child + * + * Get the parameters for the QEMU path= parameter where a file + * descriptor is accessed via a file descriptor set, for example + * /dev/fdset/10. The file descriptor must previously have been + * 'transferred' in a virCommandPassFD() call. + */ +static char * +qemuVirCommandGetDevSet(virCommandPtr cmd, int fd) +{ + char *result = NULL; + int idx = virCommandPassFDGetFDIndex(cmd, fd); + + if (idx >= 0) { + ignore_value(virAsprintf(&result, "/dev/fdset/%d", idx)); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("file descriptor %d has not been transferred"), fd); + } + return result; +} + + +/** * qemuPhysIfaceConnect: * @def: the definition of the VM (needed by 802.1Qbh and audit) * @driver: pointer to the driver instance @@ -6321,15 +6373,20 @@ qemuBuildRNGDevStr(virDomainDefPtr def,
static char *qemuBuildTPMBackendStr(const virDomainDef *def, + virCommandPtr cmd, virQEMUCapsPtr qemuCaps, - const char *emulator) + const char *emulator, + int *tpmfd, int *cancelfd) { const virDomainTPMDef *tpm = def->tpm; virBuffer buf = VIR_BUFFER_INITIALIZER; const char *type = virDomainTPMBackendTypeToString(tpm->type); - char *cancel_path; + char *cancel_path = NULL, *devset = NULL; const char *tpmdev;
+ *tpmfd = -1; + *cancelfd = -1; + virBufferAsprintf(&buf, "%s,id=tpm-%s", type, tpm->info.alias);
switch (tpm->type) { @@ -6341,11 +6398,42 @@ static char *qemuBuildTPMBackendStr(const virDomainDef *def, if (!(cancel_path = virTPMCreateCancelPath(tpmdev))) goto error;
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ADD_FD)) { + *tpmfd = open(tpmdev, O_RDWR); + if (*tpmfd < 0) { + virReportSystemError(errno, _("Could not open TPM device %s"), + tpmdev); + goto error; + } + + virCommandPassFD(cmd, *tpmfd, + VIR_COMMAND_PASS_FD_CLOSE_PARENT); + devset = qemuVirCommandGetDevSet(cmd, *tpmfd); + if (devset == NULL) + goto error; + + *cancelfd = open(cancel_path, O_WRONLY); + if (*cancelfd < 0) { + virReportSystemError(errno, + _("Could not open TPM device's cancel " + "path %s"), cancel_path); + goto error; + } + VIR_FREE(cancel_path); + + virCommandPassFD(cmd, *cancelfd, + VIR_COMMAND_PASS_FD_CLOSE_PARENT); + cancel_path = qemuVirCommandGetDevSet(cmd, *cancelfd); + if (cancel_path == NULL) + goto error; + } I'm thinking of how to test this code, but those open() complicate the
On 03.03.2015 15:40, Stefan Berger wrote: things. Unless we want to mock them in qemuxml2argvmock.c which would not be trivial after all.
ACK series.
Michal
Pushed Stefan
participants (2)
-
Michal Privoznik
-
Stefan Berger