[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5.1 3/8] xen: defer call to xen_restrict until
From: |
Stefano Stabellini |
Subject: |
Re: [Qemu-devel] [PATCH v5.1 3/8] xen: defer call to xen_restrict until just before os_setup_post |
Date: |
Thu, 26 Oct 2017 14:37:48 -0700 (PDT) |
User-agent: |
Alpine 2.10 (DEB 1266 2009-07-14) |
This patch affects non-Xen components. CC'ing the relevant maintainers.
On Fri, 20 Oct 2017, Ian Jackson wrote:
> We need to restrict *all* the control fds that qemu opens. Looking in
> /proc/PID/fd shows there are many; their allocation seems scattered
> throughout Xen support code in qemu.
>
> We must postpone the restrict call until roughly the same time as qemu
> changes its uid, chroots (if applicable), and so on.
>
> There doesn't seem to be an appropriate hook already. The RunState
> change hook fires at different times depending on exactly what mode
> qemu is operating in.
>
> And it appears that no-one but the Xen code wants a hook at this phase
> of execution. So, introduce a bare call to a new function
> xen_setup_post, just before os_setup_post. Also provide the
> appropriate stub for when Xen compilation is disabled.
>
> We do the restriction before rather than after os_setup_post, because
> xen_restrict may need to open /dev/null, and os_setup_post might have
> called chroot.
>
> Currently this does not work with migration, because when running as
> the Xen device model qemu needs to signal to the toolstack that it is
> ready. It currently does this using xenstore, and for incoming
> migration (but not for ordinary startup) that happens after
> os_setup_post.
>
> It is correct that this happens late: we want the incoming migration
> stream to be processed by a restricted qemu. The fix for this will be
> to do the startup notification a different way, without using
> xenstore. (QMP is probably a reasonable choice.)
>
> So for now this restriction feature cannot be used in conjunction with
> migration. (Note that this is not a regression in this patch, because
> previously the -xen-restrict-domid call was, in fact, simply
> ineffective!) We will revisit this in the Xen 4.11 release cycle.
>
> Signed-off-by: Ian Jackson <address@hidden>
> Reviewed-by: Anthony PERARD <address@hidden>
> ---
> v5: Discuss problems with migration startup notification
> in the commit message.
> v3: Do xen_setup_post just before, not just after, os_setup_post,
> to improve interaction with chroot. Thanks to Ross Lagerwall.
> ---
> hw/i386/xen/xen-hvm.c | 8 --------
> hw/xen/xen-common.c | 13 +++++++++++++
> include/sysemu/sysemu.h | 2 ++
> stubs/xen-hvm.c | 5 +++++
> vl.c | 1 +
> 5 files changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
> index d9ccd5d..7b60ec6 100644
> --- a/hw/i386/xen/xen-hvm.c
> +++ b/hw/i386/xen/xen-hvm.c
> @@ -1254,14 +1254,6 @@ void xen_hvm_init(PCMachineState *pcms, MemoryRegion
> **ram_memory)
> goto err;
> }
>
> - if (xen_domid_restrict) {
> - rc = xen_restrict(xen_domid);
> - if (rc < 0) {
> - error_report("failed to restrict: error %d", errno);
> - goto err;
> - }
> - }
> -
> xen_create_ioreq_server(xen_domid, &state->ioservid);
>
> state->exit.notify = xen_exit_notifier;
> diff --git a/hw/xen/xen-common.c b/hw/xen/xen-common.c
> index 632a938..4056420 100644
> --- a/hw/xen/xen-common.c
> +++ b/hw/xen/xen-common.c
> @@ -117,6 +117,19 @@ static void xen_change_state_handler(void *opaque, int
> running,
> }
> }
>
> +void xen_setup_post(void)
> +{
> + int rc;
> +
> + if (xen_domid_restrict) {
> + rc = xen_restrict(xen_domid);
> + if (rc < 0) {
> + perror("xen: failed to restrict");
> + exit(1);
> + }
> + }
> +}
> +
> static int xen_init(MachineState *ms)
> {
> xen_xc = xc_interface_open(0, 0, 0);
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index b213696..b064a55 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -93,6 +93,8 @@ void qemu_remove_machine_init_done_notifier(Notifier
> *notify);
>
> void qemu_announce_self(void);
>
> +void xen_setup_post(void);
> +
> extern int autostart;
>
> typedef enum {
> diff --git a/stubs/xen-hvm.c b/stubs/xen-hvm.c
> index 3ca6c51..9701feb 100644
> --- a/stubs/xen-hvm.c
> +++ b/stubs/xen-hvm.c
> @@ -13,6 +13,7 @@
> #include "hw/xen/xen.h"
> #include "exec/memory.h"
> #include "qmp-commands.h"
> +#include "sysemu/sysemu.h"
>
> int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
> {
> @@ -61,3 +62,7 @@ void xen_hvm_init(PCMachineState *pcms, MemoryRegion
> **ram_memory)
> void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
> {
> }
> +
> +void xen_setup_post(void)
> +{
> +}
> diff --git a/vl.c b/vl.c
> index fb1f05b..ca06553 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4792,6 +4792,7 @@ int main(int argc, char **argv, char **envp)
> vm_start();
> }
>
> + xen_setup_post();
> os_setup_post();
>
> main_loop();
> --
> 2.1.4
>
- [Qemu-devel] [PATCH 5/8] xen: move xc_interface compatibility fallback further up the file, (continued)
- [Qemu-devel] [PATCH v5.1 3/8] xen: defer call to xen_restrict until just before os_setup_post, Ian Jackson, 2017/10/20
- Re: [Qemu-devel] [PATCH v5.1 3/8] xen: defer call to xen_restrict until just before os_setup_post,
Stefano Stabellini <=
- [Qemu-devel] [PATCH v5.1 6/8] xen: destroy_hvm_domain: Try xendevicemodel_shutdown, Ian Jackson, 2017/10/20
- Re: [Qemu-devel] [PATCH v5.1 6/8] xen: destroy_hvm_domain: Try xendevicemodel_shutdown, Stefano Stabellini, 2017/10/26
- Re: [Qemu-devel] [PATCH v5.1 6/8] xen: destroy_hvm_domain: Try xendevicemodel_shutdown, Ian Jackson, 2017/10/27
- [Qemu-devel] [PATCH v5.1 4/8] xen: destroy_hvm_domain: Move reason into a variable, Ian Jackson, 2017/10/20
- Re: [Qemu-devel] [PATCH v5.1 4/8] xen: destroy_hvm_domain: Move reason into a variable, Stefano Stabellini, 2017/10/26
- [Qemu-devel] [PATCH v5.1 2/8] xen: restrict: use xentoolcore_restrict_all, Ian Jackson, 2017/10/20
- Re: [Qemu-devel] [PATCH v5.1 2/8] xen: restrict: use xentoolcore_restrict_all, Stefano Stabellini, 2017/10/26
- Re: [Qemu-devel] [PATCH v5.1 2/8] xen: restrict: use xentoolcore_restrict_all, Ian Jackson, 2017/10/27
- Re: [Qemu-devel] [PATCH v5.1 2/8] xen: restrict: use xentoolcore_restrict_all, Stefano Stabellini, 2017/10/27
- [Qemu-devel] [PATCH v5.1 7/8] os-posix: Provide new -runas <uid>:<gid> facility, Ian Jackson, 2017/10/20