qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>,
	"Michael Roth" <michael.roth@amd.com>,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	"Konstantin Kostiuk" <kkostiuk@redhat.com>,
	"Thomas Huth" <thuth@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>
Subject: [PATCH v2 00/22] qga: clean up command source locations and conditionals
Date: Thu, 13 Jun 2024 16:01:05 +0100	[thread overview]
Message-ID: <20240613150127.1361931-1-berrange@redhat.com> (raw)

This series is a side effect of other work I started, to attempt to
make the QGA safe to use in confidential VMs by automatically
restricting the permitted commands. Since this cleanup stands on
its own, I'm sending it now.

The QGA codebase has a very complicated maze of #ifdefs to create
stubs for the various commands that cannot be implemented on certain
platforms. It then has further logic to dynamically disable the stub
commands at runtime, except this is not consistently applied, so
some commands remain enabled despite being merely stubs.

The resulting code is hard to follow, when trying to understand exactly
what commands are available under what circumstances, and when changing
impls it is easy to get the #ifdefs wrong, resulting in stubs getting
missed on platforms without a real impl. In some cases, we have multiple
stubs for the same command, due to the maze of #ifdefs.

The QAPI schema language has support for many years for expressing
conditions against commands when declaring them. This results in the
QAPI code generator omitting their implementation entirely at build
time. This has mutliple benefits

 * The unsupported commands are guaranteed to not exist at runtime
 * No stubs need ever be defined in the code
 * The generated QAPI reference manual documents the build conditions

This series is broadly split into three parts

 * Moving tonnes of Linux only commands out of commands-posix.c
   into commands-linux.c to remove many #ifdefs.
 * Adding 'if' conditions in the QAPI schema to reflect the
   build conditions, removing many more #ifdefs
 * Sanitizing the logic for disabling/enabling commands at
   runtime to guarantee consistency

Changed in v2:

 - Make FSFreeze error reporting distinguish inability to enable
   VSS from user config choice

 - Fully remove ga_command_init_blockedrpcs() methods. No more
   special case disabling of commands. Either they're disabled
   at build time, or disabled by user config, or by well defined
   rule ie not permitted during FS freeze.

 - Apply rules later in startup to avoid crash from NULL config
   pointer

 - Document changed error messages in commit messages

 - Add -c / --config command line parameter

 - Fix mistaken enabling of fsfreeze hooks on win32

 - Remove pointless 'blockrpcs_key' variable

 - Allow concurrent setting of allow and block lists for
   RPC commands

Daniel P. Berrangé (22):
  qga: drop blocking of guest-get-memory-block-size command
  qga: move linux vcpu command impls to commands-linux.c
  qga: move linux suspend command impls to commands-linux.c
  qga: move linux fs/disk command impls to commands-linux.c
  qga: move linux disk/cpu stats command impls to commands-linux.c
  qga: move linux memory block command impls to commands-linux.c
  qga: move CONFIG_FSFREEZE/TRIM to be meson defined options
  qga: conditionalize schema for commands unsupported on Windows
  qga: conditionalize schema for commands unsupported on non-Linux POSIX
  qga: conditionalize schema for commands requiring getifaddrs
  qga: conditionalize schema for commands requiring linux/win32
  qga: conditionalize schema for commands only supported on Windows
  qga: conditionalize schema for commands requiring fsfreeze
  qga: conditionalize schema for commands requiring fstrim
  qga: conditionalize schema for commands requiring libudev
  qga: conditionalize schema for commands requiring utmpx
  qga: conditionalize schema for commands not supported on other UNIX
  qga: don't disable fsfreeze commands if vss_init fails
  qga: move declare of QGAConfig struct to top of file
  qga: remove pointless 'blockrpcs_key' variable
  qga: allow configuration file path via the cli
  qga: centralize logic for disabling/enabling commands

 docs/interop/qemu-ga.rst |   19 +
 meson.build              |   16 +
 qga/commands-bsd.c       |   24 -
 qga/commands-common.h    |    9 -
 qga/commands-linux.c     | 1805 +++++++++++++++++++++++++++++
 qga/commands-posix.c     | 2373 +++-----------------------------------
 qga/commands-win32.c     |   78 +-
 qga/main.c               |  216 ++--
 qga/qapi-schema.json     |  153 ++-
 9 files changed, 2234 insertions(+), 2459 deletions(-)

