Re: [Xen-devel] [PATCH QEMU-XEN v3 7/8] xen: domainbuild: reopen libxenctrl interface after forking for domain watcher.

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 7 Oct 2015, Ian Campbell wrote:
> Using an existing libxenctrl handle after a fork was never
> particularly safe (especially if foreign mappings existed at the time
> of the fork) and the xc fd has been unavailable for many releases.
> 
> Reopen the handle after fork and therefore do away with xc_fd().
> 
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
>
> The fact that xc_fd hasn't been useful since at least Xen 4.1 makes me
> question the utility of this domainbuild in QEMU. Perhaps we should
> just nuke it?

Indeed

Acked-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>

>  hw/xenpv/xen_domainbuild.c  |  9 ++++++---
>  include/hw/xen/xen_common.h | 17 -----------------
>  2 files changed, 6 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/xenpv/xen_domainbuild.c b/hw/xenpv/xen_domainbuild.c
> index c0ab753..3e8422f 100644
> --- a/hw/xenpv/xen_domainbuild.c
> +++ b/hw/xenpv/xen_domainbuild.c
> @@ -174,12 +174,15 @@ static int xen_domain_watcher(void)
>      for (i = 3; i < n; i++) {
>          if (i == fd[0])
>              continue;
> -        if (i == xc_fd(xen_xc)) {
> -            continue;
> -        }
>          close(i);
>      }
>  
> +    /*
> +     * Reopen xc interface, since the original is unsafe after fork
> +     * and was closed above.
> +     */
> +    xen_xc = xc_interface_open(0, 0, 0);
> +
>      /* ignore term signals */
>      signal(SIGINT,  SIG_IGN);
>      signal(SIGTERM, SIG_IGN);
> diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
> index 1b824aa..a7c1ca0 100644
> --- a/include/hw/xen/xen_common.h
> +++ b/include/hw/xen/xen_common.h
> @@ -116,12 +116,6 @@ static inline XenXC xen_xc_interface_open(void *logger, 
> void *dombuild_logger,
>  #define xenforeignmemory_map(h,d,p,a,e,n) xc_map_foreign_bulk(*h,d,p,a,e,n)
>  #define xenforeignmemory_unmap(h,p,s) munmap(p, s)
>  
> -static inline int xc_fd(int xen_xc)
> -{
> -    return xen_xc;
> -}
> -
> -
>  static inline int xc_domain_populate_physmap_exact
>      (XenXC xc_handle, uint32_t domid, unsigned long nr_extents,
>       unsigned int extent_order, unsigned int mem_flags, xen_pfn_t 
> *extent_start)
> @@ -193,11 +187,6 @@ static inline XenXC xen_xc_interface_open(void *logger, 
> void *dombuild_logger,
>  #define xenforeignmemory_map(h,d,p,a,e,n) xc_map_foreign_bulk(*h,d,p,a,e,n)
>  #define xenforeignmemory_unmap(h,p,s) munmap(p, s)
>  
> -/* FIXME There is now way to have the xen fd */
> -static inline int xc_fd(xc_interface *xen_xc)
> -{
> -    return -1;
> -}
>  #else /* CONFIG_XEN_CTRL_INTERFACE_VERSION >= 470 */
>  
>  #ifndef CONFIG_XEN_STABLE_LIBS
> @@ -219,12 +208,6 @@ static inline XenXC xen_xc_interface_open(void *logger, 
> void *dombuild_logger,
>      return xc_interface_open(logger, dombuild_logger, open_flags);
>  }
>  
> -/* FIXME There is now way to have the xen fd */
> -static inline int xc_fd(xc_interface *xen_xc)
> -{
> -    return -1;
> -}
> -
>  #endif
>  
>  /* Xen before 4.2 */
> -- 
> 2.1.4
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel