On Wed, 2021-12-22 at 11:05 -0800, Steve Sistare wrote:
> Add the only-cpr-capable option, which causes qemu to exit with an
> error
> if any devices that are not capable of cpr are added. This
> guarantees that
> a cpr-exec operation will not fail with an unsupported device error.
>
> Signed-off-by: Steve Sistare <steven.sist...@oracle.com>
> ---
> MAINTAINERS | 1 +
> chardev/char-socket.c | 4 ++++
> hw/vfio/common.c | 6 ++++++
> include/sysemu/sysemu.h | 1 +
> migration/migration.c | 5 +++++
> qemu-options.hx | 8 ++++++++
> softmmu/globals.c | 1 +
> softmmu/physmem.c | 5 +++++
> softmmu/vl.c | 14 +++++++++++++-
> stubs/cpr.c | 3 +++
> stubs/meson.build | 1 +
> 11 files changed, 48 insertions(+), 1 deletion(-)
> create mode 100644 stubs/cpr.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index feed239..af5abc3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2998,6 +2998,7 @@ F: migration/cpr.c
> F: qapi/cpr.json
> F: migration/cpr-state.c
> F: stubs/cpr-state.c
> +F: stubs/cpr.c
>
> Record/replay
> M: Pavel Dovgalyuk <pavel.dovga...@ispras.ru>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index c111e17..a4513a7 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -34,6 +34,7 @@
> #include "qapi/clone-visitor.h"
> #include "qapi/qapi-visit-sockets.h"
> #include "qemu/yank.h"
> +#include "sysemu/sysemu.h"
>
> #include "chardev/char-io.h"
> #include "chardev/char-socket.h"
> @@ -1416,6 +1417,9 @@ static void qmp_chardev_open_socket(Chardev
> *chr,
>
> if (!s->tls_creds && !s->is_websock) {
> qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_CPR);
> + } else if (only_cpr_capable) {
> + error_setg(errp, "error: socket %s is not cpr capable due to
> %s option",
> + chr->label, (s->tls_creds ? "TLS" :
> "websocket"));
Should the error be ignored if reopen-on-cpr is set.
> }
>
> /* be isn't opened until we get a connection */
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index f2b4a81..605ffbb 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -38,6 +38,7 @@
> #include "sysemu/kvm.h"
> #include "sysemu/reset.h"
> #include "sysemu/runstate.h"
> +#include "sysemu/sysemu.h"
> #include "trace.h"
> #include "qapi/error.h"
> #include "migration/migration.h"
> @@ -1923,12 +1924,17 @@ static void
> vfio_put_address_space(VFIOAddressSpace *space)
> static int vfio_get_iommu_type(VFIOContainer *container,
> Error **errp)
> {
> + ERRP_GUARD();
> int iommu_types[] = { VFIO_TYPE1v2_IOMMU, VFIO_TYPE1_IOMMU,
> VFIO_SPAPR_TCE_v2_IOMMU,
> VFIO_SPAPR_TCE_IOMMU };
> int i;
>
> for (i = 0; i < ARRAY_SIZE(iommu_types); i++) {
> if (ioctl(container->fd, VFIO_CHECK_EXTENSION,
> iommu_types[i])) {
> + if (only_cpr_capable && !vfio_is_cpr_capable(container,
> errp)) {
> + error_prepend(errp, "only-cpr-capable is specified:
> ");
> + return -EINVAL;
> + }
> return iommu_types[i];
> }
> }
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 8fae667..6241c20 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -9,6 +9,7 @@
> /* vl.c */
>
> extern int only_migratable;
> +extern bool only_cpr_capable;
> extern const char *qemu_name;
> extern QemuUUID qemu_uuid;
> extern bool qemu_uuid_set;
> diff --git a/migration/migration.c b/migration/migration.c
> index 3de11ae..f08db0d 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1257,6 +1257,11 @@ static bool migrate_caps_check(bool *cap_list,
> return false;
> }
>
> + if (cap_list[MIGRATION_CAPABILITY_X_COLO] && only_cpr_capable) {
> + error_setg(errp, "x-colo is not compatible with -only-cpr-
> capable");
> + return false;
> + }
> +
> return true;
> }
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 1859b55..0cbf2e3 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4434,6 +4434,14 @@ SRST
> an unmigratable state.
> ERST
>
> +DEF("only-cpr-capable", 0, QEMU_OPTION_only_cpr_capable, \
> + "-only-cpr-capable allow only cpr capable devices\n",
> QEMU_ARCH_ALL)
> +SRST
> +``-only-cpr-capable``
> + Only allow cpr capable devices, which guarantees that cpr-save
> and
> + cpr-exec will not fail with an unsupported device error.
> +ERST
> +
> DEF("nodefaults", 0, QEMU_OPTION_nodefaults, \
> "-nodefaults don't create default devices\n", QEMU_ARCH_ALL)
> SRST
> diff --git a/softmmu/globals.c b/softmmu/globals.c
> index 7d0fc81..a18fd8d 100644
> --- a/softmmu/globals.c
> +++ b/softmmu/globals.c
> @@ -59,6 +59,7 @@ int boot_menu;
> bool boot_strict;
> uint8_t *boot_splash_filedata;
> int only_migratable; /* turn it off unless user states otherwise */
> +bool only_cpr_capable;
> int icount_align_option;
>
> /* The bytes in qemu_uuid are in the order specified by RFC4122,
> _not_ in the
> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> index e227195..e7869f8 100644
> --- a/softmmu/physmem.c
> +++ b/softmmu/physmem.c
> @@ -47,6 +47,7 @@
> #include "sysemu/dma.h"
> #include "sysemu/hostmem.h"
> #include "sysemu/hw_accel.h"
> +#include "sysemu/sysemu.h"
> #include "sysemu/xen-mapcache.h"
> #include "trace/trace-root.h"
>
> @@ -2010,6 +2011,10 @@ static void ram_block_add(RAMBlock *new_block,
> Error **errp)
> addr = file_ram_alloc(new_block, maxlen, mfd,
> false, false, 0, errp);
> trace_anon_memfd_alloc(name, maxlen, addr, mfd);
> + } else if (only_cpr_capable) {
> + error_setg(errp,
> + "only-cpr-capable requires -machine memfd-
> alloc=on");
> + return;
> } else {
> addr = qemu_anon_ram_alloc(maxlen, &mr->align,
> shared, noreserve);
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 4319e1a..f14e29e 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -2743,11 +2743,20 @@ void qmp_x_exit_preconfig(Error **errp)
> qemu_create_cli_devices();
> qemu_machine_creation_done();
>
> + if (only_cpr_capable && !qemu_chr_is_cpr_capable(errp)) {
> + ; /* not reached due to error_fatal */
> + }
> +
> if (loadvm) {
> load_snapshot(loadvm, NULL, false, NULL, &error_fatal);
> }
> if (replay_mode != REPLAY_MODE_NONE) {
> - replay_vmstate_init();
> + if (only_cpr_capable) {
> + error_setg(errp, "replay is not compatible with -only-
> cpr-capable");
> + /* not reached due to error_fatal */
> + } else {
> + replay_vmstate_init();
> + }
> }
>
> if (incoming) {
> @@ -3507,6 +3516,9 @@ void qemu_init(int argc, char **argv, char
> **envp)
> case QEMU_OPTION_only_migratable:
> only_migratable = 1;
> break;
> + case QEMU_OPTION_only_cpr_capable:
> + only_cpr_capable = true;
> + break;
> case QEMU_OPTION_nodefaults:
> has_defaults = 0;
> break;
> diff --git a/stubs/cpr.c b/stubs/cpr.c
> new file mode 100644
> index 0000000..aaa189e
> --- /dev/null
> +++ b/stubs/cpr.c
> @@ -0,0 +1,3 @@
> +#include "qemu/osdep.h"
> +
> +bool only_cpr_capable;
> diff --git a/stubs/meson.build b/stubs/meson.build
> index 9565c7d..4c9c4ea 100644
> --- a/stubs/meson.build
> +++ b/stubs/meson.build
> @@ -4,6 +4,7 @@ stub_ss.add(files('blk-exp-close-all.c'))
> stub_ss.add(files('blockdev-close-all-bdrv-states.c'))
> stub_ss.add(files('change-state-handler.c'))
> stub_ss.add(files('cmos.c'))
> +stub_ss.add(files('cpr.c'))
> stub_ss.add(files('cpr-state.c'))
> stub_ss.add(files('cpu-get-clock.c'))
> stub_ss.add(files('cpus-get-virtual-clock.c'))
The only-cpr-capable option is a good way to prevent qemu from starting
if some device don't support cpr. But if this option is not provided,
the user still can perform cpr-xxx operation even there are devices
don't support cpr, in this case, the exec() will fail and the original
process cannot recovery.
How about introducing a cpr blocker (as migration blocker does) to
prevent the user from performing cpr-xxx operaton to address the
problem
--
Guoyi Tu
