* [PATCH] fix restore: xenstore entries left when restore failed
@ 2014-11-19 7:12 Chunyan Liu
2014-11-19 9:35 ` Ian Campbell
0 siblings, 1 reply; 2+ messages in thread
From: Chunyan Liu @ 2014-11-19 7:12 UTC (permalink / raw)
To: xen-devel; +Cc: ian.jackson, wei.liu2, ian.campbell, Chunyan Liu
While running libvirt-tck domain/102-broken-save-restore.t
test (save domain, corrupt saved file by truncate the last 512k,
then restore), found that restore domain failed, but domain
related xenstore entries still exist in xenstore.
Add a patch to clear xenstore entries in this case.
Signed-off-by: Chunyan Liu <cyliu@suse.com>
---
tools/libxl/libxl.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 52 insertions(+)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index de23fec..447840d 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1525,6 +1525,54 @@ static void devices_destroy_cb(libxl__egc *egc,
libxl__devices_remove_state *drs,
int rc);
+static void libxl_clear_xs_entry(libxl__gc *gc, uint32_t domid)
+{
+ const char *dom_path, *vm_path;
+ char *path;
+ unsigned int num_kinds, num_dev_xsentries;
+ char **kinds = NULL, **devs = NULL;
+ int i, j;
+
+ /* remove libxl path */
+ libxl__xs_rm_checked(gc, XBT_NULL, libxl__xs_libxl_path(gc, domid));
+
+ dom_path = libxl__xs_get_dompath(gc, domid);
+ if (!dom_path)
+ return;
+
+ /* remove backend entries */
+ path = GCSPRINTF("%s/device", dom_path);
+ kinds = libxl__xs_directory(gc, XBT_NULL, path, &num_kinds);
+ if (kinds && num_kinds) {
+ for (i = 0; i < num_kinds; i++) {
+ path = GCSPRINTF("%s/device/%s", dom_path, kinds[i]);
+ devs = libxl__xs_directory(gc, XBT_NULL, path, &num_dev_xsentries);
+ if (!devs)
+ continue;
+ for (j = 0; j < num_dev_xsentries; j++) {
+ path = GCSPRINTF("%s/device/%s/%s/backend",
+ dom_path, kinds[i], devs[j]);
+ path = libxl__xs_read(gc, XBT_NULL, path);
+ if (path)
+ libxl__xs_rm_checked(gc, XBT_NULL, path);
+ }
+ }
+ }
+
+ path = GCSPRINTF("%s/console/backend", dom_path);
+ path = libxl__xs_read(gc, XBT_NULL, path);
+ if (path)
+ libxl__xs_rm_checked(gc, XBT_NULL, path);
+
+ /* remove vm path */
+ vm_path = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/vm", dom_path));
+ if (vm_path)
+ libxl__xs_rm_checked(gc, XBT_NULL, vm_path);
+
+ /* remove dom path */
+ libxl__xs_rm_checked(gc, XBT_NULL, dom_path);
+}
+
void libxl__destroy_domid(libxl__egc *egc, libxl__destroy_domid_state *dis)
{
STATE_AO_GC(dis->ao);
@@ -1540,6 +1588,10 @@ void libxl__destroy_domid(libxl__egc *egc, libxl__destroy_domid_state *dis)
break;
case ERROR_INVAL:
LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "non-existant domain %d", domid);
+ /* domain may not started successfully but some xenstore entries
+ * might be created already in earlier stage. We need to clear
+ * those entries. */
+ libxl_clear_xs_entry(gc, domid);
default:
goto out;
}
--
1.8.4.5
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] fix restore: xenstore entries left when restore failed
2014-11-19 7:12 [PATCH] fix restore: xenstore entries left when restore failed Chunyan Liu
@ 2014-11-19 9:35 ` Ian Campbell
0 siblings, 0 replies; 2+ messages in thread
From: Ian Campbell @ 2014-11-19 9:35 UTC (permalink / raw)
To: Chunyan Liu, ian.jackson; +Cc: wei.liu2, xen-devel
On Wed, 2014-11-19 at 15:12 +0800, Chunyan Liu wrote:
> While running libvirt-tck domain/102-broken-save-restore.t
> test (save domain, corrupt saved file by truncate the last 512k,
> then restore), found that restore domain failed, but domain
> related xenstore entries still exist in xenstore.
>
> Add a patch to clear xenstore entries in this case.
>
> Signed-off-by: Chunyan Liu <cyliu@suse.com>
> ---
> tools/libxl/libxl.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 52 insertions(+)
>
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index de23fec..447840d 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -1525,6 +1525,54 @@ static void devices_destroy_cb(libxl__egc *egc,
> libxl__devices_remove_state *drs,
> int rc);
>
> +static void libxl_clear_xs_entry(libxl__gc *gc, uint32_t domid)
This seems to duplicate a bunch of stuff already done by
libxl__destroy_domid and libxl__devices_destroy etc. I think rather than
duplicating this libxl__destroy_domid should be made to Do The Right
Thing by trying to clean up any remnants of the domid even if it doesn't
currently exist.
Alternatively perhaps the real bug is in the error path of the restore
functionality, which isn't calling the correct unwind path. Ian, any
thoughts?
Ian.
> + const char *dom_path, *vm_path;
> + char *path;
> + unsigned int num_kinds, num_dev_xsentries;
> + char **kinds = NULL, **devs = NULL;
> + int i, j;
> +
> + /* remove libxl path */
> + libxl__xs_rm_checked(gc, XBT_NULL, libxl__xs_libxl_path(gc, domid));
> +
> + dom_path = libxl__xs_get_dompath(gc, domid);
> + if (!dom_path)
> + return;
> +
> + /* remove backend entries */
> + path = GCSPRINTF("%s/device", dom_path);
> + kinds = libxl__xs_directory(gc, XBT_NULL, path, &num_kinds);
> + if (kinds && num_kinds) {
> + for (i = 0; i < num_kinds; i++) {
> + path = GCSPRINTF("%s/device/%s", dom_path, kinds[i]);
> + devs = libxl__xs_directory(gc, XBT_NULL, path, &num_dev_xsentries);
> + if (!devs)
> + continue;
> + for (j = 0; j < num_dev_xsentries; j++) {
> + path = GCSPRINTF("%s/device/%s/%s/backend",
> + dom_path, kinds[i], devs[j]);
> + path = libxl__xs_read(gc, XBT_NULL, path);
> + if (path)
> + libxl__xs_rm_checked(gc, XBT_NULL, path);
> + }
> + }
> + }
> +
> + path = GCSPRINTF("%s/console/backend", dom_path);
> + path = libxl__xs_read(gc, XBT_NULL, path);
> + if (path)
> + libxl__xs_rm_checked(gc, XBT_NULL, path);
> +
> + /* remove vm path */
> + vm_path = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/vm", dom_path));
> + if (vm_path)
> + libxl__xs_rm_checked(gc, XBT_NULL, vm_path);
> +
> + /* remove dom path */
> + libxl__xs_rm_checked(gc, XBT_NULL, dom_path);
> +}
> +
> void libxl__destroy_domid(libxl__egc *egc, libxl__destroy_domid_state *dis)
> {
> STATE_AO_GC(dis->ao);
> @@ -1540,6 +1588,10 @@ void libxl__destroy_domid(libxl__egc *egc, libxl__destroy_domid_state *dis)
> break;
> case ERROR_INVAL:
> LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "non-existant domain %d", domid);
> + /* domain may not started successfully but some xenstore entries
> + * might be created already in earlier stage. We need to clear
> + * those entries. */
> + libxl_clear_xs_entry(gc, domid);
> default:
> goto out;
> }
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-11-19 9:35 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-19 7:12 [PATCH] fix restore: xenstore entries left when restore failed Chunyan Liu
2014-11-19 9:35 ` Ian Campbell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.