Update TODO list
Wrap opendir in talloc so it gets cleaned up on OOM.
Remove last call to system by open-coding "cp -al" to create
transaction.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Index: TODO
==================================================================RCS file:
/var/cvs/xeno-unstable/tools/xenstore/TODO,v
retrieving revision 1.2
retrieving revision 1.3
diff -u -r1.2 -r1.3
--- TODO 18 May 2005 06:29:53 -0000 1.2
+++ TODO 23 Jun 2005 08:23:33 -0000 1.3
@@ -4,4 +4,6 @@
- Remove calls to system() from daemon
- Timeout failed watch responses
-- Timeout blocking transactions
+- Dynamic nodes
+- Persistant storage of introductions, watches and transactions, so daemon can
restart
+- Remove assumption that rename doesn''t fail
Index: xenstored_core.c
==================================================================RCS file:
/var/cvs/xeno-unstable/tools/xenstore/xenstored_core.c,v
retrieving revision 1.3
retrieving revision 1.6
diff -u -r1.3 -r1.6
--- xenstored_core.c 21 Jun 2005 12:00:32 -0000 1.3
+++ xenstored_core.c 24 Jun 2005 03:54:07 -0000 1.6
@@ -246,7 +246,7 @@
}
/* Read everything from a talloc_open''ed fd. */
-static void *read_all(int *fd, unsigned int *size)
+void *read_all(int *fd, unsigned int *size)
{
unsigned int max = 4;
int ret;
@@ -271,7 +271,7 @@
}
/* Return a pointer to an fd, self-closing and attached to this pathname. */
-static int *talloc_open(const char *pathname, int flags, int mode)
+int *talloc_open(const char *pathname, int flags, int mode)
{
int *fd;
@@ -534,6 +534,30 @@
return tmppath;
}
+static int destroy_opendir(void *_dir)
+{
+ DIR **dir = _dir;
+ closedir(*dir);
+ return 0;
+}
+
+/* Return a pointer to a DIR*, self-closing and attached to this pathname. */
+DIR **talloc_opendir(const char *pathname)
+{
+ DIR **dir;
+
+ dir = talloc(pathname, DIR *);
+ *dir = opendir(pathname);
+ if (!*dir) {
+ int saved_errno = errno;
+ talloc_free(dir);
+ errno = saved_errno;
+ return NULL;
+ }
+ talloc_set_destructor(dir, destroy_opendir);
+ return dir;
+}
+
/* We assume rename() doesn''t fail on moves in same dir. */
static void commit_tempfile(const char *path)
{
@@ -677,7 +701,7 @@
{
char *path, *reply = talloc_strdup(node, "");
unsigned int reply_len = 0;
- DIR *dir;
+ DIR **dir;
struct dirent *dirent;
node = canonicalize(conn, node);
@@ -685,11 +709,11 @@
return send_error(conn, errno);
path = node_dir(conn->transaction, node);
- dir = opendir(path);
+ dir = talloc_opendir(path);
if (!dir)
return send_error(conn, errno);
- while ((dirent = readdir(dir)) != NULL) {
+ while ((dirent = readdir(*dir)) != NULL) {
int len = strlen(dirent->d_name) + 1;
if (!valid_chars(dirent->d_name))
@@ -699,7 +723,6 @@
strcpy(reply + reply_len, dirent->d_name);
reply_len += len;
}
- closedir(dir);
return send_reply(conn, XS_DIRECTORY, reply, reply_len);
}
@@ -783,7 +806,8 @@
return send_error(conn, EINVAL);
node = canonicalize(conn, vec[0]);
- if (!within_transaction(conn->transaction, node))
+ if (/*suppress error on write outside transaction*/ 0 &&
+ !within_transaction(conn->transaction, node))
return send_error(conn, EROFS);
if (transaction_block(conn, node))
Index: xenstored_core.h
==================================================================RCS file:
/var/cvs/xeno-unstable/tools/xenstore/xenstored_core.h,v
retrieving revision 1.2
retrieving revision 1.3
diff -u -r1.2 -r1.3
--- xenstored_core.h 20 Jun 2005 06:42:24 -0000 1.2
+++ xenstored_core.h 24 Jun 2005 03:54:07 -0000 1.3
@@ -20,6 +20,8 @@
#ifndef _XENSTORED_CORE_H
#define _XENSTORED_CORE_H
+#include <sys/types.h>
+#include <dirent.h>
#include <stdbool.h>
#include <stdint.h>
#include <errno.h>
@@ -129,7 +131,16 @@
/* Is this a valid node name? */
bool is_valid_nodename(const char *node);
+/* Return a pointer to an open dir, self-closig and attached to pathname. */
+DIR **talloc_opendir(const char *pathname);
+
+/* Return a pointer to an fd, self-closing and attached to this pathname. */
+int *talloc_open(const char *pathname, int flags, int mode);
+
/* Convenient talloc-style destructor for paths. */
int destroy_path(void *path);
+/* Read entire contents of a talloced fd. */
+void *read_all(int *fd, unsigned int *size);
+
#endif /* _XENSTORED_CORE_H */
Index: xenstored_transaction.c
==================================================================RCS file:
/var/cvs/xeno-unstable/tools/xenstore/xenstored_transaction.c,v
retrieving revision 1.1
retrieving revision 1.2
diff -u -r1.1 -r1.2
--- xenstored_transaction.c 20 Jun 2005 04:38:54 -0000 1.1
+++ xenstored_transaction.c 24 Jun 2005 03:54:07 -0000 1.2
@@ -25,6 +25,7 @@
#include <time.h>
#include <stdarg.h>
#include <stdlib.h>
+#include <fcntl.h>
#include "talloc.h"
#include "list.h"
#include "xenstored_transaction.h"
@@ -137,7 +138,7 @@
char *node_dir_inside_transaction(struct transaction *trans, const char *node)
{
- return talloc_asprintf(node, "%s%s", trans->divert,
+ return talloc_asprintf(node, "%s/%s", trans->divert,
node + strlen(trans->node));
}
@@ -170,33 +171,74 @@
}
}
-/* FIXME: Eliminate all uses of this */
-static bool do_command(const char *cmd)
+static int destroy_transaction(void *_transaction)
+{
+ struct transaction *trans = _transaction;
+
+ list_del(&trans->list);
+ return destroy_path(trans->divert);
+}
+
+static bool copy_file(const char *src, const char *dst)
{
- int ret;
+ int *infd, *outfd;
+ void *data;
+ unsigned int size;
- ret = system(cmd);
- if (ret == -1)
+ infd = talloc_open(src, O_RDONLY, 0);
+ if (!infd)
return false;
- if (!WIFEXITED(ret) || WEXITSTATUS(ret) != 0) {
- errno = EIO;
+ outfd = talloc_open(dst, O_WRONLY|O_CREAT|O_EXCL, 0640);
+ if (!outfd)
return false;
- }
- return true;
+ data = read_all(infd, &size);
+ if (!data)
+ return false;
+ return xs_write_all(*outfd, data, size);
}
-static int destroy_transaction(void *_transaction)
+static bool copy_dir(const char *src, const char *dst)
{
- struct transaction *trans = _transaction;
+ DIR **dir;
+ struct dirent *dirent;
- list_del(&trans->list);
- return destroy_path(trans->divert);
+ if (mkdir(dst, 0750) != 0)
+ return false;
+
+ dir = talloc_opendir(src);
+ if (!dir)
+ return false;
+
+ while ((dirent = readdir(*dir)) != NULL) {
+ struct stat st;
+ char *newsrc, *newdst;
+
+ if (streq(dirent->d_name, ".") || streq(dirent->d_name,
".."))
+ continue;
+
+ newsrc = talloc_asprintf(src, "%s/%s", src, dirent->d_name);
+ newdst = talloc_asprintf(src, "%s/%s", dst, dirent->d_name);
+ if (stat(newsrc, &st) != 0)
+ return false;
+
+ if (S_ISDIR(st.st_mode)) {
+ if (!copy_dir(newsrc, newdst))
+ return false;
+ } else {
+ if (!copy_file(newsrc, newdst))
+ return false;
+ }
+ /* Free now so we don''t run out of file descriptors */
+ talloc_free(newsrc);
+ talloc_free(newdst);
+ }
+ return true;
}
bool do_transaction_start(struct connection *conn, const char *node)
{
struct transaction *transaction;
- char *dir, *cmd;
+ char *dir;
if (conn->transaction)
return send_error(conn, EBUSY);
@@ -213,14 +255,9 @@
/* Attach transaction to node for autofree until it''s complete */
transaction = talloc(node, struct transaction);
transaction->node = talloc_strdup(transaction, node);
- transaction->divert = talloc_asprintf(transaction, "%s/%p/",
+ transaction->divert = talloc_asprintf(transaction, "%s/%p",
xs_daemon_transactions(),
transaction);
- cmd = talloc_asprintf(node, "cp -a %s %s", dir,
transaction->divert);
- if (!do_command(cmd))
- corrupt(conn, "Creating transaction %s", transaction->divert);
-
- talloc_steal(conn, transaction);
INIT_LIST_HEAD(&transaction->changes);
transaction->conn = conn;
timerclear(&transaction->timeout);
@@ -228,6 +265,11 @@
list_add_tail(&transaction->list, &transactions);
conn->transaction = transaction;
talloc_set_destructor(transaction, destroy_transaction);
+
+ if (!copy_dir(dir, transaction->divert))
+ return send_error(conn, errno);
+
+ talloc_steal(conn, transaction);
return send_ack(transaction->conn, XS_TRANSACTION_START);
}
--
A bad analogy is like a leaky screwdriver -- Richard Braakman
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
On Fri, 2005-06-24 at 15:57 +1000, Rusty Russell wrote:> Update TODO list > Wrap opendir in talloc so it gets cleaned up on OOM. > Remove last call to system by open-coding "cp -al" to create transaction. > > Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>Oops, disabling EROFS outside transaction snuck in. Here''s the right patch: Update TODO list Wrap opendir in talloc so it gets cleaned up on OOM. Remove last call to system by open-coding "cp -al" to create transaction. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Index: TODO ==================================================================RCS file: /var/cvs/xeno-unstable/tools/xenstore/TODO,v retrieving revision 1.2 retrieving revision 1.3 diff -u -r1.2 -r1.3 --- TODO 18 May 2005 06:29:53 -0000 1.2 +++ TODO 23 Jun 2005 08:23:33 -0000 1.3 @@ -4,4 +4,6 @@ - Remove calls to system() from daemon - Timeout failed watch responses -- Timeout blocking transactions +- Dynamic nodes +- Persistant storage of introductions, watches and transactions, so daemon can restart +- Remove assumption that rename doesn''t fail Index: xenstored_core.c ==================================================================RCS file: /var/cvs/xeno-unstable/tools/xenstore/xenstored_core.c,v retrieving revision 1.3 retrieving revision 1.6 diff -u -r1.3 -r1.6 --- xenstored_core.c 21 Jun 2005 12:00:32 -0000 1.3 +++ xenstored_core.c 24 Jun 2005 03:54:07 -0000 1.6 @@ -246,7 +246,7 @@ } /* Read everything from a talloc_open''ed fd. */ -static void *read_all(int *fd, unsigned int *size) +void *read_all(int *fd, unsigned int *size) { unsigned int max = 4; int ret; @@ -271,7 +271,7 @@ } /* Return a pointer to an fd, self-closing and attached to this pathname. */ -static int *talloc_open(const char *pathname, int flags, int mode) +int *talloc_open(const char *pathname, int flags, int mode) { int *fd; @@ -534,6 +534,30 @@ return tmppath; } +static int destroy_opendir(void *_dir) +{ + DIR **dir = _dir; + closedir(*dir); + return 0; +} + +/* Return a pointer to a DIR*, self-closing and attached to this pathname. */ +DIR **talloc_opendir(const char *pathname) +{ + DIR **dir; + + dir = talloc(pathname, DIR *); + *dir = opendir(pathname); + if (!*dir) { + int saved_errno = errno; + talloc_free(dir); + errno = saved_errno; + return NULL; + } + talloc_set_destructor(dir, destroy_opendir); + return dir; +} + /* We assume rename() doesn''t fail on moves in same dir. */ static void commit_tempfile(const char *path) { @@ -677,7 +701,7 @@ { char *path, *reply = talloc_strdup(node, ""); unsigned int reply_len = 0; - DIR *dir; + DIR **dir; struct dirent *dirent; node = canonicalize(conn, node); @@ -685,11 +709,11 @@ return send_error(conn, errno); path = node_dir(conn->transaction, node); - dir = opendir(path); + dir = talloc_opendir(path); if (!dir) return send_error(conn, errno); - while ((dirent = readdir(dir)) != NULL) { + while ((dirent = readdir(*dir)) != NULL) { int len = strlen(dirent->d_name) + 1; if (!valid_chars(dirent->d_name)) @@ -699,7 +723,6 @@ strcpy(reply + reply_len, dirent->d_name); reply_len += len; } - closedir(dir); return send_reply(conn, XS_DIRECTORY, reply, reply_len); } Index: xenstored_core.h ==================================================================RCS file: /var/cvs/xeno-unstable/tools/xenstore/xenstored_core.h,v retrieving revision 1.2 retrieving revision 1.3 diff -u -r1.2 -r1.3 --- xenstored_core.h 20 Jun 2005 06:42:24 -0000 1.2 +++ xenstored_core.h 24 Jun 2005 03:54:07 -0000 1.3 @@ -20,6 +20,8 @@ #ifndef _XENSTORED_CORE_H #define _XENSTORED_CORE_H +#include <sys/types.h> +#include <dirent.h> #include <stdbool.h> #include <stdint.h> #include <errno.h> @@ -129,7 +131,16 @@ /* Is this a valid node name? */ bool is_valid_nodename(const char *node); +/* Return a pointer to an open dir, self-closig and attached to pathname. */ +DIR **talloc_opendir(const char *pathname); + +/* Return a pointer to an fd, self-closing and attached to this pathname. */ +int *talloc_open(const char *pathname, int flags, int mode); + /* Convenient talloc-style destructor for paths. */ int destroy_path(void *path); +/* Read entire contents of a talloced fd. */ +void *read_all(int *fd, unsigned int *size); + #endif /* _XENSTORED_CORE_H */ Index: xenstored_transaction.c ==================================================================RCS file: /var/cvs/xeno-unstable/tools/xenstore/xenstored_transaction.c,v retrieving revision 1.1 retrieving revision 1.2 diff -u -r1.1 -r1.2 --- xenstored_transaction.c 20 Jun 2005 04:38:54 -0000 1.1 +++ xenstored_transaction.c 24 Jun 2005 03:54:07 -0000 1.2 @@ -25,6 +25,7 @@ #include <time.h> #include <stdarg.h> #include <stdlib.h> +#include <fcntl.h> #include "talloc.h" #include "list.h" #include "xenstored_transaction.h" @@ -137,7 +138,7 @@ char *node_dir_inside_transaction(struct transaction *trans, const char *node) { - return talloc_asprintf(node, "%s%s", trans->divert, + return talloc_asprintf(node, "%s/%s", trans->divert, node + strlen(trans->node)); } @@ -170,33 +171,74 @@ } } -/* FIXME: Eliminate all uses of this */ -static bool do_command(const char *cmd) +static int destroy_transaction(void *_transaction) +{ + struct transaction *trans = _transaction; + + list_del(&trans->list); + return destroy_path(trans->divert); +} + +static bool copy_file(const char *src, const char *dst) { - int ret; + int *infd, *outfd; + void *data; + unsigned int size; - ret = system(cmd); - if (ret == -1) + infd = talloc_open(src, O_RDONLY, 0); + if (!infd) return false; - if (!WIFEXITED(ret) || WEXITSTATUS(ret) != 0) { - errno = EIO; + outfd = talloc_open(dst, O_WRONLY|O_CREAT|O_EXCL, 0640); + if (!outfd) return false; - } - return true; + data = read_all(infd, &size); + if (!data) + return false; + return xs_write_all(*outfd, data, size); } -static int destroy_transaction(void *_transaction) +static bool copy_dir(const char *src, const char *dst) { - struct transaction *trans = _transaction; + DIR **dir; + struct dirent *dirent; - list_del(&trans->list); - return destroy_path(trans->divert); + if (mkdir(dst, 0750) != 0) + return false; + + dir = talloc_opendir(src); + if (!dir) + return false; + + while ((dirent = readdir(*dir)) != NULL) { + struct stat st; + char *newsrc, *newdst; + + if (streq(dirent->d_name, ".") || streq(dirent->d_name, "..")) + continue; + + newsrc = talloc_asprintf(src, "%s/%s", src, dirent->d_name); + newdst = talloc_asprintf(src, "%s/%s", dst, dirent->d_name); + if (stat(newsrc, &st) != 0) + return false; + + if (S_ISDIR(st.st_mode)) { + if (!copy_dir(newsrc, newdst)) + return false; + } else { + if (!copy_file(newsrc, newdst)) + return false; + } + /* Free now so we don''t run out of file descriptors */ + talloc_free(newsrc); + talloc_free(newdst); + } + return true; } bool do_transaction_start(struct connection *conn, const char *node) { struct transaction *transaction; - char *dir, *cmd; + char *dir; if (conn->transaction) return send_error(conn, EBUSY); @@ -213,14 +255,9 @@ /* Attach transaction to node for autofree until it''s complete */ transaction = talloc(node, struct transaction); transaction->node = talloc_strdup(transaction, node); - transaction->divert = talloc_asprintf(transaction, "%s/%p/", + transaction->divert = talloc_asprintf(transaction, "%s/%p", xs_daemon_transactions(), transaction); - cmd = talloc_asprintf(node, "cp -a %s %s", dir, transaction->divert); - if (!do_command(cmd)) - corrupt(conn, "Creating transaction %s", transaction->divert); - - talloc_steal(conn, transaction); INIT_LIST_HEAD(&transaction->changes); transaction->conn = conn; timerclear(&transaction->timeout); @@ -228,6 +265,11 @@ list_add_tail(&transaction->list, &transactions); conn->transaction = transaction; talloc_set_destructor(transaction, destroy_transaction); + + if (!copy_dir(dir, transaction->divert)) + return send_error(conn, errno); + + talloc_steal(conn, transaction); return send_ack(transaction->conn, XS_TRANSACTION_START); } -- A bad analogy is like a leaky screwdriver -- Richard Braakman _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel