From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.5 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8F733C07E99 for ; Mon, 12 Jul 2021 19:38:09 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B9DB56115A for ; Mon, 12 Jul 2021 19:38:08 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B9DB56115A Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:37080 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1m31kl-0005os-LD for qemu-devel@archiver.kernel.org; Mon, 12 Jul 2021 15:38:07 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:49232) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m31jj-0005A9-V7 for qemu-devel@nongnu.org; Mon, 12 Jul 2021 15:37:04 -0400 Received: from mail-ej1-x62b.google.com ([2a00:1450:4864:20::62b]:36686) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1m31jh-0006Oj-AR for qemu-devel@nongnu.org; Mon, 12 Jul 2021 15:37:03 -0400 Received: by mail-ej1-x62b.google.com with SMTP id nd37so36799915ejc.3 for ; Mon, 12 Jul 2021 12:36:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=gHGAIfVKGyNmjzzE2fdxyx6c9odBxwGW3pYoAwGIAfY=; b=EZCxrSrrwUmm01opoKOfwbChlIqHFyIAdmQy81yo5Nhflw85yhlKkQQ80ZWzq4icHi 3QpTWtuvOaVMpiuodpyd9j6q93P6zXnnB6sKp79+IDjIH3nzpVtdFPHr1QJKK/NSHiTp VD/5yG+9mIzAht5tYCc4iNBXGS01GBnOHPJ9zgpRUjzOnHAgrkRnQZ9yvnesLErf2hJn 7KV9/y2G9db3R28gL+yH3gc1GC4tFlbgQxaa6gs0oJO0twIxgTbNkSnNqmm5NcwJsNBN 1SoQi/ZRTfX5ZzcMVi7YtnO10AcSfO7CGGQFuFza5Ep5EI0u8hroOa12lomVzKLZyXiO a4HA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=gHGAIfVKGyNmjzzE2fdxyx6c9odBxwGW3pYoAwGIAfY=; b=GpN6UEL9bc+KtMh8it0i8tuCqqdjm7UDbkXiWoKfbL243tBc+cr03iZ4Glx1wvOcWw HQkAuGCE+D9Aa4Y8cKqqRvgzlVmpbjX4jgCKzbg75ZV8JDgc4NuCy2rwk7P+Vzno6NpE 7N5iMfIav0bcoR3lUNtnStSkoxmOGnnQK6F5EUqhjL6cn3c2rlVGkCH7GGK4uXb/mMIg Cq7I7XLSiDovquAW0SEEyckKg4q9diCk3y5Zx9WYMBcUfqRN3Z17ANytya4ImPgMVwrD BN+7sDO9VKDA8zyuo1qTIODRfxXqtIkL18YBDdKfhD2sBX6tsmaL3mQs7Kk98Oq3FuwZ ywHQ== X-Gm-Message-State: AOAM532qgeVvmlxelKIJhyKh2Wh1Nl6Qzcvrzpjj3hP1r22pyVJOnRD4 RkW0norzJo6neGcOxtsspC1VOAE0tJubthLjGhg= X-Google-Smtp-Source: ABdhPJz5j1sf6CzU8POQliPsyBkxjSS+PARBkhYrxmDq2chx0GDOh9nUk5fuibI+HvyBOHaph1qgrQNq5aDsAFNSi3Q= X-Received: by 2002:a17:906:39c3:: with SMTP id i3mr734607eje.527.1626118618504; Mon, 12 Jul 2021 12:36:58 -0700 (PDT) MIME-Version: 1.0 References: <1625678434-240960-1-git-send-email-steven.sistare@oracle.com> <1625678434-240960-11-git-send-email-steven.sistare@oracle.com> In-Reply-To: From: =?UTF-8?B?TWFyYy1BbmRyw6kgTHVyZWF1?= Date: Mon, 12 Jul 2021 23:36:45 +0400 Message-ID: Subject: Re: [PATCH V5 10/25] util: env var helpers To: Steven Sistare Content-Type: multipart/alternative; boundary="0000000000009e60e605c6f23e96" Received-SPF: pass client-ip=2a00:1450:4864:20::62b; envelope-from=marcandre.lureau@gmail.com; helo=mail-ej1-x62b.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Jason Zeng , Juan Quintela , Eric Blake , "Michael S. Tsirkin" , QEMU , "Dr. David Alan Gilbert" , Alex Williamson , Stefan Hajnoczi , Paolo Bonzini , "Daniel P. Berrange" , =?UTF-8?Q?Philippe_Mathieu=2DDaud=C3=A9?= , =?UTF-8?B?QWxleCBCZW5uw6ll?= , Markus Armbruster Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --0000000000009e60e605c6f23e96 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi On Mon, Jul 12, 2021 at 11:19 PM Steven Sistare wrote: > On 7/8/2021 11:10 AM, Marc-Andr=C3=A9 Lureau wrote: > > Hi > > > > On Wed, Jul 7, 2021 at 9:30 PM Steve Sistare > wrote: > > > > Add functions for saving fd's and other values in the environment v= ia > > setenv, and for reading them back via getenv. > > > > > > I understand that the rest of the series will rely on environment > variables to associate and recover the child-passed FDs, but I am not > really convinced that it is a good idea. > > > > Environment variables have a number of issues that we may encounter dow= n > the road: namespace, limits, concurrency, observability etc.. I wonder if > the VMState couldn't have a section about the FD to recover. Or maybe jus= t > another shared memory region? > > They also have some advantages. Their post-exec value can be observed vi= a > /proc/$pid/environ, > and modified values can be observed by calling printenv() in a debugger. > They are naturally carried > across exec, with no external file to create and potentially lose. > Lastly, libcs already defines > put and get methods, so the additional layered code is small and simple. > The number of variables > is small, and I would rather not over-engineer an alternate solution unti= l > the env proves > inadequate. The limits on env size are huge on Linux. The limits are > smaller on Windows, but > that is just one of multiple issues to be addressed to support live updat= e > on windows. > > For the alternatives, shared memory is no more observable (maybe less) an= d > also has no concurrency > protection. VMstate does not help because the descriptors are needed > before the vmstate file > is opened. > Why does it need to be "observable" from outside the process? I meant memory to be shared between the qemu instances (without concurrency etc). You would only need that memory fd to be passed as argument to the next qemu instance, to restore the rest of the contexts/fds I suppose. I think we need to do this right, as it may have consequences for future updates. It's effectively a kind of protocol. We have better chances to handle different versions correctly by reusing VMState imho. > > Some comments below. These new utils could also have some unit tests. > > OK. > > > Signed-off-by: Steve Sistare steven.sistare@oracle.com>> > > --- > > MAINTAINERS | 2 ++ > > include/qemu/env.h | 23 +++++++++++++ > > util/env.c | 95 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > util/meson.build | 1 + > > 4 files changed, 121 insertions(+) > > create mode 100644 include/qemu/env.h > > create mode 100644 util/env.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index c48dd37..8647a97 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -2865,6 +2865,8 @@ S: Maintained > > F: include/migration/cpr.h > > F: migration/cpr.c > > F: qapi/cpr.json > > +F: include/qemu/env.h > > +F: util/env.c > > > > Record/replay > > M: Pavel Dovgalyuk pavel.dovgaluk@ispras.ru>> > > diff --git a/include/qemu/env.h b/include/qemu/env.h > > new file mode 100644 > > index 0000000..3dad503 > > --- /dev/null > > +++ b/include/qemu/env.h > > @@ -0,0 +1,23 @@ > > +/* > > + * Copyright (c) 2021 Oracle and/or its affiliates. > > + * > > + * This work is licensed under the terms of the GNU GPL, version 2= . > > + * See the COPYING file in the top-level directory. > > + * > > + */ > > + > > +#ifndef QEMU_ENV_H > > +#define QEMU_ENV_H > > + > > +#define FD_PREFIX "QEMU_FD_" > > + > > +typedef int (*walkenv_cb)(const char *name, const char *val, void > *handle); > > + > > +int getenv_fd(const char *name); > > +void setenv_fd(const char *name, int fd); > > +void unsetenv_fd(const char *name); > > +void unsetenv_fdv(const char *fmt, ...); > > +int walkenv(const char *prefix, walkenv_cb cb, void *handle); > > +void printenv(void); > > > > > > Please use qemu prefix, that avoids potential confusion with system > libraries. > > > > + > > +#endif > > diff --git a/util/env.c b/util/env.c > > new file mode 100644 > > index 0000000..863678d > > --- /dev/null > > +++ b/util/env.c > > @@ -0,0 +1,95 @@ > > +/* > > + * Copyright (c) 2021 Oracle and/or its affiliates. > > + * > > + * This work is licensed under the terms of the GNU GPL, version 2= . > > + * See the COPYING file in the top-level directory. > > + */ > > + > > +#include "qemu/osdep.h" > > +#include "qemu/cutils.h" > > +#include "qemu/env.h" > > + > > +static uint64_t getenv_ulong(const char *prefix, const char *name, > int *err) > > +{ > > + char var[80], *val; > > + uint64_t res =3D 0; > > + > > + snprintf(var, sizeof(var), "%s%s", prefix, name); > > > > > > No check for success / truncation... > > > > Please use g_autofree char *var =3D g_strdup_printf().. > > > > + val =3D getenv(var); > > > > > > For consistency, I'd use g_getenv() > > > > + if (val) { > > + *err =3D qemu_strtoul(val, NULL, 10, &res); > > + } else { > > + *err =3D -ENOENT; > > + } > > + return res; > > +} > > + > > +static void setenv_ulong(const char *prefix, const char *name, > uint64_t val) > > +{ > > + char var[80], val_str[80]; > > + snprintf(var, sizeof(var), "%s%s", prefix, name); > > + snprintf(val_str, sizeof(val_str), "%"PRIu64, val); > > > > > > g_strdup_printf > > > > + setenv(var, val_str, 1); > > > > > > g_setenv(), and return error value (or assert() if that makes more sens= e) > > > > +} > > + > > +static void unsetenv_ulong(const char *prefix, const char *name) > > +{ > > + char var[80]; > > + snprintf(var, sizeof(var), "%s%s", prefix, name); > > > > > > g_strdup_printf > > > > > > + unsetenv(var); > > > > > > g_unsetenv > > > > +} > > + > > +int getenv_fd(const char *name) > > +{ > > + int err; > > + int fd =3D getenv_ulong(FD_PREFIX, name, &err); > > > > > > I'd try to use qemu_parse_fd() instead. > > > > + return err ? -1 : fd; > > +} > > + > > +void setenv_fd(const char *name, int fd) > > +{ > > > > > > Maybe check fd >=3D 0 ? > > > > + setenv_ulong(FD_PREFIX, name, fd); > > +} > > + > > +void unsetenv_fd(const char *name) > > +{ > > + unsetenv_ulong(FD_PREFIX, name); > > +} > > + > > +void unsetenv_fdv(const char *fmt, ...) > > +{ > > + va_list args; > > + char buf[80]; > > + va_start(args, fmt); > > + vsnprintf(buf, sizeof(buf), fmt, args); > > + va_end(args); > > > > > > That seems to be a leftover. > > It is called in the subsequent vfio cpr patches. > > > +} > > + > > +int walkenv(const char *prefix, walkenv_cb cb, void *handle) > > > > +{ > > + char *str, name[128]; > > + char **envp =3D environ; > > + size_t prefix_len =3D strlen(prefix); > > + > > + while (*envp) { > > + str =3D *envp++; > > + if (!strncmp(str, prefix, prefix_len)) { > > > > + char *val =3D strchr(str, '=3D'); > > + str +=3D prefix_len; > > + strncpy(name, str, val - str); > > > > > > g_strndup() to avoid potential buffer overflow. > > > > + name[val - str] =3D 0; > > + if (cb(name, val + 1, handle)) { > > + return 1; > > + } > > + } > > + } > > + return 0; > > +} > > + > > +void printenv(void) > > +{ > > + char **ptr =3D environ; > > + while (*ptr) { > > + puts(*ptr++); > > + } > > +} > > > > > > Is this really useful? I doubt it. > > I call it from gdb for debugging, but I can delete it and cast g_listenv(= ) > instead: > print *(((char ** (*)(void))g_listenv)())@100 > Or just *environ@N ? > Will do on the rest. > > - Steve > > > diff --git a/util/meson.build b/util/meson.build > > index 0ffd7f4..5e8097a 100644 > > --- a/util/meson.build > > +++ b/util/meson.build > > @@ -23,6 +23,7 @@ util_ss.add(files('host-utils.c')) > > util_ss.add(files('bitmap.c', 'bitops.c')) > > util_ss.add(files('fifo8.c')) > > util_ss.add(files('cacheinfo.c', 'cacheflush.c')) > > +util_ss.add(files('env.c')) > > util_ss.add(files('error.c', 'qemu-error.c')) > > util_ss.add(files('qemu-print.c')) > > util_ss.add(files('id.c')) > > -- > > 1.8.3.1 > > > > > > > > > > -- > > Marc-Andr=C3=A9 Lureau > --=20 Marc-Andr=C3=A9 Lureau --0000000000009e60e605c6f23e96 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi

On Mon, Jul 12, 2021 at 11:19 PM St= even Sistare <steven.sistar= e@oracle.com> wrote:
On 7/8/2021 11:10 AM, Marc-Andr=C3=A9 Lureau wrote:
> Hi
>
> On Wed, Jul 7, 2021 at 9:30 PM Steve Sistare <steven.sistare@oracle.com <= ;mailto:stev= en.sistare@oracle.com>> wrote:
>
>=C2=A0 =C2=A0 =C2=A0Add functions for saving fd's and other values = in the environment via
>=C2=A0 =C2=A0 =C2=A0setenv, and for reading them back via getenv.
>
>
> I understand that the rest of the series will rely on environment vari= ables to associate and recover the child-passed FDs, but I am not really co= nvinced that it is a good idea.
>
> Environment variables have a number of issues that we may encounter do= wn the road: namespace, limits, concurrency, observability etc.. I wonder i= f the VMState couldn't have a section about the FD to recover. Or maybe= just another shared memory region?

They also have some advantages.=C2=A0 Their post-exec value can be observed= via /proc/$pid/environ,
and modified values can be observed by calling printenv() in a debugger.=C2= =A0 They are naturally carried
across exec, with no external file to create and potentially lose.=C2=A0 La= stly, libcs already defines
put and get methods, so the additional layered code is small and simple.=C2= =A0 The number of variables
is small, and I would rather not over-engineer an alternate solution until = the env proves
inadequate.=C2=A0 The limits on env size are huge on Linux.=C2=A0 The limit= s are smaller on Windows, but
that is just one of multiple issues to be addressed to support live update = on windows.

For the alternatives, shared memory is no more observable (maybe less) and = also has no concurrency
protection.=C2=A0 VMstate does not help because the descriptors are needed = before the vmstate file
is opened.

Why does it need to be "= ;observable" from outside the process?

I mean= t memory to be shared between the qemu instances (without concurrency etc).=

You would only need that memory fd to be passed a= s argument to the next qemu instance, to restore the rest of the contexts/f= ds I suppose.

I think we need to do this right= , as it may have consequences for future updates. It's effectively a ki= nd of protocol. We have better chances to handle different versions correct= ly by reusing VMState imho.


> Some comments below. These new utils could also have some unit tests.<= br>
OK.

>=C2=A0 =C2=A0 =C2=A0Signed-off-by: Steve Sistare <steven.sistare@oracle.com = <mailto:s= teven.sistare@oracle.com>>
>=C2=A0 =C2=A0 =C2=A0---
>=C2=A0 =C2=A0 =C2=A0=C2=A0MAINTAINERS=C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2= =A0 2 ++
>=C2=A0 =C2=A0 =C2=A0=C2=A0include/qemu/env.h | 23 +++++++++++++
>=C2=A0 =C2=A0 =C2=A0=C2=A0util/env.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0|= 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>=C2=A0 =C2=A0 =C2=A0=C2=A0util/meson.build=C2=A0 =C2=A0|=C2=A0 1 +
>=C2=A0 =C2=A0 =C2=A0=C2=A04 files changed, 121 insertions(+)
>=C2=A0 =C2=A0 =C2=A0=C2=A0create mode 100644 include/qemu/env.h
>=C2=A0 =C2=A0 =C2=A0=C2=A0create mode 100644 util/env.c
>
>=C2=A0 =C2=A0 =C2=A0diff --git a/MAINTAINERS b/MAINTAINERS
>=C2=A0 =C2=A0 =C2=A0index c48dd37..8647a97 100644
>=C2=A0 =C2=A0 =C2=A0--- a/MAINTAINERS
>=C2=A0 =C2=A0 =C2=A0+++ b/MAINTAINERS
>=C2=A0 =C2=A0 =C2=A0@@ -2865,6 +2865,8 @@ S: Maintained
>=C2=A0 =C2=A0 =C2=A0=C2=A0F: include/migration/cpr.h
>=C2=A0 =C2=A0 =C2=A0=C2=A0F: migration/cpr.c
>=C2=A0 =C2=A0 =C2=A0=C2=A0F: qapi/cpr.json
>=C2=A0 =C2=A0 =C2=A0+F: include/qemu/env.h
>=C2=A0 =C2=A0 =C2=A0+F: util/env.c
>
>=C2=A0 =C2=A0 =C2=A0=C2=A0Record/replay
>=C2=A0 =C2=A0 =C2=A0=C2=A0M: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru <ma= ilto:pavel.do= vgaluk@ispras.ru>>
>=C2=A0 =C2=A0 =C2=A0diff --git a/include/qemu/env.h b/include/qemu/env.= h
>=C2=A0 =C2=A0 =C2=A0new file mode 100644
>=C2=A0 =C2=A0 =C2=A0index 0000000..3dad503
>=C2=A0 =C2=A0 =C2=A0--- /dev/null
>=C2=A0 =C2=A0 =C2=A0+++ b/include/qemu/env.h
>=C2=A0 =C2=A0 =C2=A0@@ -0,0 +1,23 @@
>=C2=A0 =C2=A0 =C2=A0+/*
>=C2=A0 =C2=A0 =C2=A0+ * Copyright (c) 2021 Oracle and/or its affiliates= .
>=C2=A0 =C2=A0 =C2=A0+ *
>=C2=A0 =C2=A0 =C2=A0+ * This work is licensed under the terms of the GN= U GPL, version 2.
>=C2=A0 =C2=A0 =C2=A0+ * See the COPYING file in the top-level directory= .
>=C2=A0 =C2=A0 =C2=A0+ *
>=C2=A0 =C2=A0 =C2=A0+ */
>=C2=A0 =C2=A0 =C2=A0+
>=C2=A0 =C2=A0 =C2=A0+#ifndef QEMU_ENV_H
>=C2=A0 =C2=A0 =C2=A0+#define QEMU_ENV_H
>=C2=A0 =C2=A0 =C2=A0+
>=C2=A0 =C2=A0 =C2=A0+#define FD_PREFIX "QEMU_FD_"
>=C2=A0 =C2=A0 =C2=A0+
>=C2=A0 =C2=A0 =C2=A0+typedef int (*walkenv_cb)(const char *name, const = char *val, void *handle);
>=C2=A0 =C2=A0 =C2=A0+
>=C2=A0 =C2=A0 =C2=A0+int getenv_fd(const char *name);
>=C2=A0 =C2=A0 =C2=A0+void setenv_fd(const char *name, int fd);
>=C2=A0 =C2=A0 =C2=A0+void unsetenv_fd(const char *name);
>=C2=A0 =C2=A0 =C2=A0+void unsetenv_fdv(const char *fmt, ...);
>=C2=A0 =C2=A0 =C2=A0+int walkenv(const char *prefix, walkenv_cb cb, voi= d *handle);
>=C2=A0 =C2=A0 =C2=A0+void printenv(void);
>
>
> Please use qemu prefix, that avoids potential confusion with system li= braries.
>
>=C2=A0 =C2=A0 =C2=A0+
>=C2=A0 =C2=A0 =C2=A0+#endif
>=C2=A0 =C2=A0 =C2=A0diff --git a/util/env.c b/util/env.c
>=C2=A0 =C2=A0 =C2=A0new file mode 100644
>=C2=A0 =C2=A0 =C2=A0index 0000000..863678d
>=C2=A0 =C2=A0 =C2=A0--- /dev/null
>=C2=A0 =C2=A0 =C2=A0+++ b/util/env.c
>=C2=A0 =C2=A0 =C2=A0@@ -0,0 +1,95 @@
>=C2=A0 =C2=A0 =C2=A0+/*
>=C2=A0 =C2=A0 =C2=A0+ * Copyright (c) 2021 Oracle and/or its affiliates= .
>=C2=A0 =C2=A0 =C2=A0+ *
>=C2=A0 =C2=A0 =C2=A0+ * This work is licensed under the terms of the GN= U GPL, version 2.
>=C2=A0 =C2=A0 =C2=A0+ * See the COPYING file in the top-level directory= .
>=C2=A0 =C2=A0 =C2=A0+ */
>=C2=A0 =C2=A0 =C2=A0+
>=C2=A0 =C2=A0 =C2=A0+#include "qemu/osdep.h"
>=C2=A0 =C2=A0 =C2=A0+#include "qemu/cutils.h"
>=C2=A0 =C2=A0 =C2=A0+#include "qemu/env.h"
>=C2=A0 =C2=A0 =C2=A0+
>=C2=A0 =C2=A0 =C2=A0+static uint64_t getenv_ulong(const char *prefix, c= onst char *name, int *err)
>=C2=A0 =C2=A0 =C2=A0+{
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 char var[80], *val;
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 uint64_t res =3D 0;
>=C2=A0 =C2=A0 =C2=A0+
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 snprintf(var, sizeof(var), "%s%= s", prefix, name);
>
>
> No check for success / truncation...
>
> Please use g_autofree char *var =3D g_strdup_printf()..
>
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 val =3D getenv(var);
>
>
> For consistency, I'd use g_getenv()
>
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 if (val) {
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 =C2=A0 =C2=A0 *err =3D qemu_strtoul(= val, NULL, 10, &res);
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 } else {
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 =C2=A0 =C2=A0 *err =3D -ENOENT;
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 }
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 return res;
>=C2=A0 =C2=A0 =C2=A0+}
>=C2=A0 =C2=A0 =C2=A0+
>=C2=A0 =C2=A0 =C2=A0+static void setenv_ulong(const char *prefix, const= char *name, uint64_t val)
>=C2=A0 =C2=A0 =C2=A0+{
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 char var[80], val_str[80];
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 snprintf(var, sizeof(var), "%s%= s", prefix, name);
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 snprintf(val_str, sizeof(val_str), &= quot;%"PRIu64, val);
>
>
> g_strdup_printf
>
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 setenv(var, val_str, 1);
>
>
> g_setenv(), and return error value (or assert() if that makes more sen= se)
>
>=C2=A0 =C2=A0 =C2=A0+}
>=C2=A0 =C2=A0 =C2=A0+
>=C2=A0 =C2=A0 =C2=A0+static void unsetenv_ulong(const char *prefix, con= st char *name)
>=C2=A0 =C2=A0 =C2=A0+{
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 char var[80];
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 snprintf(var, sizeof(var), "%s%= s", prefix, name);
>
>
> g_strdup_printf
> =C2=A0
>
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 unsetenv(var);
>
>
> g_unsetenv
>
>=C2=A0 =C2=A0 =C2=A0+}
>=C2=A0 =C2=A0 =C2=A0+
>=C2=A0 =C2=A0 =C2=A0+int getenv_fd(const char *name)
>=C2=A0 =C2=A0 =C2=A0+{
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 int err;
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 int fd =3D getenv_ulong(FD_PREFIX, n= ame, &err);
>
>
> I'd try to use qemu_parse_fd() instead.
>
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 return err ? -1 : fd;
>=C2=A0 =C2=A0 =C2=A0+}
>=C2=A0 =C2=A0 =C2=A0+
>=C2=A0 =C2=A0 =C2=A0+void setenv_fd(const char *name, int fd)
>=C2=A0 =C2=A0 =C2=A0+{
>
>
> Maybe check fd >=3D 0 ?
>
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 setenv_ulong(FD_PREFIX, name, fd); >=C2=A0 =C2=A0 =C2=A0+}
>=C2=A0 =C2=A0 =C2=A0+
>=C2=A0 =C2=A0 =C2=A0+void unsetenv_fd(const char *name)
>=C2=A0 =C2=A0 =C2=A0+{
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 unsetenv_ulong(FD_PREFIX, name);
>=C2=A0 =C2=A0 =C2=A0+}
>=C2=A0 =C2=A0 =C2=A0+
>=C2=A0 =C2=A0 =C2=A0+void unsetenv_fdv(const char *fmt, ...)
>=C2=A0 =C2=A0 =C2=A0+{
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 va_list args;
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 char buf[80];
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 va_start(args, fmt);
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 vsnprintf(buf, sizeof(buf), fmt, arg= s);
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 va_end(args);
>
>
> That seems to be a leftover.

It is called in the subsequent vfio cpr patches.

>=C2=A0 =C2=A0 =C2=A0+}
>=C2=A0 =C2=A0 =C2=A0+
>=C2=A0 =C2=A0 =C2=A0+int walkenv(const char *prefix, walkenv_cb cb, voi= d *handle)
>
>=C2=A0 =C2=A0 =C2=A0+{
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 char *str, name[128];
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 char **envp =3D environ;
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 size_t prefix_len =3D strlen(prefix)= ;
>=C2=A0 =C2=A0 =C2=A0+
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 while (*envp) {
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 =C2=A0 =C2=A0 str =3D *envp++;
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!strncmp(str, pref= ix, prefix_len)) {
>
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 char *va= l =3D strchr(str, '=3D');
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 str +=3D= prefix_len;
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 strncpy(= name, str, val - str);
>
>
> g_strndup() to avoid potential buffer overflow.
>
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 name[val= - str] =3D 0;
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (cb(n= ame, val + 1, handle)) {
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 return 1;
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 }
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 return 0;
>=C2=A0 =C2=A0 =C2=A0+}
>=C2=A0 =C2=A0 =C2=A0+
>=C2=A0 =C2=A0 =C2=A0+void printenv(void)
>=C2=A0 =C2=A0 =C2=A0+{
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 char **ptr =3D environ;
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 while (*ptr) {
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 =C2=A0 =C2=A0 puts(*ptr++);
>=C2=A0 =C2=A0 =C2=A0+=C2=A0 =C2=A0 }
>=C2=A0 =C2=A0 =C2=A0+}
>
>
> Is this really useful? I doubt it.

I call it from gdb for debugging, but I can delete it and cast g_listenv() = instead:
=C2=A0 print *(((char ** (*)(void))g_listenv)())@100
<= br>
Or just *environ@N ?


Will do on the rest.

- Steve

>=C2=A0 =C2=A0 =C2=A0diff --git a/util/meson.build b/util/meson.build >=C2=A0 =C2=A0 =C2=A0index 0ffd7f4..5e8097a 100644
>=C2=A0 =C2=A0 =C2=A0--- a/util/meson.build
>=C2=A0 =C2=A0 =C2=A0+++ b/util/meson.build
>=C2=A0 =C2=A0 =C2=A0@@ -23,6 +23,7 @@ util_ss.add(files('host-utils= .c'))
>=C2=A0 =C2=A0 =C2=A0=C2=A0util_ss.add(files('bitmap.c', 'bi= tops.c'))
>=C2=A0 =C2=A0 =C2=A0=C2=A0util_ss.add(files('fifo8.c'))
>=C2=A0 =C2=A0 =C2=A0=C2=A0util_ss.add(files('cacheinfo.c', '= ;cacheflush.c'))
>=C2=A0 =C2=A0 =C2=A0+util_ss.add(files('env.c'))
>=C2=A0 =C2=A0 =C2=A0=C2=A0util_ss.add(files('error.c', 'qem= u-error.c'))
>=C2=A0 =C2=A0 =C2=A0=C2=A0util_ss.add(files('qemu-print.c')) >=C2=A0 =C2=A0 =C2=A0=C2=A0util_ss.add(files('id.c'))
>=C2=A0 =C2=A0 =C2=A0--
>=C2=A0 =C2=A0 =C2=A01.8.3.1
>
>
>
>
> --
> Marc-Andr=C3=A9 Lureau


--
Marc-Andr=C3=A9 Lureau
--0000000000009e60e605c6f23e96--