* [PATCH 01/11] libxl: Make logging functions preserve errno
2010-03-25 19:04 [PATCH 00/11] Migration support for "xl" Ian Jackson
@ 2010-03-25 19:04 ` Ian Jackson
2010-03-25 19:04 ` [PATCH 02/11] libxl: Report error if logfile rotation fails Ian Jackson
` (10 subsequent siblings)
11 siblings, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2010-03-25 19:04 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Ian Jackson
This is needed by the following patches. It makes it much more
convenient for libxl functions to return the errno value from the
failure, when they fail.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
tools/libxl/libxl_internal.c | 5 ++++-
tools/libxl/libxl_internal.h | 2 ++
2 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index f48e9eb..4dcdabe 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -156,11 +156,13 @@ void xl_logv(struct libxl_ctx *ctx, int loglevel, int errnoval,
{
char *enomem = "[out of memory formatting log message]";
char *s;
- int rc;
+ int rc, esave;
if (!ctx->log_callback)
return;
+ esave = errno;
+
rc = vasprintf(&s, fmt, ap);
if (rc<0) { s = enomem; goto x; }
@@ -180,6 +182,7 @@ void xl_logv(struct libxl_ctx *ctx, int loglevel, int errnoval,
ctx->log_callback(ctx->log_userdata, loglevel, file, line, func, s);
if (s != enomem)
free(s);
+ errno = esave;
}
void xl_log(struct libxl_ctx *ctx, int loglevel, int errnoval,
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 300c108..10e72e5 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -47,6 +47,7 @@
#define XL_LOG_ERRNO(ctx, loglevel, _f, _a...)
#define XL_LOG_ERRNOVAL(ctx, loglevel, errnoval, _f, _a...)
#endif
+ /* all of these macros preserve errno (saving and restoring) */
#define XL_LOG_DEBUG 3
#define XL_LOG_INFO 2
@@ -56,6 +57,7 @@
/* logging */
void xl_logv(struct libxl_ctx *ctx, int errnoval, int loglevel, const char *file, int line, const char *func, char *fmt, va_list al);
void xl_log(struct libxl_ctx *ctx, int errnoval, int loglevel, const char *file, int line, const char *func, char *fmt, ...);
+ /* these functions preserve errno (saving and restoring) */
typedef enum {
--
1.5.6.5
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 02/11] libxl: Report error if logfile rotation fails
2010-03-25 19:04 [PATCH 00/11] Migration support for "xl" Ian Jackson
2010-03-25 19:04 ` [PATCH 01/11] libxl: Make logging functions preserve errno Ian Jackson
@ 2010-03-25 19:04 ` Ian Jackson
2010-03-25 19:04 ` [PATCH 03/11] libxl: New utility functions in for reading and writing files Ian Jackson
` (9 subsequent siblings)
11 siblings, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2010-03-25 19:04 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Ian Jackson
Check the return values from renames and errors from stat in
libxl_create_logfile (which, misleadingly, does not actually create
the logfile).
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
tools/libxl/libxl_utils.c | 28 +++++++++++++++++++++++++---
1 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 1ba9431..afc852a 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -101,11 +101,25 @@ int libxl_is_stubdom(struct libxl_ctx *ctx, uint32_t domid, uint32_t *target_dom
return 1;
}
+static int logrename(struct libxl_ctx *ctx, const char *old, const char *new) {
+ int r;
+
+ r = rename(old, new);
+ if (r) {
+ if (errno == ENOENT) return 0; /* ok */
+
+ XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "failed to rotate logfile - could not"
+ " rename %s to %s", old, new);
+ return ERROR_FAIL;
+ }
+ return 0;
+}
+
int libxl_create_logfile(struct libxl_ctx *ctx, char *name, char **full_name)
{
struct stat stat_buf;
char *logfile, *logfile_new;
- int i;
+ int i, rc;
logfile = libxl_sprintf(ctx, "/var/log/xen/%s.log", name);
if (stat(logfile, &stat_buf) == 0) {
@@ -115,11 +129,19 @@ int libxl_create_logfile(struct libxl_ctx *ctx, char *name, char **full_name)
for (i = 9; i > 0; i--) {
logfile = libxl_sprintf(ctx, "/var/log/xen/%s.log.%d", name, i);
logfile_new = libxl_sprintf(ctx, "/var/log/xen/%s.log.%d", name, i + 1);
- rename(logfile, logfile_new);
+ rc = logrename(ctx, logfile, logfile_new);
+ if (rc) return rc;
}
logfile = libxl_sprintf(ctx, "/var/log/xen/%s.log", name);
logfile_new = libxl_sprintf(ctx, "/var/log/xen/%s.log.1", name);
- rename(logfile, logfile_new);
+
+ rc = logrename(ctx, logfile, logfile_new);
+ if (rc) return rc;
+ } else {
+ if (errno != ENOENT)
+ XL_LOG_ERRNO(ctx, XL_LOG_WARNING, "problem checking existence of"
+ " logfile %s, which might have needed to be rotated",
+ name);
}
*full_name = strdup(logfile);
return 0;
--
1.5.6.5
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 03/11] libxl: New utility functions in for reading and writing files.
2010-03-25 19:04 [PATCH 00/11] Migration support for "xl" Ian Jackson
2010-03-25 19:04 ` [PATCH 01/11] libxl: Make logging functions preserve errno Ian Jackson
2010-03-25 19:04 ` [PATCH 02/11] libxl: Report error if logfile rotation fails Ian Jackson
@ 2010-03-25 19:04 ` Ian Jackson
2010-03-25 19:04 ` [PATCH 04/11] libxl: libxl_domain_restore: Put fd back to blocking mode Ian Jackson
` (8 subsequent siblings)
11 siblings, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2010-03-25 19:04 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Ian Jackson
We introduce these functions in libxl_utils.h:
int libxl_read_file_contents(struct libxl_ctx *ctx, const char *filename,
void **data_r, int *datalen_r);
int libxl_read_exactly(struct libxl_ctx *ctx, int fd, void *data, ssize_t sz,
const char *filename, const char *what);
int libxl_write_exactly(struct libxl_ctx *ctx, int fd, const void *data,
ssize_t sz, const char *filename, const char *what);
They will be needed by the following patches. They have to be in
libxl.a rather than libxutil.a because they will be used, amongst
other places, in libxl itself.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
tools/libxl/libxl_utils.c | 103 +++++++++++++++++++++++++++++++++++++++++++++
tools/libxl/libxl_utils.h | 18 ++++++++
2 files changed, 121 insertions(+), 0 deletions(-)
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index afc852a..3a1f095 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -26,6 +26,7 @@
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>
+#include <assert.h>
#include "libxl_utils.h"
#include "libxl_internal.h"
@@ -177,3 +178,105 @@ out:
return rc;
}
+int libxl_read_file_contents(struct libxl_ctx *ctx, const char *filename,
+ void **data_r, int *datalen_r) {
+ FILE *f = 0;
+ uint8_t *data = 0;
+ int datalen = 0;
+ int e;
+ struct stat stab;
+ ssize_t rs;
+
+ f = fopen(filename, "r");
+ if (!f) {
+ if (errno == ENOENT) return ENOENT;
+ XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "failed to open %s", filename);
+ goto xe;
+ }
+
+ if (fstat(fileno(f), &stab)) {
+ XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "failed to fstat %s", filename);
+ goto xe;
+ }
+
+ if (!S_ISREG(stab.st_mode)) {
+ XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "%s is not a plain file", filename);
+ errno = ENOTTY;
+ goto xe;
+ }
+
+ if (stab.st_size > INT_MAX) {
+ XL_LOG(ctx, XL_LOG_ERROR, "file %s is far too large", filename);
+ errno = EFBIG;
+ goto xe;
+ }
+
+ datalen = stab.st_size;
+
+ if (stab.st_size && data_r) {
+ data = malloc(datalen);
+ if (!data) goto xe;
+
+ rs = fread(data, 1, datalen, f);
+ if (rs != datalen) {
+ if (ferror(f))
+ XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "failed to read %s", filename);
+ else if (feof(f))
+ XL_LOG(ctx, XL_LOG_ERROR, "%s changed size while we"
+ " were reading it", filename);
+ else
+ abort();
+ goto xe;
+ }
+ }
+
+ if (fclose(f)) {
+ f = 0;
+ XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "failed to close %s", filename);
+ goto xe;
+ }
+
+ if (data_r) *data_r = data;
+ if (datalen_r) *datalen_r = datalen;
+
+ return 0;
+
+ xe:
+ e = errno;
+ assert(e != ENOENT);
+ if (f) fclose(f);
+ if (data) free(data);
+ return e;
+}
+
+#define READ_WRITE_EXACTLY(rw, zero_is_eof, constdata) \
+ \
+ int libxl_##rw##_exactly(struct libxl_ctx *ctx, int fd, \
+ constdata void *data, ssize_t sz, \
+ const char *filename, const char *what) { \
+ ssize_t got; \
+ \
+ while (sz > 0) { \
+ got = rw(fd, data, sz); \
+ if (got == -1) { \
+ if (errno == EINTR) continue; \
+ XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "failed to " #rw " %s%s%s", \
+ what?what:"", what?" from ":"", filename); \
+ return errno; \
+ } \
+ if (got == 0) { \
+ XL_LOG(ctx, XL_LOG_ERROR, \
+ zero_is_eof \
+ ? "file/stream truncated reading %s%s%s" \
+ : "file/stream write returned 0! writing %s%s%s", \
+ what?what:"", what?" from ":"", filename); \
+ return EPROTO; \
+ } \
+ sz -= got; \
+ data = (char*)data + got; \
+ } \
+ return 0; \
+ }
+
+READ_WRITE_EXACTLY(read, 1, /* */)
+READ_WRITE_EXACTLY(write, 0, const)
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index 05fa4e3..65e7e31 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -26,5 +26,23 @@ int libxl_is_stubdom(struct libxl_ctx *ctx, uint32_t domid, uint32_t *target_dom
int libxl_create_logfile(struct libxl_ctx *ctx, char *name, char **full_name);
int libxl_string_to_phystype(struct libxl_ctx *ctx, char *s, libxl_disk_phystype *phystype);
+int libxl_read_file_contents(struct libxl_ctx *ctx, const char *filename,
+ void **data_r, int *datalen_r);
+ /* Reads the contents of the plain file filename into a mallocd
+ * buffer. Returns 0 or errno. Any errors other than ENOENT are logged.
+ * If the file is empty, *data_r and *datalen_r are set to 0.
+ * On error, *data_r and *datalen_r are undefined.
+ * data_r and/or datalen_r may be 0.
+ */
+
+int libxl_read_exactly(struct libxl_ctx *ctx, int fd, void *data, ssize_t sz,
+ const char *filename, const char *what);
+int libxl_write_exactly(struct libxl_ctx *ctx, int fd, const void *data,
+ ssize_t sz, const char *filename, const char *what);
+ /* Returns 0 or errno. If file is truncated on reading, returns
+ * EPROTO and you have no way to tell how much was read. Errors are
+ * logged using filename (which is only used for logging) and what
+ * (which may be 0). */
+
#endif
--
1.5.6.5
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 04/11] libxl: libxl_domain_restore: Put fd back to blocking mode
2010-03-25 19:04 [PATCH 00/11] Migration support for "xl" Ian Jackson
` (2 preceding siblings ...)
2010-03-25 19:04 ` [PATCH 03/11] libxl: New utility functions in for reading and writing files Ian Jackson
@ 2010-03-25 19:04 ` Ian Jackson
2010-03-25 19:04 ` [PATCH 05/11] libxl: Provide libxl_domain_rename Ian Jackson
` (7 subsequent siblings)
11 siblings, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2010-03-25 19:04 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Ian Jackson
libxl_domain_restore calls, indirectly, xc_domain_restore. The
latter, when doing a live migration, sets the fd from blocking mode
(which it must be on entry, or things go wrong) to nonblocking mode
and leaves it this way. Arguably this is a bug in libxc, but to avoid
disrupting any callers we fix it in libxl.
So libxl_domain_restore now puts the fd back into blocking mode
before returning.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
tools/libxl/libxl.c | 15 ++++++++++++++-
1 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 0b1b6db..308273d 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -221,7 +221,7 @@ int libxl_domain_restore(struct libxl_ctx *ctx, libxl_domain_build_info *info,
libxl_device_model_info *dm_info)
{
char **vments = NULL, **localents = NULL;
- int i, ret;
+ int i, ret, esave, flags;
ret = build_pre(ctx, domid, info, state);
if (ret) goto out;
@@ -259,6 +259,19 @@ int libxl_domain_restore(struct libxl_ctx *ctx, libxl_domain_build_info *info,
else
dm_info->saved_state = NULL;
out:
+ esave = errno;
+
+ flags = fcntl(fd, F_GETFL);
+ if (flags == -1) {
+ XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "unable to get flags on restore fd");
+ } else {
+ flags &= ~O_NONBLOCK;
+ if (fcntl(fd, F_SETFL, flags) == -1)
+ XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "unable to put restore fd"
+ " back to blocking mode");
+ }
+
+ errno = esave;
return ret;
}
--
1.5.6.5
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 05/11] libxl: Provide libxl_domain_rename
2010-03-25 19:04 [PATCH 00/11] Migration support for "xl" Ian Jackson
` (3 preceding siblings ...)
2010-03-25 19:04 ` [PATCH 04/11] libxl: libxl_domain_restore: Put fd back to blocking mode Ian Jackson
@ 2010-03-25 19:04 ` Ian Jackson
2010-03-26 10:01 ` Vincent Hanquez
2010-03-25 19:04 ` [PATCH 06/11] libxl: Expose functions for helping with subprocesses Ian Jackson
` (6 subsequent siblings)
11 siblings, 1 reply; 29+ messages in thread
From: Ian Jackson @ 2010-03-25 19:04 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Ian Jackson
Provide a new function libxl_domain_rename. It can check that the
domain being renamed has the expected name, to avoid races.
Use the new function to set the name during domain creation.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
tools/libxl/libxl.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++-
tools/libxl/libxl.h | 6 ++++
2 files changed, 87 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 308273d..59859ab 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -82,7 +82,7 @@ int libxl_ctx_set_log(struct libxl_ctx *ctx, libxl_log_callback log_callback, vo
int libxl_domain_make(struct libxl_ctx *ctx, libxl_domain_create_info *info,
uint32_t *domid)
{
- int flags, ret, i;
+ int flags, ret, i, rc;
char *uuid_string;
char *rw_paths[] = { "device", "device/suspend/event-channel" , "data"};
char *ro_paths[] = { "cpu", "memory", "device", "error", "drivers",
@@ -146,7 +146,8 @@ retry_transaction:
xs_write(ctx->xsh, t, libxl_sprintf(ctx, "%s/vm", dom_path), vm_path, strlen(vm_path));
xs_write(ctx->xsh, t, libxl_sprintf(ctx, "%s/vss", dom_path), vss_path, strlen(vss_path));
- xs_write(ctx->xsh, t, libxl_sprintf(ctx, "%s/name", dom_path), info->name, strlen(info->name));
+ rc = libxl_domain_rename(ctx, *domid, 0, info->name, t);
+ if (rc) return rc;
for (i = 0; i < ARRAY_SIZE(rw_paths); i++) {
char *path = libxl_sprintf(ctx, "%s/%s", dom_path, rw_paths[i]);
@@ -175,6 +176,84 @@ retry_transaction:
return 0;
}
+int libxl_domain_rename(struct libxl_ctx *ctx, uint32_t domid,
+ const char *old_name, const char *new_name,
+ xs_transaction_t trans) {
+ char *dom_path = 0;
+ const char *name_path;
+ char *got_old_name;
+ unsigned int got_old_len;
+ xs_transaction_t our_trans = 0;
+ int rc;
+
+ dom_path = libxl_xs_get_dompath(ctx, domid);
+ if (!dom_path) goto x_nomem;
+
+ name_path= libxl_sprintf(ctx, "%s/name", dom_path);
+ if (!name_path) goto x_nomem;
+
+ retry_transaction:
+ if (!trans) {
+ trans = our_trans = xs_transaction_start(ctx->xsh);
+ if (!our_trans) {
+ XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, errno,
+ "create xs transaction for domain (re)name");
+ goto x_fail;
+ }
+ }
+
+ if (old_name) {
+ got_old_name = xs_read(ctx->xsh, trans, name_path, &got_old_len);
+ if (!got_old_name) {
+ XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, errno, "check old name"
+ " for domain %"PRIu32" allegedly named `%s'",
+ domid, old_name);
+ goto x_fail;
+ }
+ if (strcmp(old_name, got_old_name)) {
+ XL_LOG(ctx, XL_LOG_ERROR, "domain %"PRIu32" allegedly named "
+ "`%s' is actually named `%s' - racing ?",
+ domid, old_name, got_old_name);
+ free(got_old_name);
+ goto x_fail;
+ }
+ free(got_old_name);
+ }
+ if (!xs_write(ctx->xsh, trans, name_path,
+ new_name, strlen(new_name))) {
+ XL_LOG(ctx, XL_LOG_ERROR, "failed to write new name `%s'"
+ " for domain %"PRIu32" previously named `%s'",
+ domid, new_name, old_name);
+ goto x_fail;
+ }
+
+ if (our_trans) {
+ if (!xs_transaction_end(ctx->xsh, our_trans, 0)) {
+ trans = our_trans = 0;
+ if (errno != EAGAIN) {
+ XL_LOG(ctx, XL_LOG_ERROR, "failed to commit new name `%s'"
+ " for domain %"PRIu32" previously named `%s'",
+ domid, new_name, old_name);
+ goto x_fail;
+ }
+ XL_LOG(ctx, XL_LOG_DEBUG, "need to retry rename transaction"
+ " for domain %"PRIu32" (name_path=\"%s\", new_name=\"%s\")",
+ domid, name_path, new_name);
+ goto retry_transaction;
+ }
+ our_trans = 0;
+ }
+
+ rc = 0;
+ x_rc:
+ if (dom_path) libxl_free(ctx, dom_path);
+ if (our_trans) xs_transaction_end(ctx->xsh, our_trans, 1);
+ return rc;
+
+ x_fail: rc = ERROR_FAIL; goto x_rc;
+ x_nomem: rc = ERROR_NOMEM; goto x_rc;
+}
+
int libxl_domain_build(struct libxl_ctx *ctx, libxl_domain_build_info *info, uint32_t domid, libxl_domain_build_state *state)
{
char **vments = NULL, **localents = NULL;
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 84e412a..2d76087 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -296,6 +296,12 @@ int libxl_free_waiter(libxl_waiter *waiter);
int libxl_event_get_domain_death_info(struct libxl_ctx *ctx, uint32_t domid, libxl_event *event, xc_domaininfo_t *info);
int libxl_event_get_disk_eject_info(struct libxl_ctx *ctx, uint32_t domid, libxl_event *event, libxl_device_disk *disk);
+int libxl_domain_rename(struct libxl_ctx *ctx, uint32_t domid,
+ const char *old_name, const char *new_name,
+ xs_transaction_t trans);
+ /* if old_name is NULL, any old name is OK; otherwise we check
+ * transactionally that the domain has the old old name; if
+ * trans is not 0 we use caller's transaction and caller must do retries */
int libxl_domain_pause(struct libxl_ctx *ctx, uint32_t domid);
int libxl_domain_unpause(struct libxl_ctx *ctx, uint32_t domid);
--
1.5.6.5
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 05/11] libxl: Provide libxl_domain_rename
2010-03-25 19:04 ` [PATCH 05/11] libxl: Provide libxl_domain_rename Ian Jackson
@ 2010-03-26 10:01 ` Vincent Hanquez
2010-03-26 10:23 ` Ian Jackson
0 siblings, 1 reply; 29+ messages in thread
From: Vincent Hanquez @ 2010-03-26 10:01 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel@lists.xensource.com
On 25/03/10 19:04, Ian Jackson wrote:
> Provide a new function libxl_domain_rename. It can check that the
> domain being renamed has the expected name, to avoid races.
>
> Use the new function to set the name during domain creation.
this looks really ugly (transaction in transaction in domain make), and
has a really complicated code path (which doesn't look right either on
the first read), but
my main objection is that libxl has no business handling the uniqueness
of names or whatever you want to do here. libxl get a name string from
whoever is using it (xl in your case), so if you want somehow to have a
special feature make it so in xl. in other words, no policies in libxl.
also NULL is for pointer, 0 for integer values.
--
Vincent
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 05/11] libxl: Provide libxl_domain_rename
2010-03-26 10:01 ` Vincent Hanquez
@ 2010-03-26 10:23 ` Ian Jackson
2010-03-26 10:34 ` Vincent Hanquez
0 siblings, 1 reply; 29+ messages in thread
From: Ian Jackson @ 2010-03-26 10:23 UTC (permalink / raw)
To: xen-devel@lists.xensource.com
Vincent Hanquez writes ("Re: [Xen-devel] [PATCH 05/11] libxl: Provide libxl_domain_rename"):
> On 25/03/10 19:04, Ian Jackson wrote:
> > Provide a new function libxl_domain_rename. It can check that the
> > domain being renamed has the expected name, to avoid races.
> >
> > Use the new function to set the name during domain creation.
>
> this looks really ugly (transaction in transaction in domain make), and
> has a really complicated code path (which doesn't look right either on
> the first read), but
There is no transaction in transaction. This arrangement is necessary
because the (re)name action which is part of domain creation is
already part of a transaction. I didn't much like the "goto"-based
retry loop but that's the way it's done in the domain creation too.
> my main objection is that libxl has no business handling the uniqueness
> of names or whatever you want to do here. libxl get a name string from
> whoever is using it (xl in your case), so if you want somehow to have a
> special feature make it so in xl. in other words, no policies in libxl.
You are confused. The purpose of this check is to check that the
existing name is as expected, not that the name is not unique
(although the lack of checking that the name is unique is also a bug).
The reason we need to check that the existing name is as expected is
to ensure that we effectively "lock" the domain against multiple
simultaneous migration attempts.
> also NULL is for pointer, 0 for integer values.
0 is a perfectly acceptable null pointer constant.
Ian.
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 05/11] libxl: Provide libxl_domain_rename
2010-03-26 10:23 ` Ian Jackson
@ 2010-03-26 10:34 ` Vincent Hanquez
2010-03-26 10:40 ` Ian Jackson
2010-04-01 11:00 ` Stefano Stabellini
0 siblings, 2 replies; 29+ messages in thread
From: Vincent Hanquez @ 2010-03-26 10:34 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel@lists.xensource.com
On 26/03/10 10:23, Ian Jackson wrote:
> You are confused. The purpose of this check is to check that the
> existing name is as expected, not that the name is not unique
> (although the lack of checking that the name is unique is also a bug).
I'm not confused, there's no need to check anything, libxl is here to
plumb whatever name the user of the library has chosen for its domain
into a xenstore entry: the name is opaque to libxl, and should remains
as such.
if you want some kind of migration lock, put in xl where you define the
policy.
> 0 is a perfectly acceptable null pointer constant.
working != acceptable.
--
Vincent
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 05/11] libxl: Provide libxl_domain_rename
2010-03-26 10:34 ` Vincent Hanquez
@ 2010-03-26 10:40 ` Ian Jackson
2010-04-01 11:00 ` Stefano Stabellini
1 sibling, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2010-03-26 10:40 UTC (permalink / raw)
To: xen-devel@lists.xensource.com
Vincent Hanquez writes ("Re: [Xen-devel] [PATCH 05/11] libxl: Provide libxl_domain_rename"):
> if you want some kind of migration lock, put in xl where you define the
> policy.
That is exactly what I have done. The lock is implemented using the
name in xenstore, which xl modifies in a transaction. However,
following the principle that libxl callers should not access xenstore
directly, this is mediated by the domain rename function.
libxl callers who do not care about the old name when setting the new
one can simply pass 0 for that argument to the rename function.
Ian.
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 05/11] libxl: Provide libxl_domain_rename
2010-03-26 10:34 ` Vincent Hanquez
2010-03-26 10:40 ` Ian Jackson
@ 2010-04-01 11:00 ` Stefano Stabellini
1 sibling, 0 replies; 29+ messages in thread
From: Stefano Stabellini @ 2010-04-01 11:00 UTC (permalink / raw)
To: Vincent Hanquez; +Cc: xen-devel@lists.xensource.com, Ian Jackson
On Fri, 26 Mar 2010, Vincent Hanquez wrote:
> On 26/03/10 10:23, Ian Jackson wrote:
> > You are confused. The purpose of this check is to check that the
> > existing name is as expected, not that the name is not unique
> > (although the lack of checking that the name is unique is also a bug).
>
> I'm not confused, there's no need to check anything, libxl is here to
> plumb whatever name the user of the library has chosen for its domain
> into a xenstore entry: the name is opaque to libxl, and should remains
> as such.
>
> if you want some kind of migration lock, put in xl where you define the
> policy.
>
the domain name is not opaque to libxenlight:
- some public libxenlight structs contain the domain name;
- libxenlight writes the domain name to xenstore;
- libxenlight offers functions to convert domids into domain names and
vice versa;
all this before any of Ian's patches.
I think this proves that libxenlight takes care of domain names already
and this patch just adds another function to the library to complete its
functionalities, without breaking any unwritten rule.
But if you prefer we can move this function to libxl_utils.c.
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 06/11] libxl: Expose functions for helping with subprocesses.
2010-03-25 19:04 [PATCH 00/11] Migration support for "xl" Ian Jackson
` (4 preceding siblings ...)
2010-03-25 19:04 ` [PATCH 05/11] libxl: Provide libxl_domain_rename Ian Jackson
@ 2010-03-25 19:04 ` Ian Jackson
2010-03-25 19:04 ` [PATCH 07/11] libxl: Expose libxl_report_exitstatus Ian Jackson
` (5 subsequent siblings)
11 siblings, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2010-03-25 19:04 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Ian Jackson
* Expose libxl_fork in libxl_utils.h
* Expose libxl_pipe in libxl_utils.h
* Make libxl_exec put SIGPIPE back (so that libxl callers may
have SIGPIPE ignored)
xl would like to use libxl_fork (which is like fork(2) except that it
logs errors) and also a similar function libxl_pipe. So put these in
libxl_utils.[ch] and use them in libxl.c as appropriate, to avoid
having to duplicate code between xl and libxl.
Also, make sure that subprocesses spawned by libxl have SIGPIPE set
back to SIG_DFL as they are entitled to expect. This means that a
libxl caller which sets SIGPIPE to SIG_IGN is no longer buggy. (This
is relevant for xl migration, because xl would like to be such a
caller.)
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
tools/libxl/libxl.c | 12 +++---------
tools/libxl/libxl_exec.c | 18 +++++-------------
tools/libxl/libxl_utils.c | 23 +++++++++++++++++++++++
tools/libxl/libxl_utils.h | 4 ++++
4 files changed, 35 insertions(+), 22 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 59859ab..711f0a0 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1238,15 +1238,9 @@ int libxl_device_disk_add(struct libxl_ctx *ctx, uint32_t domid, libxl_device_di
char buf[1024], *dev;
dev = get_blktap2_device(ctx, disk->physpath, device_disk_string_of_phystype(disk->phystype));
if (dev == NULL) {
- if (pipe(p) < 0) {
- XL_LOG(ctx, XL_LOG_ERROR, "Failed to create a pipe");
- return -1;
- }
- rc = fork();
- if (rc < 0) {
- XL_LOG(ctx, XL_LOG_ERROR, "Failed to fork a new process");
- return -1;
- } else if (!rc) { /* child */
+ rc= libxl_pipe(ctx, p); if (rc==-1) return -1;
+ rc= libxl_fork(ctx); if (rc==-1) return -1;
+ if (!rc) { /* child */
int null_r, null_w;
char *args[4];
args[0] = "tapdisk2";
diff --git a/tools/libxl/libxl_exec.c b/tools/libxl/libxl_exec.c
index e8cc74c..6d4a5c5 100644
--- a/tools/libxl/libxl_exec.c
+++ b/tools/libxl/libxl_exec.c
@@ -30,19 +30,6 @@
#include "libxl.h"
#include "libxl_internal.h"
-static pid_t libxl_fork(struct libxl_ctx *ctx)
-{
- pid_t pid;
-
- pid = fork();
- if (pid == -1) {
- XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "fork failed");
- return -1;
- }
-
- return pid;
-}
-
static int call_waitpid(pid_t (*waitpid_cb)(pid_t, int *, int), pid_t pid, int *status, int options)
{
return (waitpid_cb) ? waitpid_cb(pid, status, options) : waitpid(pid, status, options);
@@ -61,6 +48,11 @@ void libxl_exec(int stdinfd, int stdoutfd, int stderrfd, char *arg0, char **args
dup2(stderrfd, STDERR_FILENO);
for (i = 4; i < 256; i++)
close(i);
+
+ signal(SIGPIPE, SIG_DFL);
+ /* in case our caller set it to IGN. subprocesses are entitled
+ * to assume they got DFL. */
+
execv(arg0, args);
_exit(-1);
}
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 3a1f095..c6989ce 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -280,3 +280,26 @@ int libxl_read_file_contents(struct libxl_ctx *ctx, const char *filename,
READ_WRITE_EXACTLY(read, 1, /* */)
READ_WRITE_EXACTLY(write, 0, const)
+
+
+pid_t libxl_fork(struct libxl_ctx *ctx)
+{
+ pid_t pid;
+
+ pid = fork();
+ if (pid == -1) {
+ XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "fork failed");
+ return -1;
+ }
+
+ return pid;
+}
+
+int libxl_pipe(struct libxl_ctx *ctx, int pipes[2])
+{
+ if (pipe(pipes) < 0) {
+ XL_LOG(ctx, XL_LOG_ERROR, "Failed to create a pipe");
+ return -1;
+ }
+ return 0;
+}
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index 65e7e31..fe8b975 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -44,5 +44,9 @@ int libxl_write_exactly(struct libxl_ctx *ctx, int fd, const void *data,
* logged using filename (which is only used for logging) and what
* (which may be 0). */
+pid_t libxl_fork(struct libxl_ctx *ctx);
+int libxl_pipe(struct libxl_ctx *ctx, int pipes[2]);
+ /* Just like fork(2), pipe(2), but log errors. */
+
#endif
--
1.5.6.5
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 07/11] libxl: Expose libxl_report_exitstatus
2010-03-25 19:04 [PATCH 00/11] Migration support for "xl" Ian Jackson
` (5 preceding siblings ...)
2010-03-25 19:04 ` [PATCH 06/11] libxl: Expose functions for helping with subprocesses Ian Jackson
@ 2010-03-25 19:04 ` Ian Jackson
2010-03-25 19:04 ` [PATCH 08/11] libxl: Per-domain data storage for the convenience of the library user Ian Jackson
` (4 subsequent siblings)
11 siblings, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2010-03-25 19:04 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Ian Jackson
xl would like to use libxl_report_exitstatus, so expose it in
libxl_utils.h to avoid having to write it twice. Also, give it a
"level" argument to set the loglevel of the resulting message.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
tools/libxl/libxl_exec.c | 16 +++++++---------
tools/libxl/libxl_internal.h | 5 -----
tools/libxl/libxl_utils.h | 11 +++++++++++
3 files changed, 18 insertions(+), 14 deletions(-)
diff --git a/tools/libxl/libxl_exec.c b/tools/libxl/libxl_exec.c
index 6d4a5c5..52dda97 100644
--- a/tools/libxl/libxl_exec.c
+++ b/tools/libxl/libxl_exec.c
@@ -57,34 +57,32 @@ void libxl_exec(int stdinfd, int stdoutfd, int stderrfd, char *arg0, char **args
_exit(-1);
}
-void libxl_report_child_exitstatus(struct libxl_ctx *ctx,
+void libxl_report_child_exitstatus(struct libxl_ctx *ctx, int level,
const char *what, pid_t pid, int status)
{
- /* treats all exit statuses as errors; if that's not what you want,
- * check status yourself first */
if (WIFEXITED(status)) {
int st = WEXITSTATUS(status);
if (st)
- XL_LOG(ctx, XL_LOG_ERROR, "%s [%ld] exited"
+ XL_LOG(ctx, level, "%s [%ld] exited"
" with error status %d", what, (unsigned long)pid, st);
else
- XL_LOG(ctx, XL_LOG_ERROR, "%s [%ld] unexpectedly"
+ XL_LOG(ctx, level, "%s [%ld] unexpectedly"
" exited status zero", what, (unsigned long)pid);
} else if (WIFSIGNALED(status)) {
int sig = WTERMSIG(status);
const char *str = strsignal(sig);
const char *coredump = WCOREDUMP(status) ? " (core dumped)" : "";
if (str)
- XL_LOG(ctx, XL_LOG_ERROR, "%s [%ld] died due to"
+ XL_LOG(ctx, level, "%s [%ld] died due to"
" fatal signal %s%s", what, (unsigned long)pid,
str, coredump);
else
- XL_LOG(ctx, XL_LOG_ERROR, "%s [%ld] died due to unknown"
+ XL_LOG(ctx, level, "%s [%ld] died due to unknown"
" fatal signal number %d%s", what, (unsigned long)pid,
sig, coredump);
} else {
- XL_LOG(ctx, XL_LOG_ERROR, "%s [%ld] gave unknown"
+ XL_LOG(ctx, level, "%s [%ld] gave unknown"
" wait status 0x%x", what, (unsigned long)pid, status);
}
}
@@ -145,7 +143,7 @@ static void report_spawn_intermediate_status(struct libxl_ctx *ctx,
char *intermediate_what = libxl_sprintf(ctx,
"%s intermediate process (startup monitor)",
for_spawn->what);
- libxl_report_child_exitstatus(ctx, intermediate_what,
+ libxl_report_child_exitstatus(ctx, XL_LOG_ERROR, intermediate_what,
for_spawn->intermediate, status);
}
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 10e72e5..d1e355c 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -49,11 +49,6 @@
#endif
/* all of these macros preserve errno (saving and restoring) */
-#define XL_LOG_DEBUG 3
-#define XL_LOG_INFO 2
-#define XL_LOG_WARNING 1
-#define XL_LOG_ERROR 0
-
/* logging */
void xl_logv(struct libxl_ctx *ctx, int errnoval, int loglevel, const char *file, int line, const char *func, char *fmt, va_list al);
void xl_log(struct libxl_ctx *ctx, int errnoval, int loglevel, const char *file, int line, const char *func, char *fmt, ...);
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index fe8b975..4c04c46 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -48,5 +48,16 @@ pid_t libxl_fork(struct libxl_ctx *ctx);
int libxl_pipe(struct libxl_ctx *ctx, int pipes[2]);
/* Just like fork(2), pipe(2), but log errors. */
+void libxl_report_child_exitstatus(struct libxl_ctx *ctx, int level,
+ const char *what, pid_t pid, int status);
+ /* treats all exit statuses as errors; if that's not what you want,
+ * check status yourself first */
+
+/* log levels: */
+#define XL_LOG_DEBUG 3
+#define XL_LOG_INFO 2
+#define XL_LOG_WARNING 1
+#define XL_LOG_ERROR 0
+
#endif
--
1.5.6.5
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 08/11] libxl: Per-domain data storage for the convenience of the library user
2010-03-25 19:04 [PATCH 00/11] Migration support for "xl" Ian Jackson
` (6 preceding siblings ...)
2010-03-25 19:04 ` [PATCH 07/11] libxl: Expose libxl_report_exitstatus Ian Jackson
@ 2010-03-25 19:04 ` Ian Jackson
2010-03-26 10:06 ` Vincent Hanquez
2010-03-25 19:04 ` [PATCH 09/11] xl: New savefile format. Save domain config when saving a domain Ian Jackson
` (3 subsequent siblings)
11 siblings, 1 reply; 29+ messages in thread
From: Ian Jackson @ 2010-03-25 19:04 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Ian Jackson
We provide a mechanism whereby a user of the libxl library is able to
store some information alongside the domain. The information stored
is a block of bytes. Its lifetime is that of the domain - ie the
userdata is garbage collected alongside the domain if the domain is
destroyed. (This is why the feature needs to be in libxl and cannot
be implemented in the user itself or in libxlutil.)
If a libxl caller does not need to use this feature it can ignore it.
The data is tagged with the (self-declared) name of the libxl user, so
that different users cannot accidentally trip over each others'
userdata. The data is not interpreted at all by libxl.
To assist developers and people debugging, there is a registry of the
known userdata userids, and the corresponding data format as declared
by that libxl user, in libxl.h next to these declarations:
int libxl_userdata_store(struct libxl_ctx *ctx, uint32_t domid,
const char *userdata_userid,
const uint8_t *data, int datalen);
int libxl_userdata_retrieve(struct libxl_ctx *ctx, uint32_t domid,
const char *userdata_userid,
uint8_t **data_r, int *datalen_r);
The next patch will introduce the data for the userid "xl".
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
tools/libxl/libxl.c | 2 +
tools/libxl/libxl.h | 31 +++++++++++
tools/libxl/libxl_dom.c | 115 ++++++++++++++++++++++++++++++++++++++++++
tools/libxl/libxl_internal.h | 1 +
4 files changed, 149 insertions(+), 0 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 711f0a0..4d069cd 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -708,6 +708,8 @@ int libxl_domain_destroy(struct libxl_ctx *ctx, uint32_t domid, int force)
if (!xs_rm(ctx->xsh, XBT_NULL, xapi_path))
XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "xs_rm failed for %s", xapi_path);
+ libxl__userdata_destroyall(ctx, domid);
+
rc = xc_domain_destroy(ctx->xch, domid);
if (rc < 0) {
XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, rc, "xc_domain_destroy failed for %d", domid);
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 2d76087..d57dd10 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -359,6 +359,37 @@ int libxl_device_pci_init(libxl_device_pci *pcidev, unsigned int domain,
unsigned int bus, unsigned int dev,
unsigned int func, unsigned int vdevfn);
+/*
+ * Functions for allowing users of libxl to store private data
+ * relating to a domain. The data is an opaque sequence of bytes and
+ * is not interpreted or used by libxl.
+ *
+ * Data is indexed by the userdata userid, which is a short printable
+ * ASCII string. The following list is a registry of userdata userids
+ * (the registry may be updated by posting a patch to xen-devel):
+ *
+ * userid Data contents
+ * "xl" domain config file in xl format, Unix line endings
+ *
+ * libxl does not enforce the registration of userdata userids or the
+ * semantics of the data. For specifications of the data formats
+ * see the code or documentation for the libxl caller in question.
+ */
+int libxl_userdata_store(struct libxl_ctx *ctx, uint32_t domid,
+ const char *userdata_userid,
+ const uint8_t *data, int datalen);
+ /* If datalen==0, data is not used and the user data for
+ * that domain and userdata_userid is deleted. */
+int libxl_userdata_retrieve(struct libxl_ctx *ctx, uint32_t domid,
+ const char *userdata_userid,
+ uint8_t **data_r, int *datalen_r);
+ /* On successful return, *data_r is from malloc.
+ * If there is no data for that domain and userdata_userid,
+ * *data_r and *datalen_r will be set to 0.
+ * data_r and datalen_r may be 0.
+ * On error return, *data_r and *datalen_r are undefined.
+ */
+
typedef enum {
POWER_BUTTON,
SLEEP_BUTTON
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 7196aa8..237febb 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -16,6 +16,8 @@
#include "libxl_osdeps.h"
#include <stdio.h>
+#include <assert.h>
+#include <glob.h>
#include <inttypes.h>
#include <string.h>
#include <sys/time.h> /* for struct timeval */
@@ -349,3 +351,116 @@ int save_device_model(struct libxl_ctx *ctx, uint32_t domid, int fd)
unlink(filename);
return 0;
}
+
+static const char *userdata_path(struct libxl_ctx *ctx, uint32_t domid,
+ const char *userdata_userid,
+ const char *wh) {
+ char *path;
+ path = libxl_sprintf(ctx, "/var/lib/xen/"
+ "userdata-%s.%"PRIu32".%s",
+ wh, domid, userdata_userid);
+ if (!path)
+ XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "unable to allocate for"
+ " userdata path");
+ return path;
+}
+
+static int userdata_delete(struct libxl_ctx *ctx, const char *path) {
+ int r;
+ r = unlink(path);
+ if (r) {
+ XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "remove failed for %s", path);
+ return errno;
+ }
+ return 0;
+}
+
+void libxl__userdata_destroyall(struct libxl_ctx *ctx, uint32_t domid) {
+ const char *pattern;
+ glob_t gl;
+ int r, i;
+
+ pattern = userdata_path(ctx, domid, "*", "?");
+ if (!pattern) return;
+
+ gl.gl_pathc = 0;
+ gl.gl_pathv = 0;
+ gl.gl_offs = 0;
+ r = glob(pattern, GLOB_ERR|GLOB_NOSORT|GLOB_MARK, 0, &gl);
+ if (r) XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "glob failed for %s", pattern);
+
+ for (i=0; i<gl.gl_pathc; i++) {
+ userdata_delete(ctx, gl.gl_pathv[i]);
+ }
+ globfree(&gl);
+}
+
+int libxl_userdata_store(struct libxl_ctx *ctx, uint32_t domid,
+ const char *userdata_userid,
+ const uint8_t *data, int datalen) {
+ const char *filename;
+ const char *newfilename;
+ int e;
+ int fd = -1;
+ FILE *f = 0;
+ size_t rs;
+
+ filename = userdata_path(ctx, domid, userdata_userid, "d");
+ if (!filename) return ENOMEM;
+
+ if (!datalen)
+ return userdata_delete(ctx, filename);
+
+ newfilename = userdata_path(ctx, domid, userdata_userid, "n");
+ if (!newfilename) return ENOMEM;
+
+ fd= open(newfilename, O_RDWR|O_CREAT|O_TRUNC, 0600);
+ if (fd<0) goto xe;
+
+ f= fdopen(fd, "wb");
+ if (!f) goto xe;
+ fd = -1;
+
+ rs = fwrite(data, 1, datalen, f);
+ if (rs != datalen) { assert(ferror(f)); goto xe; }
+
+ if (fclose(f)) goto xe;
+ f = 0;
+
+ if (rename(newfilename,filename)) goto xe;
+
+ return 0;
+
+ xe:
+ e = errno;
+ if (f) fclose(f);
+ if (fd>=0) close(fd);
+
+ XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "cannot write %s for %s",
+ newfilename, filename);
+ return e;
+}
+
+int libxl_userdata_retrieve(struct libxl_ctx *ctx, uint32_t domid,
+ const char *userdata_userid,
+ uint8_t **data_r, int *datalen_r) {
+ const char *filename;
+ int e;
+ int datalen = 0;
+ void *data = 0;
+
+ filename = userdata_path(ctx, domid, userdata_userid, "d");
+ if (!filename) return ENOMEM;
+
+ e = libxl_read_file_contents(ctx, filename, data_r ? &data : 0, &datalen);
+
+ if (!e && !datalen) {
+ XL_LOG(ctx, XL_LOG_ERROR, "userdata file %s is empty", filename);
+ if (data_r) assert(!*data_r);
+ return EPROTO;
+ }
+
+ if (data_r) *data_r = data;
+ if (datalen_r) *datalen_r = datalen;
+ return 0;
+}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index d1e355c..b35cf0f 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -130,6 +130,7 @@ int restore_common(struct libxl_ctx *ctx, uint32_t domid,
libxl_domain_build_info *info, libxl_domain_build_state *state, int fd);
int core_suspend(struct libxl_ctx *ctx, uint32_t domid, int fd, int hvm, int live, int debug);
int save_device_model(struct libxl_ctx *ctx, uint32_t domid, int fd);
+void libxl__userdata_destroyall(struct libxl_ctx *ctx, uint32_t domid);
/* from xl_device */
char *device_disk_backend_type_of_phystype(libxl_disk_phystype phystype);
--
1.5.6.5
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 08/11] libxl: Per-domain data storage for the convenience of the library user
2010-03-25 19:04 ` [PATCH 08/11] libxl: Per-domain data storage for the convenience of the library user Ian Jackson
@ 2010-03-26 10:06 ` Vincent Hanquez
2010-03-26 10:20 ` Ian Jackson
0 siblings, 1 reply; 29+ messages in thread
From: Vincent Hanquez @ 2010-03-26 10:06 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel@lists.xensource.com
On 25/03/10 19:04, Ian Jackson wrote:
> We provide a mechanism whereby a user of the libxl library is able to
> store some information alongside the domain. The information stored
> is a block of bytes. Its lifetime is that of the domain - ie the
> userdata is garbage collected alongside the domain if the domain is
> destroyed. (This is why the feature needs to be in libxl and cannot
> be implemented in the user itself or in libxlutil.)
The domain is never garbage collected without having the toolstack knows
about it (thinks crash gathering, other resources cleanups, policy
choosing).
if the toolstack knows about it, it can do anything it want which
include removing some extra userdata that it wants to store (and that
doesn't have to have a special hardcoded path for it)
--
Vincent
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 08/11] libxl: Per-domain data storage for the convenience of the library user
2010-03-26 10:06 ` Vincent Hanquez
@ 2010-03-26 10:20 ` Ian Jackson
2010-03-26 10:29 ` Vincent Hanquez
2010-04-01 11:04 ` Stefano Stabellini
0 siblings, 2 replies; 29+ messages in thread
From: Ian Jackson @ 2010-03-26 10:20 UTC (permalink / raw)
To: xen-devel@lists.xensource.com
Vincent Hanquez writes ("Re: [Xen-devel] [PATCH 08/11] libxl: Per-domain data storage for the convenience of the library user"):
> The domain is never garbage collected without having the toolstack knows
> about it [...]
This is not true unless the libxl caller (which you are calling "the
toolstack") has a never-dying daemon (or several), but it might not.
Ian.
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 08/11] libxl: Per-domain data storage for the convenience of the library user
2010-03-26 10:20 ` Ian Jackson
@ 2010-03-26 10:29 ` Vincent Hanquez
2010-03-26 10:34 ` Ian Jackson
2010-04-01 11:04 ` Stefano Stabellini
1 sibling, 1 reply; 29+ messages in thread
From: Vincent Hanquez @ 2010-03-26 10:29 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel@lists.xensource.com
On 26/03/10 10:20, Ian Jackson wrote:
> Vincent Hanquez writes ("Re: [Xen-devel] [PATCH 08/11] libxl: Per-domain data storage for the convenience of the library user"):
>> The domain is never garbage collected without having the toolstack knows
>> about it [...]
>
> This is not true unless the libxl caller (which you are calling "the
> toolstack") has a never-dying daemon (or several), but it might not.
All existing and working toolstacks are daemon (xend, XCP, XCI). There's
just no ways to do without when you consider real world cases.
--
Vincent
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 08/11] libxl: Per-domain data storage for the convenience of the library user
2010-03-26 10:29 ` Vincent Hanquez
@ 2010-03-26 10:34 ` Ian Jackson
2010-03-26 10:41 ` Vincent Hanquez
0 siblings, 1 reply; 29+ messages in thread
From: Ian Jackson @ 2010-03-26 10:34 UTC (permalink / raw)
To: Vincent Hanquez; +Cc: xen-devel@lists.xensource.com
Vincent Hanquez writes ("Re: [Xen-devel] [PATCH 08/11] libxl: Per-domain data storage for the convenience of the library user"):
> All existing and working toolstacks are daemon (xend, XCP, XCI). There's
> just no ways to do without when you consider real world cases.
xl is a caller of libxl and is intended as the replacement for
xm/xend. You may have noticed that this was part of the point of
libxl ?
Ian.
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 08/11] libxl: Per-domain data storage for the convenience of the library user
2010-03-26 10:34 ` Ian Jackson
@ 2010-03-26 10:41 ` Vincent Hanquez
2010-03-26 10:45 ` Ian Jackson
0 siblings, 1 reply; 29+ messages in thread
From: Vincent Hanquez @ 2010-03-26 10:41 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel@lists.xensource.com
On 26/03/10 10:34, Ian Jackson wrote:
> xl is a caller of libxl and is intended as the replacement for
> xm/xend. You may have noticed that this was part of the point of
> libxl ?
Do whatever suit you in xl, not in libxl.
libxl is a generic low level operation layer for toolstacks to use, the
change your are proposing are making it harder|impossible to have one
day XCP running on top of libxl.
XCP doesn't need a "locking" mechanism that use the name in xenstore for
migration.
--
Vincent
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 08/11] libxl: Per-domain data storage for the convenience of the library user
2010-03-26 10:41 ` Vincent Hanquez
@ 2010-03-26 10:45 ` Ian Jackson
2010-03-26 11:27 ` Vincent Hanquez
0 siblings, 1 reply; 29+ messages in thread
From: Ian Jackson @ 2010-03-26 10:45 UTC (permalink / raw)
To: xen-devel@lists.xensource.com
Vincent Hanquez writes ("Re: [Xen-devel] [PATCH 08/11] libxl: Per-domain data storage for the convenience of the library user"):
> XCP doesn't need a "locking" mechanism that use the name in xenstore for
> migration.
This is an unreasonable response, and I think it goes to the heart of
the problem here.
xl is a first-class caller of libxl, not some second-rate unimportant
thing. Every requirement of xl on libxl is a real requirement.
The real problem seems to me that you are only willing to agree to
features and API properties which are useful for your "toolstacks",
and not ones which are required for xl. This is not acceptable.
Ian.
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 08/11] libxl: Per-domain data storage for the convenience of the library user
2010-03-26 10:45 ` Ian Jackson
@ 2010-03-26 11:27 ` Vincent Hanquez
2010-03-26 11:56 ` Ian Jackson
0 siblings, 1 reply; 29+ messages in thread
From: Vincent Hanquez @ 2010-03-26 11:27 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel@lists.xensource.com
On 26/03/10 10:45, Ian Jackson wrote:
> Vincent Hanquez writes ("Re: [Xen-devel] [PATCH 08/11] libxl: Per-domain data storage for the convenience of the library user"):
>> XCP doesn't need a "locking" mechanism that use the name in xenstore for
>> migration.
>
> This is an unreasonable response, and I think it goes to the heart of
> the problem here.
>
> xl is a first-class caller of libxl, not some second-rate unimportant
> thing. Every requirement of xl on libxl is a real requirement.
>
> The real problem seems to me that you are only willing to agree to
> features and API properties which are useful for your "toolstacks",
> and not ones which are required for xl. This is not acceptable.
libxl has been made to "make common the low level operations of actuals
toolstacks", so that every toolstacks will share the same code base to
avoid code duplication and also give user more flexibility to move
between toolstacks.
Any others goals you have in mind, is a part of your own goals, not
libxl's. Whilst i'm happy to see more assumptions removed to have even
more generic code, adding assumptions on the other-hand, related to name
in this case, is a non-starter.
--
Vincent
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 08/11] libxl: Per-domain data storage for the convenience of the library user
2010-03-26 11:27 ` Vincent Hanquez
@ 2010-03-26 11:56 ` Ian Jackson
0 siblings, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2010-03-26 11:56 UTC (permalink / raw)
To: xen-devel@lists.xensource.com
Vincent Hanquez writes ("Re: [Xen-devel] [PATCH 08/11] libxl: Per-domain data storage for the convenience of the library user"):
> Any others goals you have in mind, is a part of your own goals, not
> libxl's. Whilst i'm happy to see more assumptions removed to have even
> more generic code, adding assumptions on the other-hand, related to name
> in this case, is a non-starter.
There is no assumption added. Callers who do not want the ability to
atomically rename a domain (which is what libxl_domain_rename
provides) can simply ignore the ability to do so.
Ian.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 08/11] libxl: Per-domain data storage for the convenience of the library user
2010-03-26 10:20 ` Ian Jackson
2010-03-26 10:29 ` Vincent Hanquez
@ 2010-04-01 11:04 ` Stefano Stabellini
1 sibling, 0 replies; 29+ messages in thread
From: Stefano Stabellini @ 2010-04-01 11:04 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel@lists.xensource.com
On Fri, 26 Mar 2010, Ian Jackson wrote:
> Vincent Hanquez writes ("Re: [Xen-devel] [PATCH 08/11] libxl: Per-domain data storage for the convenience of the library user"):
> > The domain is never garbage collected without having the toolstack knows
> > about it [...]
>
> This is not true unless the libxl caller (which you are calling "the
> toolstack") has a never-dying daemon (or several), but it might not.
>
Actually I think Vincent is right in this case: there is no point in
adding this feature to libxenlight because xl is perfectly capable
of handling userdata by itself; for example xl could store the userdata
when the domain is created, leave it there when the domain dies and
remove\rename it when another domain with the same identifier is
created.
For this reason I think you should move these two functions to libxlutil.
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 09/11] xl: New savefile format. Save domain config when saving a domain.
2010-03-25 19:04 [PATCH 00/11] Migration support for "xl" Ian Jackson
` (7 preceding siblings ...)
2010-03-25 19:04 ` [PATCH 08/11] libxl: Per-domain data storage for the convenience of the library user Ian Jackson
@ 2010-03-25 19:04 ` Ian Jackson
2010-04-01 11:08 ` Stefano Stabellini
2010-03-25 19:04 ` [PATCH 10/11] xl: Domain creation logging fixes Ian Jackson
` (2 subsequent siblings)
11 siblings, 1 reply; 29+ messages in thread
From: Ian Jackson @ 2010-03-25 19:04 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Ian Jackson
We introduce a new format for saved domains. The new format, in
contrast to the old:
* Has a magic number which can distinguish it from other kinds of file
* Is extensible
* Can contains the domain configuration file
On domain creation we remember the actual config file used (using the
toolstack data feature of libxl, just introduced), and by default save
it to the save file.
However, options are provided for the following:
* Reading and writing old-style xl saved domain files. (These will
become deprecated at some point in the future.)
* When saving a domain, supplying an alternative config file to
store in the savefile.
* When restoring a domain, supplying an alternative config file.
If a domain is restored with a different config file, it is the
responsibility of the xl user to ensure that the two configs are
"compatible". Changing the targets of virtual devices is supported;
changing other features of the domain is not recommended. Bad changes
may lead to undefined behaviour in the domain, and are in practice
likely to cause resume failures or crashes.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
tools/libxl/libxlu_cfg.c | 37 ++++++
tools/libxl/libxlutil.h | 3 +-
tools/libxl/xl.c | 280 +++++++++++++++++++++++++++++++++++++++++-----
3 files changed, 289 insertions(+), 31 deletions(-)
diff --git a/tools/libxl/libxlu_cfg.c b/tools/libxl/libxlu_cfg.c
index 632c371..0a5ccef 100644
--- a/tools/libxl/libxlu_cfg.c
+++ b/tools/libxl/libxlu_cfg.c
@@ -53,6 +53,43 @@ int xlu_cfg_readfile(XLU_Config *cfg, const char *real_filename) {
return ctx.err;
}
+int xlu_cfg_readdata(XLU_Config *cfg, const char *data, int length) {
+ CfgParseContext ctx;
+ int e, r;
+ YY_BUFFER_STATE buf= 0;
+
+ ctx.scanner= 0;
+ ctx.cfg= cfg;
+ ctx.err= 0;
+ ctx.lexerrlineno= -1;
+
+ e= xlu__cfg_yylex_init_extra(&ctx, &ctx.scanner);
+ if (e) {
+ fprintf(cfg->report,"%s: unable to create scanner: %s\n",
+ cfg->filename, strerror(e));
+ ctx.err= e;
+ ctx.scanner= 0;
+ goto xe;
+ }
+
+ buf = xlu__cfg_yy_scan_bytes(data, length, ctx.scanner);
+ if (!buf) {
+ fprintf(cfg->report,"%s: unable to allocate scanner buffer\n",
+ cfg->filename);
+ ctx.err= ENOMEM;
+ goto xe;
+ }
+
+ r= xlu__cfg_yyparse(&ctx);
+ if (r) assert(ctx.err);
+
+ xe:
+ if (buf) xlu__cfg_yy_delete_buffer(buf, ctx.scanner);
+ if (ctx.scanner) xlu__cfg_yylex_destroy(ctx.scanner);
+
+ return ctx.err;
+}
+
void xlu__cfg_set_free(XLU_ConfigSetting *set) {
free(set->name);
free(set->values);
diff --git a/tools/libxl/libxlutil.h b/tools/libxl/libxlutil.h
index 97b16aa..0262e55 100644
--- a/tools/libxl/libxlutil.h
+++ b/tools/libxl/libxlutil.h
@@ -30,7 +30,8 @@ XLU_Config *xlu_cfg_init(FILE *report, const char *report_filename);
* until the Config is destroyed. */
int xlu_cfg_readfile(XLU_Config*, const char *real_filename);
- /* If this fails, then it is undefined behaviour to call xlu_cfg_get_...
+int xlu_cfg_readdata(XLU_Config*, const char *data, int length);
+ /* If these fail, then it is undefined behaviour to call xlu_cfg_get_...
* functions. You have to just xlu_cfg_destroy. */
void xlu_cfg_destroy(XLU_Config*);
diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
index 2e23da3..f3729f3 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -30,6 +30,7 @@
#include <arpa/inet.h>
#include <xenctrl.h>
#include <ctype.h>
+#include <inttypes.h>
#include "libxl.h"
#include "libxl_utils.h"
@@ -39,6 +40,27 @@
int logfile = 2;
+static int suspend_old_xl_format = 0;
+
+static const char savefileheader_magic[32]=
+ "Xen saved domain, xl format\n \0 \r";
+
+typedef struct {
+ char magic[32]; /* savefileheader_magic */
+ /* All uint32_ts are in domain's byte order. */
+ uint32_t byteorder; /* SAVEFILE_BYTEORDER_VALUE */
+ uint32_t mandatory_flags; /* unknown flags => reject restore */
+ uint32_t optional_flags; /* unknown flags => reject restore */
+ uint32_t optional_data_len; /* skip, or skip tail, if not understood */
+} SaveFileHeader;
+
+/* Optional data, in order:
+ * 4 bytes uint32_t config file size
+ * n bytes config file in Unix text file format
+ */
+
+#define SAVEFILE_BYTEORDER_VALUE ((uint32_t)0x01020304UL)
+
void log_callback(void *userdata, int loglevel, const char *file, int line, const char *func, char *s)
{
char str[1024];
@@ -320,7 +342,9 @@ static void printf_info(libxl_domain_create_info *c_info,
}
}
-static void parse_config_file(const char *filename,
+static void parse_config_data(const char *configfile_filename_report,
+ const char *configfile_data,
+ int configfile_len,
libxl_domain_create_info *c_info,
libxl_domain_build_info *b_info,
libxl_device_disk **disks,
@@ -343,13 +367,13 @@ static void parse_config_file(const char *filename,
int pci_msitranslate = 1;
int i, e;
- config= xlu_cfg_init(stderr, filename);
+ config= xlu_cfg_init(stderr, configfile_filename_report);
if (!config) {
fprintf(stderr, "Failed to allocate for configuration\n");
exit(1);
}
- e= xlu_cfg_readfile (config, filename);
+ e= xlu_cfg_readdata(config, configfile_data, configfile_len);
if (e) {
fprintf(stderr, "Failed to parse config file: %s\n", strerror(e));
exit(1);
@@ -664,6 +688,15 @@ skip_pci:
xlu_cfg_destroy(config);
}
+#define CHK_ERRNO( call ) ({ \
+ int chk_errno = (call); \
+ if (chk_errno) { \
+ fprintf(stderr,"xl: fatal error: %s:%d: %s: %s\n", \
+ __FILE__,__LINE__, strerror(chk_errno), #call); \
+ exit(-ERROR_FAIL); \
+ } \
+ })
+
#define MUST( call ) ({ \
int must_rc = (call); \
if (must_rc) { \
@@ -673,6 +706,26 @@ skip_pci:
} \
})
+static void *xmalloc(size_t sz) {
+ void *r;
+ r = malloc(sz);
+ if (!r) { fprintf(stderr,"xl: Unable to malloc %lu bytes.\n",
+ (unsigned long)sz); exit(-ERROR_FAIL); }
+ return r;
+}
+
+static void *xrealloc(void *ptr, size_t sz) {
+ void *r;
+ if (!sz) { free(ptr); return 0; }
+ /* realloc(non-0, 0) has a useless return value;
+ * but xrealloc(anything, 0) is like free
+ */
+ r = realloc(ptr, sz);
+ if (!r) { fprintf(stderr,"xl: Unable to realloc to %lu bytes.\n",
+ (unsigned long)sz); exit(-ERROR_FAIL); }
+ return r;
+}
+
static void create_domain(int debug, int daemonize, const char *config_file, const char *restore_file, int paused)
{
struct libxl_ctx ctx;
@@ -693,27 +746,124 @@ static void create_domain(int debug, int daemonize, const char *config_file, con
int ret;
libxl_device_model_starting *dm_starting = 0;
libxl_waiter *w1 = NULL, *w2 = NULL;
+ void *config_data = 0;
+ int config_len = 0;
+ int restore_fd = -1;
+ SaveFileHeader hdr;
+
memset(&dm_info, 0x00, sizeof(dm_info));
+ if (libxl_ctx_init(&ctx, LIBXL_VERSION)) {
+ fprintf(stderr, "cannot init xl context\n");
+ exit(1);
+ }
+
+ if (restore_file) {
+ uint8_t *optdata_begin = 0;
+ const uint8_t *optdata_here = 0;
+ union { uint32_t u32; char b[4]; } u32buf;
+
+ restore_fd = open(restore_file, O_RDONLY);
+
+ if (suspend_old_xl_format) {
+ memset(&hdr,0,sizeof(hdr));
+ } else {
+ uint32_t badflags;
+
+ CHK_ERRNO( libxl_read_exactly(&ctx, restore_fd, &hdr,
+ sizeof(hdr), restore_file, "header") );
+ if (memcmp(hdr.magic, savefileheader_magic, sizeof(hdr.magic))) {
+ fprintf(stderr, "File has wrong magic number -"
+ " corrupt or needs -O?\n");
+ exit(2);
+ }
+ if (hdr.byteorder != SAVEFILE_BYTEORDER_VALUE) {
+ fprintf(stderr, "File has wrong byte order\n");
+ exit(2);
+ }
+ fprintf(stderr, "Loading new save file %s"
+ " (new xl fmt info"
+ " 0x%"PRIx32"/0x%"PRIx32"/%"PRIu32")\n",
+ restore_file, hdr.mandatory_flags, hdr.optional_flags,
+ hdr.optional_data_len);
+
+ badflags = hdr.mandatory_flags & ~( 0 /* none understood yet */ );
+ if (badflags) {
+ fprintf(stderr, "Savefile has mandatory flag(s) 0x%"PRIx32" "
+ "which are not supported; need newer xl\n",
+ badflags);
+ exit(2);
+ }
+ }
+ if (hdr.optional_data_len) {
+ optdata_begin = xmalloc(hdr.optional_data_len);
+ CHK_ERRNO( libxl_read_exactly(&ctx, restore_fd, optdata_begin,
+ hdr.optional_data_len, restore_file, "optdata") );
+ }
+
+#define OPTDATA_LEFT (hdr.optional_data_len - (optdata_here - optdata_begin))
+#define WITH_OPTDATA(amt, body) \
+ if (OPTDATA_LEFT < (amt)) { \
+ fprintf(stderr, "Savefile truncated.\n"); exit(2); \
+ } else { \
+ body; \
+ optdata_here += (amt); \
+ }
+
+ optdata_here = optdata_begin;
+
+ if (OPTDATA_LEFT) {
+ fprintf(stderr, " Savefile contains xl domain config\n");
+ WITH_OPTDATA(4, {
+ memcpy(u32buf.b, optdata_here, 4);
+ config_len = u32buf.u32;
+ });
+ WITH_OPTDATA(config_len, {
+ config_data = xmalloc(config_len);
+ memcpy(config_data, optdata_here, config_len);
+ });
+ }
+
+ }
+
+ if (config_file) {
+ free(config_data); config_data = 0;
+ ret = libxl_read_file_contents(&ctx, config_file,
+ &config_data, &config_len);
+ if (ret) { fprintf(stderr, "Failed to read config file: %s: %s\n",
+ config_file, strerror(errno)); exit(1); }
+ } else {
+ if (!config_data) {
+ fprintf(stderr, "Config file not specified and"
+ " none in save file\n");
+ exit(1);
+ }
+ config_file = "<saved>";
+ }
+
printf("Parsing config file %s\n", config_file);
- parse_config_file(config_file, &info1, &info2, &disks, &num_disks, &vifs, &num_vifs, &pcidevs, &num_pcidevs, &vfbs, &num_vfbs, &vkbs, &num_vkbs, &dm_info);
+
+ parse_config_data(config_file, config_data, config_len, &info1, &info2, &disks, &num_disks, &vifs, &num_vifs, &pcidevs, &num_pcidevs, &vfbs, &num_vfbs, &vkbs, &num_vkbs, &dm_info);
+
if (debug)
printf_info(&info1, &info2, disks, num_disks, vifs, num_vifs, pcidevs, num_pcidevs, vfbs, num_vfbs, vkbs, num_vkbs, &dm_info);
start:
domid = 0;
- if (libxl_ctx_init(&ctx, LIBXL_VERSION)) {
- fprintf(stderr, "cannot init xl context\n");
- return;
- }
-
libxl_ctx_set_log(&ctx, log_callback, NULL);
ret = libxl_domain_make(&ctx, &info1, &domid);
if (ret) {
fprintf(stderr, "cannot make domain: %d\n", ret);
- return;
+ exit(1);
+ }
+
+ ret = libxl_userdata_store(&ctx, domid, "xl",
+ config_data, config_len);
+ if (ret) {
+ perror("cannot save config file");
+ exit(1);
}
if (!restore_file || !need_daemon) {
@@ -723,16 +873,13 @@ start:
}
ret = libxl_domain_build(&ctx, &info2, domid, &state);
} else {
- int restore_fd;
-
- restore_fd = open(restore_file, O_RDONLY);
ret = libxl_domain_restore(&ctx, &info2, domid, restore_fd, &state, &dm_info);
close(restore_fd);
}
if (ret) {
fprintf(stderr, "cannot (re-)build domain: %d\n", ret);
- return;
+ exit(1);
}
for (i = 0; i < num_disks; i++) {
@@ -740,7 +887,7 @@ start:
ret = libxl_device_disk_add(&ctx, domid, &disks[i]);
if (ret) {
fprintf(stderr, "cannot add disk %d to domain: %d\n", i, ret);
- return;
+ exit(1);
}
}
for (i = 0; i < num_vifs; i++) {
@@ -748,7 +895,7 @@ start:
ret = libxl_device_nic_add(&ctx, domid, &vifs[i]);
if (ret) {
fprintf(stderr, "cannot add nic %d to domain: %d\n", i, ret);
- return;
+ exit(1);
}
}
if (info1.hvm) {
@@ -795,8 +942,8 @@ start:
need_daemon = 0;
}
LOG("Waiting for domain %s (domid %d) to die", info1.name, domid);
- w1 = (libxl_waiter*) malloc(sizeof(libxl_waiter) * num_disks);
- w2 = (libxl_waiter*) malloc(sizeof(libxl_waiter));
+ w1 = (libxl_waiter*) xmalloc(sizeof(libxl_waiter) * num_disks);
+ w2 = (libxl_waiter*) xmalloc(sizeof(libxl_waiter));
libxl_wait_for_disk_ejects(&ctx, domid, disks, num_disks, w1);
libxl_wait_for_domain_death(&ctx, domid, w2);
libxl_get_wait_fd(&ctx, &fd);
@@ -852,6 +999,7 @@ start:
free(vfbs);
free(vkbs);
free(pcidevs);
+ free(config_data);
}
static void help(char *command)
@@ -900,16 +1048,18 @@ static void help(char *command)
printf("Usage: xl unpause <Domain>\n\n");
printf("Unpause a paused domain.\n\n");
} else if(!strcmp(command, "save")) {
- printf("Usage: xl save [options] <Domain> <CheckpointFile>\n\n");
+ printf("Usage: xl save [options] <Domain> <CheckpointFile> [<ConfigFile>]\n\n");
printf("Save a domain state to restore later.\n\n");
printf("Options:\n\n");
printf("-h Print this help.\n");
+ printf("-O Old (configless) xl save format.\n");
printf("-c Leave domain running after creating the snapshot.\n");
} else if(!strcmp(command, "restore")) {
- printf("Usage: xl restore [options] <ConfigFile> <CheckpointFile>\n\n");
+ printf("Usage: xl restore [options] [<ConfigFile>] <CheckpointFile>\n\n");
printf("Restore a domain from a saved state.\n\n");
printf("Options:\n\n");
printf("-h Print this help.\n");
+ printf("-O Old (configless) xl save format.\n");
printf("-p Do not unpause domain after restoring it.\n");
printf("-e Do not wait in the background for the death of the domain.\n");
} else if(!strcmp(command, "destroy")) {
@@ -1413,11 +1563,12 @@ void list_vm(void)
free(info);
}
-int save_domain(char *p, char *filename, int checkpoint)
+int save_domain(char *p, char *filename, int checkpoint,
+ const char *override_config_file)
{
struct libxl_ctx ctx;
uint32_t domid;
- int fd;
+ int fd, rc;
if (libxl_ctx_init(&ctx, LIBXL_VERSION)) {
fprintf(stderr, "cannot init xl context\n");
@@ -1434,6 +1585,65 @@ int save_domain(char *p, char *filename, int checkpoint)
fprintf(stderr, "Failed to open temp file %s for writing\n", filename);
exit(2);
}
+
+ if (!suspend_old_xl_format) {
+ SaveFileHeader hdr;
+ uint8_t *config_data = 0;
+ int config_len;
+ uint8_t *optdata_begin;
+ union { uint32_t u32; char b[4]; } u32buf;
+
+ memset(&hdr, 0, sizeof(hdr));
+ memcpy(hdr.magic, savefileheader_magic, sizeof(hdr.magic));
+ hdr.byteorder = SAVEFILE_BYTEORDER_VALUE;
+
+ optdata_begin= 0;
+
+#define ADD_OPTDATA(ptr, len) ({ \
+ if ((len)) { \
+ hdr.optional_data_len += (len); \
+ optdata_begin = xrealloc(optdata_begin, hdr.optional_data_len); \
+ memcpy(optdata_begin + hdr.optional_data_len - (len), \
+ (ptr), (len)); \
+ } \
+ })
+
+ /* configuration file in optional data: */
+
+ if (override_config_file) {
+ void *config_v = 0;
+ rc = libxl_read_file_contents(&ctx, override_config_file,
+ &config_v, &config_len);
+ config_data = config_v;
+ } else {
+ rc = libxl_userdata_retrieve(&ctx, domid, "xl",
+ &config_data, &config_len);
+ }
+ if (rc) {
+ fputs("Unable to get config file\n",stderr);
+ exit(2);
+ }
+ if (!config_len) {
+ fputs(" Savefile will not contain xl domain config\n", stderr);
+ }
+
+ u32buf.u32 = config_len;
+ ADD_OPTDATA(u32buf.b, 4);
+ ADD_OPTDATA(config_data, config_len);
+
+ /* that's the optional data */
+
+ CHK_ERRNO( libxl_write_exactly(&ctx, fd,
+ &hdr, sizeof(hdr), filename, "header") );
+ CHK_ERRNO( libxl_write_exactly(&ctx, fd,
+ optdata_begin, hdr.optional_data_len, filename, "header") );
+
+ fprintf(stderr, "Saving to %s new xl format (info"
+ " 0x%"PRIx32"/0x%"PRIx32"/%"PRIu32")\n",
+ filename, hdr.mandatory_flags, hdr.optional_flags,
+ hdr.optional_data_len);
+ }
+
libxl_domain_suspend(&ctx, NULL, domid, fd);
close(fd);
@@ -1452,7 +1662,7 @@ int main_restore(int argc, char **argv)
int paused = 0, debug = 0, daemonize = 1;
int opt;
- while ((opt = getopt(argc, argv, "hpde")) != -1) {
+ while ((opt = getopt(argc, argv, "hpdeO")) != -1) {
switch (opt) {
case 'p':
paused = 1;
@@ -1466,19 +1676,24 @@ int main_restore(int argc, char **argv)
case 'h':
help("restore");
exit(0);
+ case 'O':
+ suspend_old_xl_format = 1;
+ break;
default:
fprintf(stderr, "option not supported\n");
break;
}
}
- if (optind >= argc - 1) {
+ if (argc-optind == 1) {
+ checkpoint_file = argv[optind];
+ } else if (argc-optind == 2) {
+ config_file = argv[optind];
+ checkpoint_file = argv[optind + 1];
+ } else {
help("restore");
exit(2);
}
-
- config_file = argv[optind];
- checkpoint_file = argv[optind + 1];
create_domain(debug, daemonize, config_file, checkpoint_file, paused);
exit(0);
}
@@ -1486,10 +1701,11 @@ int main_restore(int argc, char **argv)
int main_save(int argc, char **argv)
{
char *filename = NULL, *p = NULL;
+ const char *config_filename;
int checkpoint = 0;
int opt;
- while ((opt = getopt(argc, argv, "hc")) != -1) {
+ while ((opt = getopt(argc, argv, "hcO")) != -1) {
switch (opt) {
case 'c':
checkpoint = 1;
@@ -1497,20 +1713,24 @@ int main_save(int argc, char **argv)
case 'h':
help("save");
exit(0);
+ case 'O':
+ suspend_old_xl_format = 1;
+ break;
default:
fprintf(stderr, "option not supported\n");
break;
}
}
- if (optind >= argc - 1) {
+ if (argc-optind < 1 || argc-optind > 3) {
help("save");
exit(2);
}
p = argv[optind];
filename = argv[optind + 1];
- save_domain(p, filename, checkpoint);
+ config_filename = argv[optind + 2];
+ save_domain(p, filename, checkpoint, config_filename);
exit(0);
}
--
1.5.6.5
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 09/11] xl: New savefile format. Save domain config when saving a domain.
2010-03-25 19:04 ` [PATCH 09/11] xl: New savefile format. Save domain config when saving a domain Ian Jackson
@ 2010-04-01 11:08 ` Stefano Stabellini
0 siblings, 0 replies; 29+ messages in thread
From: Stefano Stabellini @ 2010-04-01 11:08 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel@lists.xensource.com
On Thu, 25 Mar 2010, Ian Jackson wrote:
> We introduce a new format for saved domains. The new format, in
> contrast to the old:
> * Has a magic number which can distinguish it from other kinds of file
> * Is extensible
> * Can contains the domain configuration file
>
> On domain creation we remember the actual config file used (using the
> toolstack data feature of libxl, just introduced), and by default save
> it to the save file.
>
> However, options are provided for the following:
> * Reading and writing old-style xl saved domain files. (These will
> become deprecated at some point in the future.)
> * When saving a domain, supplying an alternative config file to
> store in the savefile.
> * When restoring a domain, supplying an alternative config file.
>
> If a domain is restored with a different config file, it is the
> responsibility of the xl user to ensure that the two configs are
> "compatible". Changing the targets of virtual devices is supported;
> changing other features of the domain is not recommended. Bad changes
> may lead to undefined behaviour in the domain, and are in practice
> likely to cause resume failures or crashes.
>
I think that providing compatibility with the old-style xl saved domain
files is overkill and just adds complexity for no good reason.
We should stick to the new format and forget about the old one
altogether.
For the same reason I would remove the option to save\restore from an
alternative config file.
Please keep it simple.
> +
> +static const char savefileheader_magic[32]=
> + "Xen saved domain, xl format\n \0 \r";
> +
> +typedef struct {
> + char magic[32]; /* savefileheader_magic */
> + /* All uint32_ts are in domain's byte order. */
> + uint32_t byteorder; /* SAVEFILE_BYTEORDER_VALUE */
> + uint32_t mandatory_flags; /* unknown flags => reject restore */
> + uint32_t optional_flags; /* unknown flags => reject restore */
> + uint32_t optional_data_len; /* skip, or skip tail, if not understood */
> +} SaveFileHeader;
> +
I don't think SaveFileHeader is consistent with the naming convention
used so far; could you please call it save_file_header instead?
> -static void parse_config_file(const char *filename,
> +static void parse_config_data(const char *configfile_filename_report,
> + const char *configfile_data,
> + int configfile_len,
> libxl_domain_create_info *c_info,
> libxl_domain_build_info *b_info,
> libxl_device_disk **disks,
I think at this point we should remove const 'char *filename' altogether
> @@ -693,27 +746,124 @@ static void create_domain(int debug, int daemonize, const char *config_file, con
> int ret;
> libxl_device_model_starting *dm_starting = 0;
> libxl_waiter *w1 = NULL, *w2 = NULL;
> + void *config_data = 0;
> + int config_len = 0;
> + int restore_fd = -1;
> + SaveFileHeader hdr;
> +
> memset(&dm_info, 0x00, sizeof(dm_info));
>
> + if (libxl_ctx_init(&ctx, LIBXL_VERSION)) {
> + fprintf(stderr, "cannot init xl context\n");
> + exit(1);
> + }
> +
> + if (restore_file) {
> + uint8_t *optdata_begin = 0;
> + const uint8_t *optdata_here = 0;
> + union { uint32_t u32; char b[4]; } u32buf;
> +
> + restore_fd = open(restore_file, O_RDONLY);
> +
> + if (suspend_old_xl_format) {
> + memset(&hdr,0,sizeof(hdr));
just remove this case
> + } else {
> + uint32_t badflags;
> +
> + CHK_ERRNO( libxl_read_exactly(&ctx, restore_fd, &hdr,
> + sizeof(hdr), restore_file, "header") );
> + if (memcmp(hdr.magic, savefileheader_magic, sizeof(hdr.magic))) {
> + fprintf(stderr, "File has wrong magic number -"
> + " corrupt or needs -O?\n");
> + exit(2);
> + }
> + if (hdr.byteorder != SAVEFILE_BYTEORDER_VALUE) {
> + fprintf(stderr, "File has wrong byte order\n");
> + exit(2);
> + }
> + fprintf(stderr, "Loading new save file %s"
> + " (new xl fmt info"
> + " 0x%"PRIx32"/0x%"PRIx32"/%"PRIu32")\n",
> + restore_file, hdr.mandatory_flags, hdr.optional_flags,
> + hdr.optional_data_len);
> +
> + badflags = hdr.mandatory_flags & ~( 0 /* none understood yet */ );
> + if (badflags) {
> + fprintf(stderr, "Savefile has mandatory flag(s) 0x%"PRIx32" "
> + "which are not supported; need newer xl\n",
> + badflags);
> + exit(2);
> + }
> + }
> + if (hdr.optional_data_len) {
> + optdata_begin = xmalloc(hdr.optional_data_len);
> + CHK_ERRNO( libxl_read_exactly(&ctx, restore_fd, optdata_begin,
> + hdr.optional_data_len, restore_file, "optdata") );
> + }
> +
> +#define OPTDATA_LEFT (hdr.optional_data_len - (optdata_here - optdata_begin))
> +#define WITH_OPTDATA(amt, body) \
> + if (OPTDATA_LEFT < (amt)) { \
> + fprintf(stderr, "Savefile truncated.\n"); exit(2); \
> + } else { \
> + body; \
> + optdata_here += (amt); \
> + }
> +
> + optdata_here = optdata_begin;
> +
> + if (OPTDATA_LEFT) {
> + fprintf(stderr, " Savefile contains xl domain config\n");
> + WITH_OPTDATA(4, {
> + memcpy(u32buf.b, optdata_here, 4);
> + config_len = u32buf.u32;
> + });
> + WITH_OPTDATA(config_len, {
> + config_data = xmalloc(config_len);
> + memcpy(config_data, optdata_here, config_len);
> + });
> + }
> +
> + }
> +
> + if (config_file) {
> + free(config_data); config_data = 0;
> + ret = libxl_read_file_contents(&ctx, config_file,
> + &config_data, &config_len);
> + if (ret) { fprintf(stderr, "Failed to read config file: %s: %s\n",
> + config_file, strerror(errno)); exit(1); }
> + } else {
> + if (!config_data) {
> + fprintf(stderr, "Config file not specified and"
> + " none in save file\n");
> + exit(1);
> + }
> + config_file = "<saved>";
> + }
> +
> printf("Parsing config file %s\n", config_file);
> - parse_config_file(config_file, &info1, &info2, &disks, &num_disks, &vifs, &num_vifs, &pcidevs, &num_pcidevs, &vfbs, &num_vfbs, &vkbs, &num_vkbs, &dm_info);
> +
> + parse_config_data(config_file, config_data, config_len, &info1, &info2, &disks, &num_disks, &vifs, &num_vifs, &pcidevs, &num_pcidevs, &vfbs, &num_vfbs, &vkbs, &num_vkbs, &dm_info);
> +
just remove config_file
> @@ -900,16 +1048,18 @@ static void help(char *command)
> printf("Usage: xl unpause <Domain>\n\n");
> printf("Unpause a paused domain.\n\n");
> } else if(!strcmp(command, "save")) {
> - printf("Usage: xl save [options] <Domain> <CheckpointFile>\n\n");
> + printf("Usage: xl save [options] <Domain> <CheckpointFile> [<ConfigFile>]\n\n");
> printf("Save a domain state to restore later.\n\n");
> printf("Options:\n\n");
> printf("-h Print this help.\n");
> + printf("-O Old (configless) xl save format.\n");
> printf("-c Leave domain running after creating the snapshot.\n");
> } else if(!strcmp(command, "restore")) {
> - printf("Usage: xl restore [options] <ConfigFile> <CheckpointFile>\n\n");
> + printf("Usage: xl restore [options] [<ConfigFile>] <CheckpointFile>\n\n");
> printf("Restore a domain from a saved state.\n\n");
> printf("Options:\n\n");
> printf("-h Print this help.\n");
> + printf("-O Old (configless) xl save format.\n");
> printf("-p Do not unpause domain after restoring it.\n");
> printf("-e Do not wait in the background for the death of the domain.\n");
do not add [<ConfigFile>], if you really want to provide this feature,
use an option instead, so that we keep compatibility with xm:
xl save [options] <Domain> <CheckpointFile>
xl restore [options] <CheckpointFile>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 10/11] xl: Domain creation logging fixes
2010-03-25 19:04 [PATCH 00/11] Migration support for "xl" Ian Jackson
` (8 preceding siblings ...)
2010-03-25 19:04 ` [PATCH 09/11] xl: New savefile format. Save domain config when saving a domain Ian Jackson
@ 2010-03-25 19:04 ` Ian Jackson
2010-03-25 19:04 ` [PATCH 11/11] xl: Migration support Ian Jackson
2010-04-01 11:10 ` [PATCH 00/11] Migration support for "xl" Stefano Stabellini
11 siblings, 0 replies; 29+ messages in thread
From: Ian Jackson @ 2010-03-25 19:04 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Ian Jackson
* Make create_domain always return to caller
* Have create_domain set its log callback sooner
* Actually write things to logfile, and some error checking
With some combinations of options, create_domain would never return to
the caller, since it would have called daemon and will later exit. So
we fork an additional time, so that we can call daemon in the child
and also return to the caller in the parent. It's a shame that
there's no version of daemon(3) that allows us to do this without the
extra code and pointless extra fork.
daemon(0,0) closes all the fds. So we need to call daemon(0,1) and
organise detaching our stdin/out/err ourselves. Doing this makes
messages actually appear in the xl logfile in /var/log/xen.
Finally, make create_domain call libxl_ctx_set_log sooner. This makes
some lost messages appear.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
tools/libxl/xl.c | 43 ++++++++++++++++++++++++++++++++++++-------
1 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
index f3729f3..7d35db1 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -743,7 +743,7 @@ static void create_domain(int debug, int daemonize, const char *config_file, con
int num_disks = 0, num_vifs = 0, num_pcidevs = 0, num_vfbs = 0, num_vkbs = 0;
int i, fd;
int need_daemon = 1;
- int ret;
+ int ret, rc;
libxl_device_model_starting *dm_starting = 0;
libxl_waiter *w1 = NULL, *w2 = NULL;
void *config_data = 0;
@@ -757,6 +757,7 @@ static void create_domain(int debug, int daemonize, const char *config_file, con
fprintf(stderr, "cannot init xl context\n");
exit(1);
}
+ libxl_ctx_set_log(&ctx, log_callback, NULL);
if (restore_file) {
uint8_t *optdata_begin = 0;
@@ -851,8 +852,6 @@ static void create_domain(int debug, int daemonize, const char *config_file, con
start:
domid = 0;
- libxl_ctx_set_log(&ctx, log_callback, NULL);
-
ret = libxl_domain_make(&ctx, &info1, &domid);
if (ret) {
fprintf(stderr, "cannot make domain: %d\n", ret);
@@ -931,17 +930,47 @@ start:
if (need_daemon) {
char *fullname, *name;
+ pid_t child1, got_child;
+ int nullfd;
+
+ child1 = libxl_fork(&ctx);
+ if (child1) {
+ int status;
+ for (;;) {
+ got_child = waitpid(child1, &status, 0);
+ if (got_child == child1) break;
+ assert(got_child == -1);
+ if (errno != EINTR) {
+ perror("failed to wait for daemonizing child");
+ return ERROR_FAIL;
+ }
+ }
+ if (status) {
+ libxl_report_child_exitstatus(&ctx, XL_LOG_ERROR,
+ "daemonizing child", child1, status);
+ return ERROR_FAIL;
+ }
+ return 0; /* caller gets success in parent */
+ }
asprintf(&name, "xl-%s", info1.name);
- libxl_create_logfile(&ctx, name, &fullname);
- logfile = open(fullname, O_WRONLY|O_CREAT, 0644);
+ rc = libxl_create_logfile(&ctx, name, &fullname);
+ if (rc) return rc;
+
+ CHK_ERRNO(( logfile = open(fullname, O_WRONLY|O_CREAT, 0644) )<0);
free(fullname);
free(name);
- daemon(0, 0);
+ CHK_ERRNO(( nullfd = open("/dev/null", O_RDONLY) )<0);
+ dup2(nullfd, 0);
+ dup2(logfile, 1);
+ dup2(logfile, 2);
+
+ daemon(0, 1);
need_daemon = 0;
}
- LOG("Waiting for domain %s (domid %d) to die", info1.name, domid);
+ LOG("Waiting for domain %s (domid %d) to die [pid %ld]",
+ info1.name, domid, (long)getpid());
w1 = (libxl_waiter*) xmalloc(sizeof(libxl_waiter) * num_disks);
w2 = (libxl_waiter*) xmalloc(sizeof(libxl_waiter));
libxl_wait_for_disk_ejects(&ctx, domid, disks, num_disks, w1);
--
1.5.6.5
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 11/11] xl: Migration support
2010-03-25 19:04 [PATCH 00/11] Migration support for "xl" Ian Jackson
` (9 preceding siblings ...)
2010-03-25 19:04 ` [PATCH 10/11] xl: Domain creation logging fixes Ian Jackson
@ 2010-03-25 19:04 ` Ian Jackson
2010-04-01 11:10 ` Stefano Stabellini
2010-04-01 11:10 ` [PATCH 00/11] Migration support for "xl" Stefano Stabellini
11 siblings, 1 reply; 29+ messages in thread
From: Ian Jackson @ 2010-03-25 19:04 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Ian Jackson
Implement "xl migrate".
ssh is used as the transport by default, although this can be
overridden by specifying a different sshcommand. This is a very
standard approach nowadays and avoids the need for daemons at the
target host in the default configuration, while providing flexibility
to admins. (In the future it might be nice to support plain
unencrypted migration over TCP, which we do not rule out now, although
it is not currently implemented.)
Properties of the migration protocol:
* The domain on the target machine is named "<domname>--incoming"
while it is being transferred.
* The domain on the source machine is renamed "<domain>--migratedaway"
before we give the target permission to rename and unpause.
* The locking in libxl_domain_rename ensures that of two
simultaneous migration attempts no more than one will succeed.
* We go to some considerable effort to avoid leaving the domain in
a bad state if something goes wrong with one of the ends or the
network, although there is still (inevitably) a possibility of a
unresolvable state (in case of very badly timed network failure)
which is probably best resolved by destroying the domain at both
ends.
Incidental changes:
create_domain now returns a libxl error code rather than exiting on error.
domain_qualifier_to_domid takes an argument for saving the supplied name.
Various common information (eg the domid we are operating on) is
now in static variables rather than locals.
New ERROR_BADFAIL error code for reporting unpleasant failures.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
migration fixes
Migration fixup error reporting
migration - resume, not unpause
Error handling fixes; initialise common_ctx
migration fixes
---
tools/libxl/libxl.h | 1 +
tools/libxl/xl.c | 675 ++++++++++++++++++++++++++++++++++++++++++++-------
2 files changed, 590 insertions(+), 86 deletions(-)
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index d57dd10..62b6cff 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -241,6 +241,7 @@ enum {
ERROR_NI = -3,
ERROR_NOMEM = -4,
ERROR_INVAL = -5,
+ ERROR_BADFAIL = -6,
};
#define LIBXL_VERSION 0
diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
index 7d35db1..53be99c 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -17,6 +17,7 @@
#include "libxl_osdeps.h"
#include <stdio.h>
+#include <assert.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
@@ -42,9 +43,33 @@ int logfile = 2;
static int suspend_old_xl_format = 0;
+static struct libxl_ctx common_ctx;
+static uint32_t common_domid;
+static const char *common_domname;
+static uint8_t *common_config_data;
+static int common_config_len;
+
+static char *migration_domname;
+
static const char savefileheader_magic[32]=
"Xen saved domain, xl format\n \0 \r";
+static const char migrate_receiver_banner[]=
+ "xl migration receiver ready, send binary domain data.\n";
+static const char migrate_receiver_ready[]=
+ "domain received, ready to unpause";
+static const char migrate_permission_to_go[]=
+ "domain is yours, you are cleared to unpause";
+static const char migrate_report[]=
+ "my copy unpause results are as follows";
+ /* followed by one byte:
+ * 0: everything went well, domain is running
+ * next thing is we all exit
+ * non-0: things went badly
+ * next thing should be a migrate_permission_to_go
+ * from target to source
+ */
+
typedef struct {
char magic[32]; /* savefileheader_magic */
/* All uint32_ts are in domain's byte order. */
@@ -69,7 +94,7 @@ void log_callback(void *userdata, int loglevel, const char *file, int line, cons
write(logfile, str, strlen(str));
}
-static int domain_qualifier_to_domid(struct libxl_ctx *ctx, char *p, uint32_t *domid)
+static int domain_qualifier_to_domid(struct libxl_ctx *ctx, char *p, uint32_t *domid, const char **expected_name_r)
{
int i, alldigit;
@@ -83,10 +108,12 @@ static int domain_qualifier_to_domid(struct libxl_ctx *ctx, char *p, uint32_t *d
if (i > 0 && alldigit) {
*domid = strtoul(p, NULL, 10);
+ if (expected_name_r) *expected_name_r = 0;
return 0;
} else {
/* check here if it's a uuid and do proper conversion */
}
+ if (expected_name_r) *expected_name_r = p;
return libxl_name_to_domid(ctx, p, domid);
}
@@ -726,7 +753,7 @@ static void *xrealloc(void *ptr, size_t sz) {
return r;
}
-static void create_domain(int debug, int daemonize, const char *config_file, const char *restore_file, int paused)
+static int create_domain(int debug, int daemonize, const char *config_file, const char *restore_file, int paused, int migrate_fd /* -1 means none */)
{
struct libxl_ctx ctx;
uint32_t domid;
@@ -755,7 +782,7 @@ static void create_domain(int debug, int daemonize, const char *config_file, con
if (libxl_ctx_init(&ctx, LIBXL_VERSION)) {
fprintf(stderr, "cannot init xl context\n");
- exit(1);
+ return ERROR_FAIL;
}
libxl_ctx_set_log(&ctx, log_callback, NULL);
@@ -763,8 +790,9 @@ static void create_domain(int debug, int daemonize, const char *config_file, con
uint8_t *optdata_begin = 0;
const uint8_t *optdata_here = 0;
union { uint32_t u32; char b[4]; } u32buf;
-
- restore_fd = open(restore_file, O_RDONLY);
+
+ restore_fd = migrate_fd >= 0 ? migrate_fd :
+ open(restore_file, O_RDONLY);
if (suspend_old_xl_format) {
memset(&hdr,0,sizeof(hdr));
@@ -776,11 +804,11 @@ static void create_domain(int debug, int daemonize, const char *config_file, con
if (memcmp(hdr.magic, savefileheader_magic, sizeof(hdr.magic))) {
fprintf(stderr, "File has wrong magic number -"
" corrupt or needs -O?\n");
- exit(2);
+ return ERROR_INVAL;
}
if (hdr.byteorder != SAVEFILE_BYTEORDER_VALUE) {
fprintf(stderr, "File has wrong byte order\n");
- exit(2);
+ return ERROR_INVAL;
}
fprintf(stderr, "Loading new save file %s"
" (new xl fmt info"
@@ -793,7 +821,7 @@ static void create_domain(int debug, int daemonize, const char *config_file, con
fprintf(stderr, "Savefile has mandatory flag(s) 0x%"PRIx32" "
"which are not supported; need newer xl\n",
badflags);
- exit(2);
+ return ERROR_INVAL;
}
}
if (hdr.optional_data_len) {
@@ -803,12 +831,13 @@ static void create_domain(int debug, int daemonize, const char *config_file, con
}
#define OPTDATA_LEFT (hdr.optional_data_len - (optdata_here - optdata_begin))
-#define WITH_OPTDATA(amt, body) \
- if (OPTDATA_LEFT < (amt)) { \
- fprintf(stderr, "Savefile truncated.\n"); exit(2); \
- } else { \
- body; \
- optdata_here += (amt); \
+#define WITH_OPTDATA(amt, body) \
+ if (OPTDATA_LEFT < (amt)) { \
+ fprintf(stderr, "Savefile truncated.\n"); \
+ return ERROR_INVAL; \
+ } else { \
+ body; \
+ optdata_here += (amt); \
}
optdata_here = optdata_begin;
@@ -832,12 +861,12 @@ static void create_domain(int debug, int daemonize, const char *config_file, con
ret = libxl_read_file_contents(&ctx, config_file,
&config_data, &config_len);
if (ret) { fprintf(stderr, "Failed to read config file: %s: %s\n",
- config_file, strerror(errno)); exit(1); }
+ config_file, strerror(errno)); return ERROR_FAIL; }
} else {
if (!config_data) {
fprintf(stderr, "Config file not specified and"
" none in save file\n");
- exit(1);
+ return ERROR_INVAL;
}
config_file = "<saved>";
}
@@ -846,6 +875,17 @@ static void create_domain(int debug, int daemonize, const char *config_file, con
parse_config_data(config_file, config_data, config_len, &info1, &info2, &disks, &num_disks, &vifs, &num_vifs, &pcidevs, &num_pcidevs, &vfbs, &num_vfbs, &vkbs, &num_vkbs, &dm_info);
+ if (migrate_fd >= 0) {
+ if (info1.name) {
+ /* when we receive a domain we get its name from the config
+ * file; and we receive it to a temporary name */
+ assert(!common_domname);
+ common_domname = info1.name;
+ asprintf(&migration_domname, "%s--incoming", info1.name);
+ info1.name = migration_domname;
+ }
+ }
+
if (debug)
printf_info(&info1, &info2, disks, num_disks, vifs, num_vifs, pcidevs, num_pcidevs, vfbs, num_vfbs, vkbs, num_vkbs, &dm_info);
@@ -855,14 +895,15 @@ start:
ret = libxl_domain_make(&ctx, &info1, &domid);
if (ret) {
fprintf(stderr, "cannot make domain: %d\n", ret);
- exit(1);
+ return ERROR_FAIL;
}
+ common_domid = domid;
ret = libxl_userdata_store(&ctx, domid, "xl",
config_data, config_len);
if (ret) {
perror("cannot save config file");
- exit(1);
+ return ERROR_FAIL;
}
if (!restore_file || !need_daemon) {
@@ -873,12 +914,11 @@ start:
ret = libxl_domain_build(&ctx, &info2, domid, &state);
} else {
ret = libxl_domain_restore(&ctx, &info2, domid, restore_fd, &state, &dm_info);
- close(restore_fd);
}
if (ret) {
fprintf(stderr, "cannot (re-)build domain: %d\n", ret);
- exit(1);
+ return ERROR_FAIL;
}
for (i = 0; i < num_disks; i++) {
@@ -886,7 +926,7 @@ start:
ret = libxl_device_disk_add(&ctx, domid, &disks[i]);
if (ret) {
fprintf(stderr, "cannot add disk %d to domain: %d\n", i, ret);
- exit(1);
+ return ERROR_FAIL;
}
}
for (i = 0; i < num_vifs; i++) {
@@ -894,7 +934,7 @@ start:
ret = libxl_device_nic_add(&ctx, domid, &vifs[i]);
if (ret) {
fprintf(stderr, "cannot add nic %d to domain: %d\n", i, ret);
- exit(1);
+ return ERROR_FAIL;
}
}
if (info1.hvm) {
@@ -926,7 +966,7 @@ start:
libxl_domain_unpause(&ctx, domid);
if (!daemonize)
- exit(0);
+ return 0; /* caller gets success in parent */
if (need_daemon) {
char *fullname, *name;
@@ -1023,12 +1063,7 @@ start:
}
close(logfile);
- free(disks);
- free(vifs);
- free(vfbs);
- free(vkbs);
- free(pcidevs);
- free(config_data);
+ exit(0);
}
static void help(char *command)
@@ -1091,6 +1126,25 @@ static void help(char *command)
printf("-O Old (configless) xl save format.\n");
printf("-p Do not unpause domain after restoring it.\n");
printf("-e Do not wait in the background for the death of the domain.\n");
+ printf("-d Enable debug messages.\n");
+ } else if(!strcmp(command, "migrate")) {
+ printf("Usage: xl migrate [options] <Domain> <host>\n\n");
+ printf("Save a domain state to restore later.\n\n");
+ printf("Options:\n\n");
+ printf("-h Print this help.\n");
+ printf("-C <config> Send <config> instead of config file from creation.\n");
+ printf("-s <sshcommand> Use <sshcommand> instead of ssh. String will be passed to sh. If empty, run <host> instead of ssh <host> xl migrate-receive [-d -e]\n");
+ printf("-e Do not wait in the background (on <host>) for the death of the domain.\n");
+ } else if(!strcmp(command, "migrate-receive")) {
+ printf("Usage: xl migrate-receive - for internal use only");
+ } else if(!strcmp(command, "restore")) {
+ printf("Usage: xl restore [options] [<ConfigFile>] <CheckpointFile>\n\n");
+ printf("Restore a domain from a saved state.\n\n");
+ printf("Options:\n\n");
+ printf("-h Print this help.\n");
+ printf("-O Old (configless) xl save format.\n");
+ printf("-p Do not unpause domain after restoring it.\n");
+ printf("-e Do not wait in the background for the death of the domain.\n");
} else if(!strcmp(command, "destroy")) {
printf("Usage: xl destroy <Domain>\n\n");
printf("Terminate a domain immediately.\n\n");
@@ -1126,7 +1180,7 @@ void set_memory_target(char *p, char *mem)
}
libxl_ctx_set_log(&ctx, log_callback, NULL);
- if (domain_qualifier_to_domid(&ctx, p, &domid) < 0) {
+ if (domain_qualifier_to_domid(&ctx, p, &domid, 0) < 0) {
fprintf(stderr, "%s is an invalid domain identifier\n", p);
exit(2);
}
@@ -1177,7 +1231,7 @@ void console(char *p, int cons_num)
}
libxl_ctx_set_log(&ctx, log_callback, NULL);
- if (domain_qualifier_to_domid(&ctx, p, &domid) < 0) {
+ if (domain_qualifier_to_domid(&ctx, p, &domid, 0) < 0) {
fprintf(stderr, "%s is an invalid domain identifier\n", p);
exit(2);
}
@@ -1197,7 +1251,7 @@ void cd_insert(char *dom, char *virtdev, char *phys)
}
libxl_ctx_set_log(&ctx, log_callback, NULL);
- if (domain_qualifier_to_domid(&ctx, dom, &domid) < 0) {
+ if (domain_qualifier_to_domid(&ctx, dom, &domid, 0) < 0) {
fprintf(stderr, "%s is an invalid domain identifier\n", dom);
exit(2);
}
@@ -1333,7 +1387,7 @@ void pcilist(char *dom)
}
libxl_ctx_set_log(&ctx, log_callback, NULL);
- if (domain_qualifier_to_domid(&ctx, dom, &domid) < 0) {
+ if (domain_qualifier_to_domid(&ctx, dom, &domid, 0) < 0) {
fprintf(stderr, "%s is an invalid domain identifier\n", dom);
exit(2);
}
@@ -1386,7 +1440,7 @@ void pcidetach(char *dom, char *bdf)
}
libxl_ctx_set_log(&ctx, log_callback, NULL);
- if (domain_qualifier_to_domid(&ctx, dom, &domid) < 0) {
+ if (domain_qualifier_to_domid(&ctx, dom, &domid, 0) < 0) {
fprintf(stderr, "%s is an invalid domain identifier\n", dom);
exit(2);
}
@@ -1435,7 +1489,7 @@ void pciattach(char *dom, char *bdf, char *vs)
}
libxl_ctx_set_log(&ctx, log_callback, NULL);
- if (domain_qualifier_to_domid(&ctx, dom, &domid) < 0) {
+ if (domain_qualifier_to_domid(&ctx, dom, &domid, 0) < 0) {
fprintf(stderr, "%s is an invalid domain identifier\n", dom);
exit(2);
}
@@ -1486,7 +1540,7 @@ void pause_domain(char *p)
}
libxl_ctx_set_log(&ctx, log_callback, NULL);
- if (domain_qualifier_to_domid(&ctx, p, &domid) < 0) {
+ if (domain_qualifier_to_domid(&ctx, p, &domid, 0) < 0) {
fprintf(stderr, "%s is an invalid domain identifier\n", p);
exit(2);
}
@@ -1504,7 +1558,7 @@ void unpause_domain(char *p)
}
libxl_ctx_set_log(&ctx, log_callback, NULL);
- if (domain_qualifier_to_domid(&ctx, p, &domid) < 0) {
+ if (domain_qualifier_to_domid(&ctx, p, &domid, 0) < 0) {
fprintf(stderr, "%s is an invalid domain identifier\n", p);
exit(2);
}
@@ -1522,7 +1576,7 @@ void destroy_domain(char *p)
}
libxl_ctx_set_log(&ctx, log_callback, NULL);
- if (domain_qualifier_to_domid(&ctx, p, &domid) < 0) {
+ if (domain_qualifier_to_domid(&ctx, p, &domid, 0) < 0) {
fprintf(stderr, "%s is an invalid domain identifier\n", p);
exit(2);
}
@@ -1592,33 +1646,45 @@ void list_vm(void)
free(info);
}
-int save_domain(char *p, char *filename, int checkpoint,
- const char *override_config_file)
+static void save_domain_core_begin(char *domain_spec,
+ const char *override_config_file)
{
- struct libxl_ctx ctx;
- uint32_t domid;
- int fd, rc;
-
- if (libxl_ctx_init(&ctx, LIBXL_VERSION)) {
+ /* fixme clone and hack of this init should be eliminated */
+ int rc;
+
+ if (libxl_ctx_init(&common_ctx, LIBXL_VERSION)) {
fprintf(stderr, "cannot init xl context\n");
exit(2);
}
- libxl_ctx_set_log(&ctx, log_callback, NULL);
+ libxl_ctx_set_log(&common_ctx, log_callback, NULL);
- if (domain_qualifier_to_domid(&ctx, p, &domid) < 0) {
- fprintf(stderr, "%s is an invalid domain identifier\n", p);
+ if (domain_qualifier_to_domid(&common_ctx, domain_spec, &common_domid, &common_domname)<0) {
+ fprintf(stderr, "%s is an invalid domain identifier\n", domain_spec);
exit(2);
}
- fd = open(filename, O_WRONLY|O_CREAT|O_TRUNC, 0644);
- if (fd < 0) {
- fprintf(stderr, "Failed to open temp file %s for writing\n", filename);
- exit(2);
+ if (!suspend_old_xl_format) {
+ /* configuration file in optional data: */
+
+ if (override_config_file) {
+ void *config_v = 0;
+ rc = libxl_read_file_contents(&common_ctx, override_config_file,
+ &config_v, &common_config_len);
+ common_config_data = config_v;
+ } else {
+ rc = libxl_userdata_retrieve(&common_ctx, common_domid, "xl",
+ &common_config_data, &common_config_len);
+ }
+ if (rc) {
+ fputs("Unable to get config file\n",stderr);
+ exit(2);
+ }
}
+}
+void save_domain_core_writeconfig(int fd, const char *filename)
+{
if (!suspend_old_xl_format) {
SaveFileHeader hdr;
- uint8_t *config_data = 0;
- int config_len;
uint8_t *optdata_begin;
union { uint32_t u32; char b[4]; } u32buf;
@@ -1637,34 +1703,15 @@ int save_domain(char *p, char *filename, int checkpoint,
} \
})
- /* configuration file in optional data: */
-
- if (override_config_file) {
- void *config_v = 0;
- rc = libxl_read_file_contents(&ctx, override_config_file,
- &config_v, &config_len);
- config_data = config_v;
- } else {
- rc = libxl_userdata_retrieve(&ctx, domid, "xl",
- &config_data, &config_len);
- }
- if (rc) {
- fputs("Unable to get config file\n",stderr);
- exit(2);
- }
- if (!config_len) {
- fputs(" Savefile will not contain xl domain config\n", stderr);
- }
-
- u32buf.u32 = config_len;
+ u32buf.u32 = common_config_len;
ADD_OPTDATA(u32buf.b, 4);
- ADD_OPTDATA(config_data, config_len);
+ ADD_OPTDATA(common_config_data, common_config_len);
/* that's the optional data */
- CHK_ERRNO( libxl_write_exactly(&ctx, fd,
+ CHK_ERRNO( libxl_write_exactly(&common_ctx, fd,
&hdr, sizeof(hdr), filename, "header") );
- CHK_ERRNO( libxl_write_exactly(&ctx, fd,
+ CHK_ERRNO( libxl_write_exactly(&common_ctx, fd,
optdata_begin, hdr.optional_data_len, filename, "header") );
fprintf(stderr, "Saving to %s new xl format (info"
@@ -1672,14 +1719,371 @@ int save_domain(char *p, char *filename, int checkpoint,
filename, hdr.mandatory_flags, hdr.optional_flags,
hdr.optional_data_len);
}
+}
+
+int save_domain(char *p, char *filename, int checkpoint,
+ const char *override_config_file)
+{
+ int fd;
+
+ save_domain_core_begin(p, override_config_file);
+
+ if (!suspend_old_xl_format && !common_config_len) {
+ fputs(" Savefile will not contain xl domain config\n", stderr);
+ }
+
+ fd = open(filename, O_WRONLY|O_CREAT|O_TRUNC, 0644);
+ if (fd < 0) {
+ fprintf(stderr, "Failed to open temp file %s for writing\n", filename);
+ exit(2);
+ }
+
+ save_domain_core_writeconfig(fd, filename);
- libxl_domain_suspend(&ctx, NULL, domid, fd);
+ libxl_domain_suspend(&common_ctx, NULL, common_domid, fd);
close(fd);
if (checkpoint)
- libxl_domain_unpause(&ctx, domid);
+ libxl_domain_unpause(&common_ctx, common_domid);
else
- libxl_domain_destroy(&ctx, domid, 0);
+ libxl_domain_destroy(&common_ctx, common_domid, 0);
+
+ exit(0);
+}
+
+static int migrate_read_fixedmessage(int fd, const void *msg, int msgsz,
+ const char *what, const char *rune) {
+ char buf[msgsz];
+ const char *stream;
+ int rc;
+
+ stream = rune ? "migration receiver stream" : "migration stream";
+ rc = libxl_read_exactly(&common_ctx, fd, buf, msgsz, stream, what);
+ if (rc) return ERROR_FAIL;
+
+ if (memcmp(buf, msg, msgsz)) {
+ fprintf(stderr, "%s contained unexpected data instead of %s\n",
+ stream, what);
+ if (rune)
+ fprintf(stderr, "(command run was: %s )\n", rune);
+ return ERROR_FAIL;
+ }
+ return 0;
+}
+
+static void migration_child_report(pid_t migration_child, int recv_fd) {
+ pid_t child;
+ int status, sr;
+ struct timeval now, waituntil, timeout;
+ static const struct timeval pollinterval = { 0, 1000 }; /* 1ms */
+
+ if (!migration_child) return;
+
+ CHK_ERRNO( gettimeofday(&waituntil, 0) );
+ waituntil.tv_sec += 2;
+
+ for (;;) {
+ child = waitpid(migration_child, &status, WNOHANG);
+
+ if (child == migration_child) {
+ if (status)
+ libxl_report_child_exitstatus(&common_ctx, XL_LOG_INFO,
+ "migration target process",
+ migration_child, status);
+ break;
+ }
+ if (child == -1) {
+ if (errno == EINTR) continue;
+ fprintf(stderr, "wait for migration child [%ld] failed: %s\n",
+ (long)migration_child, strerror(errno));
+ break;
+ }
+ assert(child == 0);
+
+ CHK_ERRNO( gettimeofday(&now, 0) );
+ if (timercmp(&now, &waituntil, >)) {
+ fprintf(stderr, "migration child [%ld] not exiting, no longer"
+ " waiting (exit status will be unreported)\n",
+ (long)migration_child);
+ break;
+ }
+ timersub(&waituntil, &now, &timeout);
+
+ if (recv_fd >= 0) {
+ fd_set readfds, exceptfds;
+ FD_ZERO(&readfds);
+ FD_ZERO(&exceptfds);
+ FD_SET(recv_fd, &readfds);
+ FD_SET(recv_fd, &exceptfds);
+ sr = select(recv_fd+1, &readfds,0,&exceptfds, &timeout);
+ } else {
+ if (timercmp(&timeout, &pollinterval, >))
+ timeout = pollinterval;
+ sr = select(0,0,0,0, &timeout);
+ }
+ if (sr > 0) {
+ recv_fd = -1;
+ } else if (sr == 0) {
+ } else if (sr == -1) {
+ if (errno != EINTR) {
+ fprintf(stderr, "migration child [%ld] exit wait select"
+ " failed unexpectedly: %s\n",
+ (long)migration_child, strerror(errno));
+ break;
+ }
+ }
+ }
+ migration_child = 0;
+}
+
+static void migrate_domain(char *domain_spec, const char *rune,
+ const char *override_config_file)
+{
+ pid_t child = -1;
+ int rc;
+ int sendpipe[2], recvpipe[2];
+ int send_fd, recv_fd;
+ libxl_domain_suspend_info suspinfo;
+ char *away_domname;
+ char rc_buf;
+
+ save_domain_core_begin(domain_spec, override_config_file);
+
+ if (!common_domname) {
+ common_domname = libxl_domid_to_name(&common_ctx, common_domid);
+ /* libxl_domid_to_name fails ? don't bother with names then */
+ }
+
+ if (!common_config_len) {
+ fprintf(stderr, "No config file stored for running domain and "
+ "none supplied - cannot migrate.\n");
+ exit(1);
+ }
+
+ MUST( libxl_pipe(&common_ctx, sendpipe) );
+ MUST( libxl_pipe(&common_ctx, recvpipe) );
+
+ child = libxl_fork(&common_ctx);
+ if (child==-1) exit(1);
+
+ if (!child) {
+ dup2(sendpipe[0], 0);
+ dup2(recvpipe[1], 1);
+ close(sendpipe[0]); close(sendpipe[1]);
+ close(recvpipe[0]); close(recvpipe[1]);
+ execlp("sh","sh","-c",rune,(char*)0);
+ perror("failed to exec sh");
+ exit(-1);
+ }
+
+ close(sendpipe[0]);
+ close(recvpipe[1]);
+ send_fd = sendpipe[1];
+ recv_fd = recvpipe[0];
+
+ signal(SIGPIPE, SIG_IGN);
+ /* if receiver dies, we get an error and can clean up
+ rather than just dying */
+
+ rc = migrate_read_fixedmessage(recv_fd, migrate_receiver_banner,
+ sizeof(migrate_receiver_banner)-1,
+ "banner", rune);
+ if (rc) {
+ close(send_fd);
+ migration_child_report(child, recv_fd);
+ exit(-rc);
+ }
+
+ save_domain_core_writeconfig(send_fd, "migration stream");
+
+ memset(&suspinfo, 0, sizeof(suspinfo));
+ suspinfo.flags |= XL_SUSPEND_LIVE;
+ rc = libxl_domain_suspend(&common_ctx, &suspinfo, common_domid, send_fd);
+ if (rc) {
+ fprintf(stderr, "migration sender: libxl_domain_suspend failed"
+ " (rc=%d)\n", rc);
+ goto failed_resume;
+ }
+
+ fprintf(stderr, "migration sender: Transfer complete.\n");
+
+ rc = migrate_read_fixedmessage(recv_fd, migrate_receiver_ready,
+ sizeof(migrate_receiver_ready),
+ "ready message", rune);
+ if (rc) goto failed_resume;
+
+ /* right, at this point we are about give the destination
+ * permission to rename and resume, so we must first rename the
+ * domain away ourselves */
+
+ fprintf(stderr, "migration sender: Target has acknowledged transfer.\n");
+
+ if (common_domname) {
+ asprintf(&away_domname, "%s--migratedaway", common_domname);
+ rc = libxl_domain_rename(&common_ctx, common_domid,
+ common_domname, away_domname, 0);
+ if (rc) goto failed_resume;
+ }
+
+ /* point of no return - as soon as we have tried to say
+ * "go" to the receiver, it's not safe to carry on. We leave
+ * the domain renamed to %s--migratedaway in case that's helpful.
+ */
+
+ fprintf(stderr, "migration sender: Giving target permission to start.\n");
+
+ rc = libxl_write_exactly(&common_ctx, send_fd,
+ migrate_permission_to_go,
+ sizeof(migrate_permission_to_go),
+ "migration stream", "GO message");
+ if (rc) goto failed_badly;
+
+ rc = migrate_read_fixedmessage(recv_fd, migrate_report,
+ sizeof(migrate_report),
+ "success/failure report message", rune);
+ if (rc) goto failed_badly;
+
+ rc = libxl_read_exactly(&common_ctx, recv_fd,
+ &rc_buf, 1,
+ "migration ack stream", "success/failure status");
+ if (rc) goto failed_badly;
+
+ if (rc_buf) {
+ fprintf(stderr, "migration sender: Target reports startup failure"
+ " (status code %d).\n", rc_buf);
+
+ rc = migrate_read_fixedmessage(recv_fd, migrate_permission_to_go,
+ sizeof(migrate_permission_to_go),
+ "permission for sender to resume",
+ rune);
+ if (rc) goto failed_badly;
+
+ fprintf(stderr, "migration sender: Trying to resume at our end.\n");
+
+ if (common_domname) {
+ libxl_domain_rename(&common_ctx, common_domid,
+ away_domname, common_domname, 0);
+ }
+ rc = libxl_domain_resume(&common_ctx, common_domid);
+ if (!rc) fprintf(stderr, "migration sender: Resumed OK.\n");
+
+ fprintf(stderr, "Migration failed due to problems at target.\n");
+ exit(-ERROR_FAIL);
+ }
+
+ fprintf(stderr, "migration sender: Target reports successful startup.\n");
+ libxl_domain_destroy(&common_ctx, common_domid, 1); /* bang! */
+ fprintf(stderr, "Migration successful.\n");
+ exit(0);
+
+ failed_resume:
+ close(send_fd);
+ migration_child_report(child, recv_fd);
+ fprintf(stderr, "Migration failed, resuming at sender.\n");
+ libxl_domain_resume(&common_ctx, common_domid);
+ exit(-ERROR_FAIL);
+
+ failed_badly:
+ fprintf(stderr,
+ "** Migration failed during final handshake **\n"
+ "Domain state is now undefined !\n"
+ "Please CHECK AT BOTH ENDS for running instances, before renaming and\n"
+ " resuming at most one instance. Two simultaneous instances of the domain\n"
+ " would probably result in SEVERE DATA LOSS and it is now your\n"
+ " responsibility to avoid that. Sorry.\n");
+
+ close(send_fd);
+ migration_child_report(child, recv_fd);
+ exit(-ERROR_BADFAIL);
+}
+
+static void migrate_receive(int debug, int daemonize)
+{
+ int rc, rc2;
+ char rc_buf;
+
+ signal(SIGPIPE, SIG_IGN);
+ /* if we get SIGPIPE we'd rather just have it as an error */
+
+ fprintf(stderr, "migration target: Ready to receive domain.\n");
+
+ CHK_ERRNO( libxl_write_exactly(&common_ctx, 1,
+ migrate_receiver_banner,
+ sizeof(migrate_receiver_banner)-1,
+ "migration ack stream",
+ "banner") );
+
+ rc = create_domain(debug, daemonize,
+ 0 /* no config file, use incoming */,
+ "incoming migration stream", 1, 0);
+ if (rc) {
+ fprintf(stderr, "migration target: Domain creation failed"
+ " (code %d).\n", rc);
+ exit(-rc);
+ }
+
+ fprintf(stderr, "migration target: Transfer complete,"
+ " requesting permission to start domain.\n");
+
+ rc = libxl_write_exactly(&common_ctx, 1,
+ migrate_receiver_ready,
+ sizeof(migrate_receiver_ready),
+ "migration ack stream", "ready message");
+ if (rc) exit(-rc);
+
+ rc = migrate_read_fixedmessage(0, migrate_permission_to_go,
+ sizeof(migrate_permission_to_go),
+ "GO message", 0);
+ if (rc) goto perhaps_destroy_notify_rc;
+
+ fprintf(stderr, "migration target: Got permission, starting domain.\n");
+
+ if (migration_domname) {
+ rc = libxl_domain_rename(&common_ctx, common_domid,
+ migration_domname, common_domname, 0);
+ if (rc) goto perhaps_destroy_notify_rc;
+ }
+
+ rc = libxl_domain_unpause(&common_ctx, common_domid);
+ if (rc) goto perhaps_destroy_notify_rc;
+
+ fprintf(stderr, "migration target: Domain started successsfully.\n");
+ rc = 0;
+
+ perhaps_destroy_notify_rc:
+ rc2 = libxl_write_exactly(&common_ctx, 1,
+ migrate_report, sizeof(migrate_report),
+ "migration ack stream",
+ "success/failure report");
+ if (rc2) exit(-ERROR_BADFAIL);
+
+ rc_buf = -rc;
+ assert(!!rc_buf == !!rc);
+ rc2 = libxl_write_exactly(&common_ctx, 1, &rc_buf, 1,
+ "migration ack stream",
+ "success/failure code");
+ if (rc2) exit(-ERROR_BADFAIL);
+
+ if (rc) {
+ fprintf(stderr, "migration target: Failure, destroying our copy.\n");
+
+ rc2 = libxl_domain_destroy(&common_ctx, common_domid, 1);
+ if (rc2) {
+ fprintf(stderr, "migration target: Failed to destroy our copy"
+ " (code %d).\n", rc2);
+ exit(-ERROR_BADFAIL);
+ }
+
+ fprintf(stderr, "migration target: Cleanup OK, granting sender"
+ " permission to resume.\n");
+
+ rc2 = libxl_write_exactly(&common_ctx, 1,
+ migrate_permission_to_go,
+ sizeof(migrate_permission_to_go),
+ "migration ack stream",
+ "permission to sender to have domain back");
+ if (rc2) exit(-ERROR_BADFAIL);
+ }
exit(0);
}
@@ -1689,7 +2093,7 @@ int main_restore(int argc, char **argv)
char *checkpoint_file = NULL;
char *config_file = NULL;
int paused = 0, debug = 0, daemonize = 1;
- int opt;
+ int opt, rc;
while ((opt = getopt(argc, argv, "hpdeO")) != -1) {
switch (opt) {
@@ -1723,7 +2127,39 @@ int main_restore(int argc, char **argv)
help("restore");
exit(2);
}
- create_domain(debug, daemonize, config_file, checkpoint_file, paused);
+ rc = create_domain(debug, daemonize, config_file,
+ checkpoint_file, paused, -1);
+ exit(-rc);
+}
+
+int main_migrate_receive(int argc, char **argv)
+{
+ int debug = 0, daemonize = 1;
+ int opt;
+
+ while ((opt = getopt(argc, argv, "hed")) != -1) {
+ switch (opt) {
+ case 'h':
+ help("restore");
+ exit(2);
+ break;
+ case 'e':
+ daemonize = 0;
+ break;
+ case 'd':
+ debug = 1;
+ break;
+ default:
+ fprintf(stderr, "option not supported\n");
+ break;
+ }
+ }
+
+ if (argc-optind != 0) {
+ help("restore");
+ exit(2);
+ }
+ migrate_receive(debug, daemonize);
exit(0);
}
@@ -1763,6 +2199,60 @@ int main_save(int argc, char **argv)
exit(0);
}
+int main_migrate(int argc, char **argv)
+{
+ char *p = NULL;
+ const char *config_filename = NULL;
+ const char *ssh_command = "ssh";
+ char *rune = NULL;
+ char *host;
+ int opt, daemonize = 1, debug = 0;
+
+ while ((opt = getopt(argc, argv, "hC:s:ed")) != -1) {
+ switch (opt) {
+ case 'h':
+ help("migrate");
+ break;
+ case 'C':
+ help("save");
+ config_filename = optarg;
+ break;
+ case 's':
+ ssh_command = optarg;
+ break;
+ case 'e':
+ daemonize = 0;
+ break;
+ case 'd':
+ debug = 1;
+ break;
+ default:
+ fprintf(stderr, "option not supported\n");
+ break;
+ }
+ }
+
+ if (argc-optind < 2 || argc-optind > 2) {
+ help("save");
+ exit(2);
+ }
+
+ p = argv[optind];
+ host = argv[optind + 1];
+
+ if (!ssh_command[0]) {
+ rune= host;
+ } else {
+ asprintf(&rune, "exec %s %s xl migrate-receive%s%s",
+ ssh_command, host,
+ daemonize ? "" : " -e",
+ debug ? " -d" : "");
+ }
+
+ migrate_domain(p, rune, config_filename);
+ exit(0);
+}
+
int main_pause(int argc, char **argv)
{
int opt;
@@ -1885,7 +2375,7 @@ int main_create(int argc, char **argv)
{
char *filename = NULL;
int debug = 0, daemonize = 1;
- int opt;
+ int opt, rc;
while ((opt = getopt(argc, argv, "hde")) != -1) {
switch (opt) {
@@ -1910,8 +2400,8 @@ int main_create(int argc, char **argv)
}
filename = argv[optind];
- create_domain(debug, daemonize, filename, NULL, 0);
- exit(0);
+ rc = create_domain(debug, daemonize, filename, NULL, 0, -1);
+ exit(-rc);
}
void button_press(char *p, char *b)
@@ -1923,7 +2413,7 @@ void button_press(char *p, char *b)
libxl_ctx_init(&ctx, LIBXL_VERSION);
libxl_ctx_set_log(&ctx, log_callback, NULL);
- if (domain_qualifier_to_domid(&ctx, p, &domid) < 0) {
+ if (domain_qualifier_to_domid(&ctx, p, &domid, 0) < 0) {
fprintf(stderr, "%s is an invalid domain identifier\n", p);
exit(2);
}
@@ -1977,6 +2467,15 @@ int main(int argc, char **argv)
srand(time(0));
+ if (libxl_ctx_init(&common_ctx, LIBXL_VERSION)) {
+ fprintf(stderr, "cannot init xl common context\n");
+ exit(-ERROR_FAIL);
+ }
+ if (libxl_ctx_set_log(&common_ctx, log_callback, NULL)) {
+ fprintf(stderr, "cannot set xl log callback\n");
+ exit(-ERROR_FAIL);
+ }
+
if (!strcmp(argv[1], "create")) {
main_create(argc - 1, argv + 1);
} else if (!strcmp(argv[1], "list")) {
@@ -1999,8 +2498,12 @@ int main(int argc, char **argv)
main_console(argc - 1, argv + 1);
} else if (!strcmp(argv[1], "save")) {
main_save(argc - 1, argv + 1);
+ } else if (!strcmp(argv[1], "migrate")) {
+ main_migrate(argc - 1, argv + 1);
} else if (!strcmp(argv[1], "restore")) {
main_restore(argc - 1, argv + 1);
+ } else if (!strcmp(argv[1], "migrate-receive")) {
+ main_migrate_receive(argc - 1, argv + 1);
} else if (!strcmp(argv[1], "cd-insert")) {
main_cd_insert(argc - 1, argv + 1);
} else if (!strcmp(argv[1], "cd-eject")) {
--
1.5.6.5
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 11/11] xl: Migration support
2010-03-25 19:04 ` [PATCH 11/11] xl: Migration support Ian Jackson
@ 2010-04-01 11:10 ` Stefano Stabellini
0 siblings, 0 replies; 29+ messages in thread
From: Stefano Stabellini @ 2010-04-01 11:10 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel@lists.xensource.com
On Thu, 25 Mar 2010, Ian Jackson wrote:
> diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
> index 7d35db1..53be99c 100644
> --- a/tools/libxl/xl.c
> +++ b/tools/libxl/xl.c
> @@ -17,6 +17,7 @@
> #include "libxl_osdeps.h"
>
> #include <stdio.h>
> +#include <assert.h>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
> @@ -42,9 +43,33 @@ int logfile = 2;
>
> static int suspend_old_xl_format = 0;
>
> +static struct libxl_ctx common_ctx;
> +static uint32_t common_domid;
> +static const char *common_domname;
> +static uint8_t *common_config_data;
> +static int common_config_len;
> +
> +static char *migration_domname;
> +
I really don't like the usage of global variables to pass parameters
between different functions.
In any case if you want to start using global variables you should do it
consistently across all the file: please write a separate patch that
does this clean up.
> static const char savefileheader_magic[32]=
> "Xen saved domain, xl format\n \0 \r";
>
> +static const char migrate_receiver_banner[]=
> + "xl migration receiver ready, send binary domain data.\n";
> +static const char migrate_receiver_ready[]=
> + "domain received, ready to unpause";
> +static const char migrate_permission_to_go[]=
> + "domain is yours, you are cleared to unpause";
> +static const char migrate_report[]=
> + "my copy unpause results are as follows";
> + /* followed by one byte:
> + * 0: everything went well, domain is running
> + * next thing is we all exit
> + * non-0: things went badly
> + * next thing should be a migrate_permission_to_go
> + * from target to source
> + */
> +
> typedef struct {
> char magic[32]; /* savefileheader_magic */
> /* All uint32_ts are in domain's byte order. */
> @@ -69,7 +94,7 @@ void log_callback(void *userdata, int loglevel, const char *file, int line, cons
> write(logfile, str, strlen(str));
> }
>
> -static int domain_qualifier_to_domid(struct libxl_ctx *ctx, char *p, uint32_t *domid)
> +static int domain_qualifier_to_domid(struct libxl_ctx *ctx, char *p, uint32_t *domid, const char **expected_name_r)
> {
> int i, alldigit;
>
> @@ -83,10 +108,12 @@ static int domain_qualifier_to_domid(struct libxl_ctx *ctx, char *p, uint32_t *d
>
> if (i > 0 && alldigit) {
> *domid = strtoul(p, NULL, 10);
> + if (expected_name_r) *expected_name_r = 0;
> return 0;
> } else {
> /* check here if it's a uuid and do proper conversion */
> }
> + if (expected_name_r) *expected_name_r = p;
> return libxl_name_to_domid(ctx, p, domid);
> }
>
can we avoid modifying domain_qualifier_to_domid and use libxl_domid_to_name instead?
> @@ -1091,6 +1126,25 @@ static void help(char *command)
> printf("-O Old (configless) xl save format.\n");
> printf("-p Do not unpause domain after restoring it.\n");
> printf("-e Do not wait in the background for the death of the domain.\n");
> + printf("-d Enable debug messages.\n");
> + } else if(!strcmp(command, "migrate")) {
> + printf("Usage: xl migrate [options] <Domain> <host>\n\n");
> + printf("Save a domain state to restore later.\n\n");
> + printf("Options:\n\n");
> + printf("-h Print this help.\n");
> + printf("-C <config> Send <config> instead of config file from creation.\n");
> + printf("-s <sshcommand> Use <sshcommand> instead of ssh. String will be passed to sh. If empty, run <host> instead of ssh <host> xl migrate-receive [-d -e]\n");
> + printf("-e Do not wait in the background (on <host>) for the death of the domain.\n");
> + } else if(!strcmp(command, "migrate-receive")) {
> + printf("Usage: xl migrate-receive - for internal use only");
> + } else if(!strcmp(command, "restore")) {
> + printf("Usage: xl restore [options] [<ConfigFile>] <CheckpointFile>\n\n");
> + printf("Restore a domain from a saved state.\n\n");
> + printf("Options:\n\n");
> + printf("-h Print this help.\n");
> + printf("-O Old (configless) xl save format.\n");
> + printf("-p Do not unpause domain after restoring it.\n");
> + printf("-e Do not wait in the background for the death of the domain.\n");
> } else if(!strcmp(command, "destroy")) {
> printf("Usage: xl destroy <Domain>\n\n");
> printf("Terminate a domain immediately.\n\n");
just remove the old "configless" save format
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 00/11] Migration support for "xl"
2010-03-25 19:04 [PATCH 00/11] Migration support for "xl" Ian Jackson
` (10 preceding siblings ...)
2010-03-25 19:04 ` [PATCH 11/11] xl: Migration support Ian Jackson
@ 2010-04-01 11:10 ` Stefano Stabellini
11 siblings, 0 replies; 29+ messages in thread
From: Stefano Stabellini @ 2010-04-01 11:10 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel@lists.xensource.com
On Thu, 25 Mar 2010, Ian Jackson wrote:
> This patch series provides support for "xl migrate".
>
> There are a number of bugfixes and extensions to libxl, followed by a
> new savefile format (which can contain the domain config file and is
> more extensible), followed by actual migration support.
>
> This should probably not go into 4.0 :-).
>
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
>
>
I have comments on patch 8, 9 and 11; all the rest:
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
^ permalink raw reply [flat|nested] 29+ messages in thread