Re: [Xen-devel] [PATCH 11 of 12] xenpaging: add watch thread to catch guest shutdown

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2011-06-07 at 10:53 +0100, Olaf Hering wrote:
> # HG changeset patch
> # User Olaf Hering <olaf@xxxxxxxxx>
> # Date 1307437221 -7200
> # Node ID 9484d35ec6e802aa3727cb6332e649b41e80a615
> # Parent  fb445bd61233844f4b9d42b6eca172670f75eb98
> xenpaging: add watch thread to catch guest shutdown
> 
> If xenpaging is started manually then no event is sent to xenpaging when
> the guest is shutdown or rebooted.  Add a watch on the shutdown node to
> leave the loop and gracefully shutdown the pager.
> 
> Signed-off-by: Olaf Hering <olaf@xxxxxxxxx>

There are ways for a domain to shutdown which do not involve the
shutdown node.

The correct way to watch for domain shutdown is to watch the special
"@releaseDomain" node. IIRC this notifies you when _any_ domain has
shutdown so you need to check for the domain you are actually interested
in. Doing this also removes the need for the hacky extra xs_read_watch
which you have -- which seems very racy to me and is almost certainly
incorrect.

Lastly I don't think you need a new thread for this, you can integrate
the xs fd (from xs_fileno()) into your existing poll loop from
xc_wait_for_event_or_timeout (which is a terrible name for a function
which isn't in libxc, this should either be moved into the library or
renamed depending on it's actual use cases. Same for the other xc_* in
xenpaging...)

Ian.

> diff -r fb445bd61233 -r 9484d35ec6e8 tools/xenpaging/Makefile
> --- a/tools/xenpaging/Makefile        Tue Jun 07 11:00:20 2011 +0200
> +++ b/tools/xenpaging/Makefile        Tue Jun 07 11:00:21 2011 +0200
> @@ -8,6 +8,7 @@ POLICY    = default
>  
>  SRC      :=
>  SRCS     += file_ops.c xc.c xenpaging.c policy_$(POLICY).c
> +SRCS     += watch.c
>  
>  CFLAGS   += -Werror
>  CFLAGS   += -Wno-unused
> diff -r fb445bd61233 -r 9484d35ec6e8 tools/xenpaging/watch.c
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/tools/xenpaging/watch.c Tue Jun 07 11:00:21 2011 +0200
> @@ -0,0 +1,79 @@
> +/* watch for guest shutdown in case xenpaging is started manually */
> +#include <unistd.h>
> +#include <pthread.h>
> +#include <string.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <xs.h>
> +#include <xen/xen.h>
> +
> +struct watch_args {
> +    domid_t domain_id;
> +    void (*fn)(void);
> +};
> +
> +static pthread_t watch_thread;
> +static struct watch_args watch_args;
> +static const char shutdown[] = "/control/shutdown";
> +
> +static void *watch_domain(void *arg)
> +{
> +    struct watch_args *wa = arg;
> +    struct xs_handle *xs;
> +    char *dom_path, *path, **vec;
> +    unsigned int num;
> +    size_t malloc_len;
> +    bool ret;
> +
> +    xs = xs_daemon_open_readonly();
> +    if ( xs == NULL )
> +        goto exit;
> +
> +    dom_path = xs_get_domain_path(xs, wa->domain_id);
> +    if ( dom_path == NULL )
> +        goto close_exit;
> +
> +    malloc_len = strlen(dom_path) + strlen(shutdown) + 1;
> +    path = malloc(malloc_len);
> +    if ( path == NULL )
> +        goto close_exit;
> +
> +    snprintf(path, malloc_len, "%s%s", dom_path, shutdown);
> +
> +    ret = xs_watch(xs, path, "");
> +    if ( ret == true )
> +    {
> +        /* first watch fires right away */
> +        vec = xs_read_watch(xs, &num);
> +        free(vec);
> +        /* wait for real event */
> +        vec = xs_read_watch(xs, &num);
> +        free(vec);
> +        xs_unwatch(xs, path, "");
> +    
> +        /* notify pager */
> +        wa->fn();
> +    }
> +
> +    free(path);
> +close_exit:
> +    xs_daemon_close(xs);
> +exit:
> +    pthread_exit(NULL);
> +}
> +
> +void create_watch_thread(domid_t domain_id, void (*fn)(void))
> +{
> +    watch_args.domain_id = domain_id;
> +    watch_args.fn = fn;
> +    pthread_create(&watch_thread, NULL, watch_domain, &watch_args);
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-set-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End: 
> + */
> diff -r fb445bd61233 -r 9484d35ec6e8 tools/xenpaging/xenpaging.c
> --- a/tools/xenpaging/xenpaging.c     Tue Jun 07 11:00:20 2011 +0200
> +++ b/tools/xenpaging/xenpaging.c     Tue Jun 07 11:00:21 2011 +0200
> @@ -57,6 +57,11 @@ static void close_handler(int sig)
>      unlink_pagefile();
>  }
>  
> +static void set_interrupted_quit(void)
> +{
> +    interrupted = SIGQUIT;
> +}
> +
>  static void *init_page(void)
>  {
>      void *buffer;
> @@ -577,6 +582,9 @@ int main(int argc, char *argv[])
>      sigaction(SIGINT,  &act, NULL);
>      sigaction(SIGALRM, &act, NULL);
>  
> +    /* watch for shutdown of domain_id */
> +    create_watch_thread(paging->mem_event.domain_id, set_interrupted_quit);
> +
>      /* Evict pages */
>      for ( i = 0; i < paging->num_pages; i++ )
>      {
> diff -r fb445bd61233 -r 9484d35ec6e8 tools/xenpaging/xenpaging.h
> --- a/tools/xenpaging/xenpaging.h     Tue Jun 07 11:00:20 2011 +0200
> +++ b/tools/xenpaging/xenpaging.h     Tue Jun 07 11:00:21 2011 +0200
> @@ -55,6 +55,7 @@ typedef struct xenpaging_victim {
>      unsigned long gfn;
>  } xenpaging_victim_t;
>  
> +extern void create_watch_thread(domid_t domain_id, void (*fn)(void));
>  
>  #endif // __XEN_PAGING_H__
>  
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel