WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: Olaf Hering <olaf@xxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 11 of 12] xenpaging: add watch thread to catch guest shutdown
From: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
Date: Tue, 7 Jun 2011 11:23:05 +0100
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Tue, 07 Jun 2011 03:23:37 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <9484d35ec6e802aa3727.1307440393@xxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Organization: Citrix Systems, Inc.
References: <patchbomb.1307440382@xxxxxxxxxxxx> <9484d35ec6e802aa3727.1307440393@xxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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

<Prev in Thread] Current Thread [Next in Thread>