| Message ID | 1508506702-17704-3-git-send-email-ian.jackson@eu.citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <Ian.Jackson@eu.citrix.com> > Reviewed-by: Anthony PERARD <anthony.perard@citrix.com> > --- > 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 >
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();