From nobody Sun Feb 8 13:53:31 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) client-ip=170.10.133.124; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-124.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1614701566; cv=none; d=zohomail.com; s=zohoarc; b=mVWWSkg8AmFNLjj/88IvKFtMEuqRbKfiG8GV7xHPZIQQoOIJdco4qH359A9TvybS5wtSPBAVSxRzDWc5aLdx1KvNiEmJmPyli12hNMyp3qGobmB84ehBbRuEu9tFkVOGfUqqTnq3vnGxDP6KzgosvG4CygKPHoPjdYL8ZEcX+D0= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1614701566; h=Content-Type:Content-Transfer-Encoding:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=8M7RPKffF2jMaTsUQ6cxpyeF3wx6WnlRDN8qTgD8TYo=; b=llTEkxS5KiQfiV8xQLS1d/xv9SLhCLvD3WKUToeCMoc/5ukxvOgfkVV5s04L8IKLZ6H9aAgX+5kGIzZJwqTiPtkiHpxpOjaGqdv3DDkYn6cpv2tYMemOeKzk23PUCJXVgwI1DtT1NgDPiM7WwrwR3A5FOHYpXM4hJhb7Hi+R3Qs= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) header.from= Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.zohomail.com with SMTPS id 16147015663991015.8122292855385; Tue, 2 Mar 2021 08:12:46 -0800 (PST) Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-104-68e9zOSYMSyBhkLcjttpkw-1; Tue, 02 Mar 2021 11:12:42 -0500 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id C5D2C803F48; Tue, 2 Mar 2021 16:12:36 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9E1696A035; Tue, 2 Mar 2021 16:12:36 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 67D041809C91; Tue, 2 Mar 2021 16:12:36 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 122GCNZ5024039 for ; Tue, 2 Mar 2021 11:12:23 -0500 Received: by smtp.corp.redhat.com (Postfix) id 79E2060C43; Tue, 2 Mar 2021 16:12:23 +0000 (UTC) Received: from speedmetal.lan (unknown [10.40.208.53]) by smtp.corp.redhat.com (Postfix) with ESMTP id C2AAA60BFA for ; Tue, 2 Mar 2021 16:12:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1614701565; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=8M7RPKffF2jMaTsUQ6cxpyeF3wx6WnlRDN8qTgD8TYo=; b=dnP0U8yNq+jgw5AFdPfK/23ll5RtfUZgS5Vuf+RVqxFqkwGHZKkUzIPcnXHYSWkKZFyYs6 5MsScko0oouCz2vS/eDQYdSGs8QfIR4c6Zl5RX3PpW8/qJPXD1xoH97LN0SDOo/xn00haH yIZpcdkSy4EPk/nNKbY1oARtRFJx2rE= X-MC-Unique: 68e9zOSYMSyBhkLcjttpkw-1 From: Peter Krempa To: libvir-list@redhat.com Subject: [PATCH 5/6] virCommandSetSendBuffer: Provide saner semantics Date: Tue, 2 Mar 2021 17:12:07 +0100 Message-Id: In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-loop: libvir-list@redhat.com X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=libvir-list-bounces@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) Content-Type: text/plain; charset="utf-8" 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 Reviewed-by: J=C3=A1n Tomko --- src/qemu/qemu_tpm.c | 50 +++++++++---------------------------------- src/util/vircommand.c | 39 ++++++++++++++++++++++----------- src/util/vircommand.h | 6 +++--- tests/commandtest.c | 32 ++++++--------------------- 4 files changed, 46 insertions(+), 81 deletions(-) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index d994816484..d8c518fc73 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -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 =3D -1; - int pipefd[2] =3D { -1, -1 }; - virConnectPtr conn; + g_autoptr(virConnect) conn =3D NULL; g_autofree uint8_t *secret =3D NULL; size_t secret_len; virSecretLookupTypeDef seclookupdef =3D { @@ -375,27 +375,9 @@ qemuTPMSetupEncryption(const unsigned char *secretuuid, if (virSecretGetSecretString(conn, &seclookupdef, VIR_SECRET_USAGE_TYPE_VTPM, &secret, &secret_len) < 0) - goto error; - - if (virPipe(pipefd) < 0) - goto error; - - if (virCommandSetSendBuffer(cmd, pipefd[1], secret, secret_len) < 0) - goto error; - - secret =3D NULL; - ret =3D pipefd[0]; - - cleanup: - virObjectUnref(conn); - - return ret; - - error: - VIR_FORCE_CLOSE(pipefd[1]); - VIR_FORCE_CLOSE(pipefd[0]); + return -1; - goto cleanup; + return virCommandSetSendBuffer(cmd, g_steal_pointer(&secret), secret_l= en); } /* @@ -549,8 +531,8 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm, bool created =3D false; g_autofree char *pidfile =3D NULL; g_autofree char *swtpm =3D virTPMGetSwtpm(); - VIR_AUTOCLOSE pwdfile_fd =3D -1; - VIR_AUTOCLOSE migpwdfile_fd =3D -1; + int pwdfile_fd =3D -1; + int migpwdfile_fd =3D -1; const unsigned char *secretuuid =3D NULL; if (!swtpm) @@ -618,25 +600,13 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm, } pwdfile_fd =3D qemuTPMSetupEncryption(tpm->data.emulator.secretuui= d, cmd); - if (pwdfile_fd) { - migpwdfile_fd =3D qemuTPMSetupEncryption(tpm->data.emulator.se= cretuuid, - cmd); - } - - if (pwdfile_fd < 0 || migpwdfile_fd < 0) - goto error; + migpwdfile_fd =3D qemuTPMSetupEncryption(tpm->data.emulator.secret= uuid, cmd); virCommandAddArg(cmd, "--key"); - virCommandAddArgFormat(cmd, "pwdfd=3D%d,mode=3Daes-256-cbc", - pwdfile_fd); - virCommandPassFD(cmd, pwdfile_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT= ); - pwdfile_fd =3D -1; + virCommandAddArgFormat(cmd, "pwdfd=3D%d,mode=3Daes-256-cbc", pwdfi= le_fd); virCommandAddArg(cmd, "--migration-key"); - virCommandAddArgFormat(cmd, "pwdfd=3D%d,mode=3Daes-256-cbc", - migpwdfile_fd); - virCommandPassFD(cmd, migpwdfile_fd, VIR_COMMAND_PASS_FD_CLOSE_PAR= ENT); - migpwdfile_fd =3D -1; + virCommandAddArgFormat(cmd, "pwdfd=3D%d,mode=3Daes-256-cbc", migpw= dfile_fd); } return g_steal_pointer(&cmd); diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 579c77fd9f..04964b730b 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -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 virC= ommand. + * 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 =3D g_steal_pointer(&buffer); + int pipefd[2] =3D { -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 =3D errno; + return -1; + } + + if (fcntl(pipefd[1], F_SETFL, O_NONBLOCK) < 0) { cmd->has_error =3D errno; + VIR_FORCE_CLOSE(pipefd[0]); + VIR_FORCE_CLOSE(pipefd[1]); return -1; } i =3D virCommandGetNumSendBuffers(cmd); ignore_value(VIR_REALLOC_N(cmd->sendBuffers, i + 1)); - cmd->sendBuffers[i].fd =3D fd; - cmd->sendBuffers[i].buffer =3D buffer; + cmd->sendBuffers[i].fd =3D pipefd[1]; + cmd->sendBuffers[i].buffer =3D g_steal_pointer(&localbuf); cmd->sendBuffers[i].buflen =3D buflen; cmd->sendBuffers[i].offset =3D 0; cmd->numSendBuffers++; - return 0; + virCommandPassFD(cmd, pipefd[0], VIR_COMMAND_PASS_FD_CLOSE_PARENT); + + return pipefd[0]; } @@ -2835,7 +2851,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) { diff --git a/src/util/vircommand.h b/src/util/vircommand.h index 9fb625ec4b..a00f30f51f 100644 --- a/src/util/vircommand.h +++ b/src/util/vircommand.h @@ -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); diff --git a/tests/commandtest.c b/tests/commandtest.c index df86725f0e..524c959eba 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -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 =3D virCommandNew(abs_builddir "/commandhelp= er"); - int pipe1[2]; - int pipe2[2]; + int buf1fd; + int buf2fd; int ret =3D -1; size_t buflen =3D 1024 * 128; g_autofree char *buffer0 =3D NULL; @@ -1078,30 +1078,14 @@ static int test27(const void *unused G_GNUC_UNUSED) errexpect =3D 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] =3D -1; - pipe2[1] =3D -1; - buffer1 =3D NULL; - buffer2 =3D NULL; + buf1fd =3D virCommandSetSendBuffer(cmd, (unsigned char *) g_steal_poin= ter(&buffer1), buflen - 1); + buf2fd =3D virCommandSetSendBuffer(cmd, (unsigned char *) g_steal_poin= ter(&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 =3D 0; cleanup: - VIR_FORCE_CLOSE(pipe1[0]); - VIR_FORCE_CLOSE(pipe2[0]); - VIR_FORCE_CLOSE(pipe1[1]); - VIR_FORCE_CLOSE(pipe2[1]); return ret; } --=20 2.29.2