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-changelog

[Xen-changelog] [xen-3.4-testing] fs-back: better error handling in fs-b

To: xen-changelog@xxxxxxxxxxxxxxxxxxx
Subject: [Xen-changelog] [xen-3.4-testing] fs-back: better error handling in fs-backend
From: "Xen patchbot-3.4-testing" <patchbot-3.4-testing@xxxxxxxxxxxxxxxxxxx>
Date: Mon, 13 Jul 2009 04:00:55 -0700
Delivery-date: Mon, 13 Jul 2009 04:02:25 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-changelog-request@lists.xensource.com?subject=help>
List-id: BK change log <xen-changelog.lists.xensource.com>
List-post: <mailto:xen-changelog@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-changelog>, <mailto:xen-changelog-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-changelog>, <mailto:xen-changelog-request@lists.xensource.com?subject=unsubscribe>
Reply-to: xen-devel@xxxxxxxxxxxxxxxxxxx
Sender: xen-changelog-bounces@xxxxxxxxxxxxxxxxxxx
# HG changeset patch
# User Keir Fraser <keir.fraser@xxxxxxxxxx>
# Date 1247059600 -3600
# Node ID 8d8ca2e56396d97d71c2f02a9d8b3682a9e7842a
# Parent  457be4ce8e7be7128e14f888e847eaa754c4cbb9
fs-back: better error handling in fs-backend

Currently most of the error checking in fs-backend is done by the use
of asserts that would terminate the daemon in case of a single error
on a single request.  This patch replaces the asserts with debugging
messages and terminates the connection on which the error occurred.
With this patch applied I was able to complete successfully over 1000
live migrations with stubdoms.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
xen-unstable changeset:   19910:5c40f649a6a4
xen-unstable date:        Wed Jul 08 10:58:09 2009 +0100
---
 tools/fs-back/fs-backend.c |   67 +++++++++++++++++++++++++++++++++-------
 tools/fs-back/fs-backend.h |   11 +++---
 tools/fs-back/fs-ops.c     |   69 ++++++++++++++++++++++++++++++-----------
 tools/fs-back/fs-xenbus.c  |   75 +++++++++++++++++++++++++--------------------
 4 files changed, 155 insertions(+), 67 deletions(-)

diff -r 457be4ce8e7b -r 8d8ca2e56396 tools/fs-back/fs-backend.c
--- a/tools/fs-back/fs-backend.c        Wed Jul 08 14:26:13 2009 +0100
+++ b/tools/fs-back/fs-backend.c        Wed Jul 08 14:26:40 2009 +0100
@@ -144,7 +144,8 @@ moretodo:
         xc_evtchn_notify(mount->evth, mount->local_evtchn);
 }
 
