External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-12-22 05:35:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

virCommandSetSendBuffer: Provide saner semantics

Browse Source 
The function is used to automatically feed a buffer into a pipe which
can be used by the command to read contents of the buffer.

Rather than passing in a pipe, let's create the pipe inside
virCommandSetSendBuffer and directly associate the reader end with the
command. This way the ownership of both ends of the pipe will end up
with the virCommand right away reducing the need of cleanup in callers.

The returned value then can be used just to format the appropriate
arguments without worrying about cleanup or failure.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This commit is contained in:
Peter Krempa 2021-03-01 11:04:54 +01:00
parent aa5c57b407
commit ef4c325f25
4 changed files with 46 additions and 81 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
src/qemu/qemu_tpm.c
View File

@ -353,14 +353,14 @@ qemuTPMEmulatorPrepareHost(virDomainTPMDefPtr tpm,
* This function reads the passphrase and writes it into the
* write-end of a pipe so that the read-end of the pipe can be
* passed to the emulator for reading the passphrase from.
*
* Note that the returned FD is owned by @cmd.
*/
static int
qemuTPMSetupEncryption(const unsigned char *secretuuid,
virCommandPtr cmd)
{
int ret = -1;
int pipefd[2] = { -1, -1 };
virConnectPtr conn;
g_autoptr(virConnect) conn = NULL;
g_autofree uint8_t *secret = NULL;
size_t secret_len;
virSecretLookupTypeDef seclookupdef = {
@ -375,27 +375,9 @@ qemuTPMSetupEncryption(const unsigned char *secretuuid,
if (virSecretGetSecretString(conn, &seclookupdef,
VIR_SECRET_USAGE_TYPE_VTPM,
&secret, &secret_len) < 0)
goto error;
return -1;
if (virPipe(pipefd) < 0)
goto error;
if (virCommandSetSendBuffer(cmd, pipefd[1], secret, secret_len) < 0)
goto error;
secret = NULL;
ret = pipefd[0];
cleanup:
virObjectUnref(conn);
return ret;
error:
VIR_FORCE_CLOSE(pipefd[1]);
VIR_FORCE_CLOSE(pipefd[0]);
goto cleanup;
return virCommandSetSendBuffer(cmd, g_steal_pointer(&secret), secret_len);
}
/*
@ -549,8 +531,8 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm,
bool created = false;
g_autofree char *pidfile = NULL;
g_autofree char *swtpm = virTPMGetSwtpm();
VIR_AUTOCLOSE pwdfile_fd = -1;
VIR_AUTOCLOSE migpwdfile_fd = -1;
int pwdfile_fd = -1;
int migpwdfile_fd = -1;
const unsigned char *secretuuid = NULL;
if (!swtpm)
@ -618,25 +600,13 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm,
}
pwdfile_fd = qemuTPMSetupEncryption(tpm->data.emulator.secretuuid, cmd);
if (pwdfile_fd) {
migpwdfile_fd = qemuTPMSetupEncryption(tpm->data.emulator.secretuuid,
cmd);
}
if (pwdfile_fd < 0 || migpwdfile_fd < 0)
goto error;
migpwdfile_fd = qemuTPMSetupEncryption(tpm->data.emulator.secretuuid, cmd);
virCommandAddArg(cmd, "--key");
virCommandAddArgFormat(cmd, "pwdfd=%d,mode=aes-256-cbc",
pwdfile_fd);
virCommandPassFD(cmd, pwdfile_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
pwdfile_fd = -1;
virCommandAddArgFormat(cmd, "pwdfd=%d,mode=aes-256-cbc", pwdfile_fd);
virCommandAddArg(cmd, "--migration-key");
virCommandAddArgFormat(cmd, "pwdfd=%d,mode=aes-256-cbc",
migpwdfile_fd);
virCommandPassFD(cmd, migpwdfile_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
migpwdfile_fd = -1;
virCommandAddArgFormat(cmd, "pwdfd=%d,mode=aes-256-cbc", migpwdfile_fd);
}
return g_steal_pointer(&cmd);

39
src/util/vircommand.c
View File

@ -1694,39 +1694,55 @@ virCommandFreeSendBuffers(virCommandPtr cmd)
/**
* virCommandSetSendBuffer
* @cmd: the command to modify
* @buffer: buffer to pass to the filedescriptror
* @buflen: length of @buffer
*
* Pass a buffer to virCommand that will be written into the
* given file descriptor. The buffer will be freed automatically
* and the file descriptor closed.
* Registers @buffer as an input buffer for @cmd which will be accessible via
* the returned file descriptor. The returned file descriptor is already
* registered to be passed to @cmd, so callers must use it only to format the
* appropriate argument of @cmd.
*
* @buffer is always stolen regardless of the return value. This function
* doesn't raise a libvirt error, but rather propagates the error via virCommand.
* Thus callers don't need to take a special action if -1 is returned.
*/
int
virCommandSetSendBuffer(virCommandPtr cmd,
int fd,
unsigned char *buffer, size_t buflen)
unsigned char *buffer,
size_t buflen)
{
g_autofree unsigned char *localbuf = g_steal_pointer(&buffer);
int pipefd[2] = { -1, -1 };
size_t i;
if (virCommandHasError(cmd))
return -1;
if (fcntl(fd, F_SETFL, O_NONBLOCK) < 0) {
virReportSystemError(errno, "%s",
_("fcntl failed to set O_NONBLOCK"));
if (virPipeQuiet(pipefd) < 0) {
cmd->has_error = errno;
return -1;
}
if (fcntl(pipefd[1], F_SETFL, O_NONBLOCK) < 0) {
cmd->has_error = errno;
VIR_FORCE_CLOSE(pipefd[0]);
VIR_FORCE_CLOSE(pipefd[1]);
return -1;
}
i = virCommandGetNumSendBuffers(cmd);
ignore_value(VIR_REALLOC_N(cmd->sendBuffers, i + 1));
cmd->sendBuffers[i].fd = fd;
cmd->sendBuffers[i].buffer = buffer;
cmd->sendBuffers[i].fd = pipefd[1];
cmd->sendBuffers[i].buffer = g_steal_pointer(&localbuf);
cmd->sendBuffers[i].buflen = buflen;
cmd->sendBuffers[i].offset = 0;
cmd->numSendBuffers++;
return 0;
virCommandPassFD(cmd, pipefd[0], VIR_COMMAND_PASS_FD_CLOSE_PARENT);
return pipefd[0];
}
@ -2851,7 +2867,6 @@ int virCommandHandshakeNotify(virCommandPtr cmd)
#else /* WIN32 */
int
virCommandSetSendBuffer(virCommandPtr cmd,
int fd G_GNUC_UNUSED,
unsigned char *buffer G_GNUC_UNUSED,
size_t buflen G_GNUC_UNUSED)
{

6
src/util/vircommand.h
View File

@ -141,9 +141,9 @@ void virCommandSetWorkingDirectory(virCommandPtr cmd,
const char *pwd) ATTRIBUTE_NONNULL(2);
int virCommandSetSendBuffer(virCommandPtr cmd,
int fd,
unsigned char *buffer, size_t buflen)
ATTRIBUTE_NONNULL(3);
unsigned char *buffer,
size_t buflen)
ATTRIBUTE_NONNULL(2);
void virCommandSetInputBuffer(virCommandPtr cmd,
const char *inbuf) ATTRIBUTE_NONNULL(2);

32
tests/commandtest.c
View File

@ -1039,8 +1039,8 @@ static int test26(const void *unused G_GNUC_UNUSED)
static int test27(const void *unused G_GNUC_UNUSED)
{
g_autoptr(virCommand) cmd = virCommandNew(abs_builddir "/commandhelper");
int pipe1[2];
int pipe2[2];
int buf1fd;
int buf2fd;
int ret = -1;
size_t buflen = 1024 * 128;
g_autofree char *buffer0 = NULL;
@ -1078,30 +1078,14 @@ static int test27(const void *unused G_GNUC_UNUSED)
errexpect = g_strdup_printf(TEST27_ERREXPECT_TEMP,
buffer0, buffer1, buffer2);
if (virPipeQuiet(pipe1) < 0 || virPipeQuiet(pipe2) < 0) {
printf("Could not create pipe: %s\n", g_strerror(errno));
goto cleanup;
}
if (virCommandSetSendBuffer(cmd, pipe1[1],
(unsigned char *)buffer1, buflen - 1) < 0 ||
virCommandSetSendBuffer(cmd, pipe2[1],
(unsigned char *)buffer2, buflen - 1) < 0) {
printf("Could not set send buffers\n");
goto cleanup;
}
pipe1[1] = -1;
pipe2[1] = -1;
buffer1 = NULL;
buffer2 = NULL;
buf1fd = virCommandSetSendBuffer(cmd, (unsigned char *) g_steal_pointer(&buffer1), buflen - 1);
buf2fd = virCommandSetSendBuffer(cmd, (unsigned char *) g_steal_pointer(&buffer2), buflen - 1);
virCommandAddArg(cmd, "--readfd");
virCommandAddArgFormat(cmd, "%d", pipe1[0]);
virCommandPassFD(cmd, pipe1[0], 0);
virCommandAddArgFormat(cmd, "%d", buf1fd);
virCommandAddArg(cmd, "--readfd");
virCommandAddArgFormat(cmd, "%d", pipe2[0]);
virCommandPassFD(cmd, pipe2[0], 0);
virCommandAddArgFormat(cmd, "%d", buf2fd);
virCommandSetInputBuffer(cmd, buffer0);
virCommandSetOutputBuffer(cmd, &outactual);
@ -1130,10 +1114,6 @@ static int test27(const void *unused G_GNUC_UNUSED)
ret = 0;
cleanup:
VIR_FORCE_CLOSE(pipe1[0]);
VIR_FORCE_CLOSE(pipe2[0]);
VIR_FORCE_CLOSE(pipe1[1]);
VIR_FORCE_CLOSE(pipe2[1]);
return ret;
}