-- 
2.45.1



             reply	other threads:[~2024-06-13 15:02 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-13 15:01 Daniel P. Berrangé [this message]
2024-06-13 15:01 ` [PATCH v2 01/22] qga: drop blocking of guest-get-memory-block-size command Daniel P. Berrangé
2024-07-12  9:33   ` Konstantin Kostiuk
2024-06-13 15:01 ` [PATCH v2 02/22] qga: move linux vcpu command impls to commands-linux.c Daniel P. Berrangé
2024-07-03  8:45   ` Philippe Mathieu-Daudé
2024-07-12  8:30   ` Konstantin Kostiuk
2024-06-13 15:01 ` [PATCH v2 03/22] qga: move linux suspend " Daniel P. Berrangé
2024-07-03  8:45   ` Philippe Mathieu-Daudé
2024-07-12  8:29   ` Konstantin Kostiuk
2024-06-13 15:01 ` [PATCH v2 04/22] qga: move linux fs/disk " Daniel P. Berrangé
2024-07-03  8:46   ` Philippe Mathieu-Daudé
2024-07-12  8:29   ` Konstantin Kostiuk
2024-06-13 15:01 ` [PATCH v2 05/22] qga: move linux disk/cpu stats " Daniel P. Berrangé
2024-07-03  8:25   ` Philippe Mathieu-Daudé
2024-07-12  8:33   ` Konstantin Kostiuk
2024-06-13 15:43 ` [PATCH v2 06/22] qga: move linux memory block " Daniel P. Berrangé
2024-06-13 15:43   ` [PATCH v2 07/22] qga: move CONFIG_FSFREEZE/TRIM to be meson defined options Daniel P. Berrangé
2024-06-13 15:43   ` [PATCH v2 08/22] qga: conditionalize schema for commands unsupported on Windows Daniel P. Berrangé
2024-07-03  8:30     ` Philippe Mathieu-Daudé
2024-07-12  8:34     ` Konstantin Kostiuk
2024-06-13 15:43   ` [PATCH v2 09/22] qga: conditionalize schema for commands unsupported on non-Linux POSIX Daniel P. Berrangé
2024-07-03  8:31     ` Philippe Mathieu-Daudé
2024-07-12  8:35     ` Konstantin Kostiuk
2024-06-13 15:43   ` [PATCH v2 10/22] qga: conditionalize schema for commands requiring getifaddrs Daniel P. Berrangé
2024-07-03  8:32     ` Philippe Mathieu-Daudé
2024-07-12  8:35     ` Konstantin Kostiuk
2024-06-13 15:43   ` [PATCH v2 11/22] qga: conditionalize schema for commands requiring linux/win32 Daniel P. Berrangé
2024-06-13 15:43   ` [PATCH v2 12/22] qga: conditionalize schema for commands only supported on Windows Daniel P. Berrangé
2024-07-03  8:35     ` Philippe Mathieu-Daudé
2024-07-12  8:37     ` Konstantin Kostiuk
2024-06-13 15:43   ` [PATCH v2 13/22] qga: conditionalize schema for commands requiring fsfreeze Daniel P. Berrangé
2024-07-03  8:37     ` Philippe Mathieu-Daudé
2024-07-12  8:37     ` Konstantin Kostiuk
2024-06-13 15:43   ` [PATCH v2 14/22] qga: conditionalize schema for commands requiring fstrim Daniel P. Berrangé
2024-07-03  8:36     ` Philippe Mathieu-Daudé
2024-07-12  8:38     ` Konstantin Kostiuk
2024-06-13 15:43   ` [PATCH v2 15/22] qga: conditionalize schema for commands requiring libudev Daniel P. Berrangé
2024-07-03  8:37     ` Philippe Mathieu-Daudé
2024-07-12  8:40     ` Konstantin Kostiuk
2024-06-13 15:44   ` [PATCH v2 16/22] qga: conditionalize schema for commands requiring utmpx Daniel P. Berrangé
2024-07-03  8:38     ` Philippe Mathieu-Daudé
2024-07-12  8:43     ` Konstantin Kostiuk
2024-06-13 15:44   ` [PATCH v2 17/22] qga: conditionalize schema for commands not supported on other UNIX Daniel P. Berrangé
2024-07-03  8:39     ` Philippe Mathieu-Daudé
2024-07-12  8:43     ` Konstantin Kostiuk
2024-06-13 15:44   ` [PATCH v2 18/22] qga: don't disable fsfreeze commands if vss_init fails Daniel P. Berrangé
2024-07-03 10:21     ` Manos Pitsidianakis
2024-07-12 12:45       ` Daniel P. Berrangé
2024-06-13 15:44   ` [PATCH v2 19/22] qga: move declare of QGAConfig struct to top of file Daniel P. Berrangé
2024-07-03  8:40     ` Philippe Mathieu-Daudé
2024-07-12  8:44     ` Konstantin Kostiuk
2024-06-13 15:44   ` [PATCH v2 20/22] qga: remove pointless 'blockrpcs_key' variable Daniel P. Berrangé
2024-07-03  8:41     ` Philippe Mathieu-Daudé
2024-07-12  8:46     ` Konstantin Kostiuk
2024-06-13 15:44   ` [PATCH v2 21/22] qga: allow configuration file path via the cli Daniel P. Berrangé
2024-07-03  8:44     ` Philippe Mathieu-Daudé
2024-07-12  9:05     ` Konstantin Kostiuk
2024-07-12  9:18       ` Daniel P. Berrangé
2024-06-13 15:44   ` [PATCH v2 22/22] qga: centralize logic for disabling/enabling commands Daniel P. Berrangé
2024-07-03 10:01     ` Manos Pitsidianakis
2024-07-03 12:09       ` Philippe Mathieu-Daudé
2024-07-12 13:01       ` Daniel P. Berrangé
2024-07-03  8:26   ` [PATCH v2 06/22] qga: move linux memory block command impls to commands-linux.c Philippe Mathieu-Daudé
2024-07-12  8:34   ` Konstantin Kostiuk
2024-06-14  8:34 ` [PATCH v2 00/22] qga: clean up command source locations and conditionals Marc-André Lureau
2024-06-14  9:19   ` Daniel P. Berrangé
2024-07-02 18:00 ` Daniel P. Berrangé
2024-07-03  6:15   ` Marc-André Lureau
2024-07-03  8:06     ` Daniel P. Berrangé
2024-07-03  8:17       ` Marc-André Lureau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240613150127.1361931-1-berrange@redhat.com \
    --to=berrange@redhat.com \
    --cc=kkostiuk@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=michael.roth@amd.com \
    --cc=pbonzini@redhat.com \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).