-static void terminate_mount_request(struct fs_mount *mount) {
+void terminate_mount_request(struct fs_mount *mount)
+{
     int count = 0, i;
 
     FS_DEBUG("terminate_mount_request %s\n", mount->frontend);
@@ -158,7 +159,13 @@ static void terminate_mount_request(stru
         }
     mount->nr_entries = count;
 
-    while (!xenbus_frontend_state_changed(mount, STATE_CLOSING));
+    /* wait for the frontend to shut down but don't wait more than 3
+     * seconds */
+    i = 0;
+    while (!xenbus_frontend_state_changed(mount, STATE_CLOSING) && i < 3) {
+        sleep(1);
+        i++;
+    }
     xenbus_write_backend_state(mount, STATE_CLOSED);
 
     xc_gnttab_munmap(mount->gnth, mount->ring.sring, mount->shared_ring_size);
@@ -183,7 +190,7 @@ static void handle_connection(int fronte
 {
     struct fs_mount *mount;
     struct fs_export *export;
-    struct fsif_sring *sring;
+    struct fsif_sring *sring = NULL;
     uint32_t dom_ids[MAX_RING_SIZE];
     int i;
 
@@ -204,24 +211,38 @@ static void handle_connection(int fronte
     }
 
     mount = (struct fs_mount*)malloc(sizeof(struct fs_mount));
+    memset(mount, 0, sizeof(struct fs_mount));
     mount->dom_id = frontend_dom_id;
     mount->export = export;
     mount->mount_id = mount_id++;
-    xenbus_read_mount_request(mount, frontend);
+    if (xenbus_read_mount_request(mount, frontend) < 0)
+        goto error;
     FS_DEBUG("Frontend found at: %s (gref=%d, evtchn=%d)\n", 
             mount->frontend, mount->grefs[0], mount->remote_evtchn);
-    xenbus_write_backend_node(mount);
+    if (!xenbus_write_backend_node(mount)) {
+        FS_DEBUG("ERROR: failed to write backend node on xenbus\n");
+        goto error;
+    }
     mount->evth = -1;
     mount->evth = xc_evtchn_open(); 
-    assert(mount->evth != -1);
+    if (mount->evth < 0) {
+        FS_DEBUG("ERROR: Couldn't open evtchn!\n");
+        goto error;
+    }
     mount->local_evtchn = -1;
     mount->local_evtchn = xc_evtchn_bind_interdomain(mount->evth, 
                                                      mount->dom_id, 
                                                      mount->remote_evtchn);
-    assert(mount->local_evtchn != -1);
+    if (mount->local_evtchn < 0) {
+        FS_DEBUG("ERROR: Couldn't bind evtchn!\n");
+        goto error;
+    }
     mount->gnth = -1;
     mount->gnth = xc_gnttab_open(); 
-    assert(mount->gnth != -1);
+    if (mount->gnth < 0) {
+        FS_DEBUG("ERROR: Couldn't open gnttab!\n");
+        goto error;
+    }
     for(i=0; i<mount->shared_ring_size; i++)
         dom_ids[i] = mount->dom_id;
     sring = xc_gnttab_map_grant_refs(mount->gnth,
@@ -230,16 +251,40 @@ static void handle_connection(int fronte
                                      mount->grefs,
                                      PROT_READ | PROT_WRITE);
 
+    if (!sring) {
+        FS_DEBUG("ERROR: Couldn't amp grant refs!\n");
+        goto error;
+    }
+
     BACK_RING_INIT(&mount->ring, sring, mount->shared_ring_size * 
XC_PAGE_SIZE);
     mount->nr_entries = mount->ring.nr_ents; 
     for (i = 0; i < MAX_FDS; i++)
         mount->fds[i] = -1;
 
     LIST_INSERT_HEAD(&mount_requests_head, mount, entries);
-    xenbus_watch_frontend_state(mount);
-    xenbus_write_backend_state(mount, STATE_READY);
-    
+    if (!xenbus_watch_frontend_state(mount)) {
+        FS_DEBUG("ERROR: failed to watch frontend state on xenbus\n");
+        goto error;
+    }
+    if (!xenbus_write_backend_state(mount, STATE_READY)) {
+        FS_DEBUG("ERROR: failed to write backend state to xenbus\n");
+        goto error;
+    }
+
     allocate_request_array(mount);
+
+    return;
+
+error:
+    xenbus_write_backend_state(mount, STATE_CLOSED);
+    if (sring)
+        xc_gnttab_munmap(mount->gnth, mount->ring.sring, 
mount->shared_ring_size);
+    if (mount->gnth > 0)
+        xc_gnttab_close(mount->gnth);
+    if (mount->local_evtchn > 0)
+        xc_evtchn_unbind(mount->evth, mount->local_evtchn);
+    if (mount->evth > 0)
+        xc_evtchn_close(mount->evth);
 }
 
 static void await_connections(void)
diff -r 457be4ce8e7b -r 8d8ca2e56396 tools/fs-back/fs-backend.h
--- a/tools/fs-back/fs-backend.h        Wed Jul 08 14:26:13 2009 +0100
+++ b/tools/fs-back/fs-backend.h        Wed Jul 08 14:26:40 2009 +0100
@@ -56,6 +56,7 @@ struct fs_mount
     LIST_ENTRY(fs_mount) entries;
 };
 
+void terminate_mount_request(struct fs_mount *mount);
 
 /* Handle to XenStore driver */
 extern struct xs_handle *xsh;
@@ -63,12 +64,12 @@ bool xenbus_create_request_node(void);
 bool xenbus_create_request_node(void);
 int xenbus_register_export(struct fs_export *export);
 int xenbus_get_watch_fd(void);
-void xenbus_read_mount_request(struct fs_mount *mount, char *frontend);
-void xenbus_write_backend_node(struct fs_mount *mount);
-void xenbus_write_backend_state(struct fs_mount *mount, const char *state);
+int xenbus_read_mount_request(struct fs_mount *mount, char *frontend);
+bool xenbus_write_backend_node(struct fs_mount *mount);
+bool xenbus_write_backend_state(struct fs_mount *mount, const char *state);
 int xenbus_frontend_state_changed(struct fs_mount *mount, const char 
*oldstate);
-void xenbus_watch_frontend_state(struct fs_mount *mount);
-void xenbus_unwatch_frontend_state(struct fs_mount *mount);
+bool xenbus_watch_frontend_state(struct fs_mount *mount);
+bool xenbus_unwatch_frontend_state(struct fs_mount *mount);
 char* xenbus_read_frontend_state(struct fs_mount *mount);
 
 /* File operations, implemented in fs-ops.c */
diff -r 457be4ce8e7b -r 8d8ca2e56396 tools/fs-back/fs-ops.c
--- a/tools/fs-back/fs-ops.c    Wed Jul 08 14:26:13 2009 +0100
+++ b/tools/fs-back/fs-ops.c    Wed Jul 08 14:26:40 2009 +0100
@@ -99,7 +99,10 @@ static void dispatch_file_open(struct fs
         }
     }
 out:
-    assert(xc_gnttab_munmap(mount->gnth, file_name, 1) == 0);
+    if (xc_gnttab_munmap(mount->gnth, file_name, 1) != 0) {
+        FS_DEBUG("ERROR: xc_gnttab_munmap failed errno=%d\n", errno);
+        terminate_mount_request(mount);
+    }
     /* We can advance the request consumer index, from here on, the request
      * should not be used (it may be overrinden by a response) */
     mount->ring.req_cons++;
@@ -187,7 +190,11 @@ static void dispatch_file_read(struct fs
     priv_req->aiocb.aio_sigevent.sigev_notify = SIGEV_SIGNAL;
     priv_req->aiocb.aio_sigevent.sigev_signo = SIGUSR2;
     priv_req->aiocb.aio_sigevent.sigev_value.sival_ptr = priv_req;
-    assert(aio_read(&priv_req->aiocb) >= 0);
+    if (aio_read(&priv_req->aiocb) < 0) {
+        FS_DEBUG("ERROR: aio_read failed errno=%d\n", errno);
+        xc_gnttab_munmap(mount->gnth, priv_req->page, priv_req->count);
+        terminate_mount_request(mount);
+    }
 
     /* We can advance the request consumer index, from here on, the request
      * should not be used (it may be overrinden by a response) */
@@ -201,9 +208,10 @@ static void end_file_read(struct fs_moun
     uint16_t req_id;
 
     /* Release the grant */
-    assert(xc_gnttab_munmap(mount->gnth, 
-                            priv_req->page, 
-                            priv_req->count) == 0);
+    if (xc_gnttab_munmap(mount->gnth, priv_req->page, priv_req->count) != 0) {
+        FS_DEBUG("ERROR: xc_gnttab_munmap failed errno=%d\n", errno);
+        terminate_mount_request(mount);
+    }
 
     /* Get a response from the ring */
     rsp_idx = mount->ring.rsp_prod_pvt++;
@@ -257,7 +265,11 @@ static void dispatch_file_write(struct f
     priv_req->aiocb.aio_sigevent.sigev_notify = SIGEV_SIGNAL;
     priv_req->aiocb.aio_sigevent.sigev_signo = SIGUSR2;
     priv_req->aiocb.aio_sigevent.sigev_value.sival_ptr = priv_req;
-    assert(aio_write(&priv_req->aiocb) >= 0);
+    if (aio_write(&priv_req->aiocb) < 0) {
+        FS_DEBUG("ERROR: aio_write failed errno=%d\n", errno);
+        xc_gnttab_munmap(mount->gnth, priv_req->page, priv_req->count);
+        terminate_mount_request(mount);
+    }
 
      
     /* We can advance the request consumer index, from here on, the request
@@ -272,9 +284,10 @@ static void end_file_write(struct fs_mou
     uint16_t req_id;
 
     /* Release the grant */
-    assert(xc_gnttab_munmap(mount->gnth, 
-                            priv_req->page, 
-                            priv_req->count) == 0);
+    if (xc_gnttab_munmap(mount->gnth, priv_req->page, priv_req->count) != 0) {
+        FS_DEBUG("ERROR: xc_gnttab_munmap failed errno=%d\n", errno);
+        terminate_mount_request(mount);
+    }
     
     /* Get a response from the ring */
     rsp_idx = mount->ring.rsp_prod_pvt++;
@@ -392,7 +405,10 @@ static void dispatch_remove(struct fs_mo
         ret = remove(file_name);
     }
     FS_DEBUG("Got ret: %d\n", ret);
-    assert(xc_gnttab_munmap(mount->gnth, file_name, 1) == 0);
+    if (xc_gnttab_munmap(mount->gnth, file_name, 1) != 0) {
+        FS_DEBUG("ERROR: xc_gnttab_munmap failed errno=%d\n", errno);
+        terminate_mount_request(mount);
+    }
     /* We can advance the request consumer index, from here on, the request
      * should not be used (it may be overrinden by a response) */
     mount->ring.req_cons++;
@@ -435,7 +451,10 @@ static void dispatch_rename(struct fs_mo
         ret = rename(old_file_name, new_file_name);
     }
     FS_DEBUG("Got ret: %d\n", ret);
-    assert(xc_gnttab_munmap(mount->gnth, buf, 1) == 0);
+    if (xc_gnttab_munmap(mount->gnth, buf, 1) != 0) {
+        FS_DEBUG("ERROR: xc_gnttab_munmap failed errno=%d\n", errno);
+        terminate_mount_request(mount);
+    }
     /* We can advance the request consumer index, from here on, the request
      * should not be used (it may be overrinden by a response) */
     mount->ring.req_cons++;
@@ -500,7 +519,10 @@ static void dispatch_create(struct fs_mo
         }
     }
 out:
-    assert(xc_gnttab_munmap(mount->gnth, file_name, 1) == 0);
+    if (xc_gnttab_munmap(mount->gnth, file_name, 1) != 0) {
+        FS_DEBUG("ERROR: xc_gnttab_munmap failed errno=%d\n", errno);
+        terminate_mount_request(mount);
+    }
     FS_DEBUG("Got ret %d (errno=%d)\n", ret, errno);
 
     /* Get a response from the ring */
@@ -556,7 +578,8 @@ static void dispatch_list(struct fs_moun
     /* If there was any error with reading the directory, errno will be set */
     error_code = errno;
     /* Copy file names of the remaining non-NULL dirents into buf */
-    assert(NAME_MAX < XC_PAGE_SIZE >> 1);
+    if (NAME_MAX >= XC_PAGE_SIZE >> 1)
+        goto error_out;
     while(dirent != NULL && 
             (XC_PAGE_SIZE - ((unsigned long)buf & XC_PAGE_MASK) > NAME_MAX))
     {
@@ -572,8 +595,11 @@ error_out:
     ret_val = ((nr_files << NR_FILES_SHIFT) & NR_FILES_MASK) | 
               ((error_code << ERROR_SHIFT) & ERROR_MASK) | 
               (dirent != NULL ? HAS_MORE_FLAG : 0);
-    assert(xc_gnttab_munmap(mount->gnth, file_name, 1) == 0);
-    
+    if (xc_gnttab_munmap(mount->gnth, file_name, 1) != 0) {
+        FS_DEBUG("ERROR: xc_gnttab_munmap failed errno=%d\n", errno);
+        terminate_mount_request(mount);
+    }
+
     /* Get a response from the ring */
     rsp_idx = mount->ring.rsp_prod_pvt++;
     FS_DEBUG("Writing response at: idx=%d, id=%d\n", rsp_idx, req_id);
@@ -640,7 +666,10 @@ static void dispatch_fs_space(struct fs_
     if(ret >= 0)
         ret = stat.f_bsize * stat.f_bfree;
 
-    assert(xc_gnttab_munmap(mount->gnth, file_name, 1) == 0);
+    if (xc_gnttab_munmap(mount->gnth, file_name, 1) != 0) {
+        FS_DEBUG("ERROR: xc_gnttab_munmap failed errno=%d\n", errno);
+        terminate_mount_request(mount);
+    }
     /* We can advance the request consumer index, from here on, the request
      * should not be used (it may be overrinden by a response) */
     mount->ring.req_cons++;
@@ -680,9 +709,11 @@ static void dispatch_file_sync(struct fs
     priv_req->aiocb.aio_sigevent.sigev_notify = SIGEV_SIGNAL;
     priv_req->aiocb.aio_sigevent.sigev_signo = SIGUSR2;
     priv_req->aiocb.aio_sigevent.sigev_value.sival_ptr = priv_req;
-    assert(aio_fsync(O_SYNC, &priv_req->aiocb) >= 0);
-
-     
+    if (aio_fsync(O_SYNC, &priv_req->aiocb) < 0) {
+        FS_DEBUG("ERROR: aio_fsync failed errno=%d\n", errno);
+        terminate_mount_request(mount);
+    }
+
     /* We can advance the request consumer index, from here on, the request
      * should not be used (it may be overrinden by a response) */
     mount->ring.req_cons++;
diff -r 457be4ce8e7b -r 8d8ca2e56396 tools/fs-back/fs-xenbus.c
--- a/tools/fs-back/fs-xenbus.c Wed Jul 08 14:26:13 2009 +0100
+++ b/tools/fs-back/fs-xenbus.c Wed Jul 08 14:26:40 2009 +0100
@@ -107,11 +107,15 @@ int xenbus_get_watch_fd(void)
     int res;
     assert(xsh != NULL);
     res = xs_watch(xsh, WATCH_NODE, "conn-watch");
-    assert(res);
+    if (!res) {
+        FS_DEBUG("ERROR: xs_watch %s failed ret=%d errno=%d\n",
+                 WATCH_NODE, res, errno);
+        return -1;
+    }
     return xs_fileno(xsh); 
 }
 
-void xenbus_read_mount_request(struct fs_mount *mount, char *frontend)
+int xenbus_read_mount_request(struct fs_mount *mount, char *frontend)
 {
     char node[1024];
     char *s;
@@ -126,12 +130,18 @@ void xenbus_read_mount_request(struct fs
     mount->frontend = frontend;
     snprintf(node, sizeof(node), "%s/state", frontend);
     s = xs_read(xsh, XBT_NULL, node, NULL);
-    assert(strcmp(s, STATE_READY) == 0);
+    if (strcmp(s, STATE_READY) != 0) {
+        FS_DEBUG("ERROR: frontend not read\n");
+        goto error;
+    }
     free(s);
     snprintf(node, sizeof(node), "%s/ring-size", frontend);
     s = xs_read(xsh, XBT_NULL, node, NULL);
     mount->shared_ring_size = atoi(s);
-    assert(mount->shared_ring_size <= MAX_RING_SIZE);
+    if (mount->shared_ring_size > MAX_RING_SIZE) {
+        FS_DEBUG("ERROR: shared_ring_size (%d) > MAX_RING_SIZE\n", 
mount->shared_ring_size);
+        goto error;
+    }
     free(s);
     for(i=0; i<mount->shared_ring_size; i++)
     {
@@ -144,6 +154,11 @@ void xenbus_read_mount_request(struct fs
     s = xs_read(xsh, XBT_NULL, node, NULL);
     mount->remote_evtchn = atoi(s);
     free(s);
+    return 0;
+
+error:
+    free(s);
+    return -1;
 }
 
 /* Small utility function to figure out our domain id */
@@ -161,7 +176,7 @@ static int get_self_id(void)
 } 
 
 
-void xenbus_write_backend_node(struct fs_mount *mount)
+bool xenbus_write_backend_node(struct fs_mount *mount)
 {
     char node[1024], backend_node[1024];
     int self_id;
@@ -175,10 +190,10 @@ void xenbus_write_backend_node(struct fs
     xs_write(xsh, XBT_NULL, node, backend_node, strlen(backend_node));
 
     snprintf(node, sizeof(node), ROOT_NODE"/%d/state", mount->mount_id);
-    xs_write(xsh, XBT_NULL, node, STATE_INITIALISED, 
strlen(STATE_INITIALISED));
-}
-
-void xenbus_write_backend_state(struct fs_mount *mount, const char *state)
+    return xs_write(xsh, XBT_NULL, node, STATE_INITIALISED, 
strlen(STATE_INITIALISED));
+}
+
+bool xenbus_write_backend_state(struct fs_mount *mount, const char *state)
 {
     char node[1024];
     int self_id;
@@ -186,29 +201,25 @@ void xenbus_write_backend_state(struct f
     assert(xsh != NULL);
     self_id = get_self_id();
     snprintf(node, sizeof(node), ROOT_NODE"/%d/state", mount->mount_id);
-    xs_write(xsh, XBT_NULL, node, state, strlen(state));
-}
-
-void xenbus_watch_frontend_state(struct fs_mount *mount)
-{
-    int res;
-    char statepath[1024];
-
-    assert(xsh != NULL);
-    snprintf(statepath, sizeof(statepath), "%s/state", mount->frontend);
-    res = xs_watch(xsh, statepath, "frontend-state");
-    assert(res);
-}
-
-void xenbus_unwatch_frontend_state(struct fs_mount *mount)
-{
-    int res;
-    char statepath[1024];
-
-    assert(xsh != NULL);
-    snprintf(statepath, sizeof(statepath), "%s/state", mount->frontend);
-    res = xs_unwatch(xsh, statepath, "frontend-state");
-    assert(res);
+    return xs_write(xsh, XBT_NULL, node, state, strlen(state));
+}
+
+bool xenbus_watch_frontend_state(struct fs_mount *mount)
+{
+    char statepath[1024];
+
+    assert(xsh != NULL);
+    snprintf(statepath, sizeof(statepath), "%s/state", mount->frontend);
+    return xs_watch(xsh, statepath, "frontend-state");
+}
+
+bool xenbus_unwatch_frontend_state(struct fs_mount *mount)
+{
+    char statepath[1024];
+
+    assert(xsh != NULL);
+    snprintf(statepath, sizeof(statepath), "%s/state", mount->frontend);
+    return xs_unwatch(xsh, statepath, "frontend-state");
 }
 
 int xenbus_frontend_state_changed(struct fs_mount *mount, const char *oldstate)

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

<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-changelog] [xen-3.4-testing] fs-back: better error handling in fs-backend, Xen patchbot-3.4-testing <=