Roger Pau Monne
2011-Dec-05 10:10 UTC
[PATCH 0 of 2] libxl: fix domain destruction with libxl hotplug script calling
When calling hotplug scripts from libxl we have to make sure the device is disconnected before attepmting to execute hotplug scripts. This patch series sets frontend status to 6 when a domain is destroyed, so the devices are disconnected and then execute hotplug scripts. Also the syntax of the libxl_domain_destroy has been changed to always force the destruction. This series should be applied after "libxl: add support for hotplug script calling from libxl". Please review, Roger.
Roger Pau Monne
2011-Dec-05 10:10 UTC
[PATCH 1 of 2] libxl: set frontend status to 6 on domain destroy
# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1323079583 -3600
# Node ID bc90cfd8dd220d69d09cf94a3d39ff3cef76d021
# Parent 274fa4aea2a30fb82228513f969d7cb807813bb8
libxl: set frontend status to 6 on domain destroy
Set frontend status to 6 on domain destruction and wait for devices to
be disconnected before executing hotplug scripts.
Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>
diff -r 274fa4aea2a3 -r bc90cfd8dd22 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c Thu Dec 01 16:01:31 2011 +0100
+++ b/tools/libxl/libxl_device.c Mon Dec 05 11:06:23 2011 +0100
@@ -513,10 +513,7 @@ int libxl__device_destroy(libxl__gc *gc,
char *be_path = libxl__device_backend_path(gc, dev);
char *fe_path = libxl__device_frontend_path(gc, dev);
- /*
- * Run hotplug scripts, which will probably not be able to
- * execute successfully since the device may still be plugged
- */
+ libxl__xs_write(gc, XBT_NULL, libxl__sprintf(gc, "%s/%s",
fe_path, "state"), "6");
libxl__device_execute_hotplug(gc, dev, DISCONNECT);
xs_rm(ctx->xsh, XBT_NULL, be_path);
Roger Pau Monne
2011-Dec-05 10:10 UTC
[PATCH 2 of 2] libxl: remove force parameter from libxl_domain_destroy
# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# Date 1323079605 -3600
# Node ID c0d51df66b829995c4eb3902b5b9914c710a6c01
# Parent bc90cfd8dd220d69d09cf94a3d39ff3cef76d021
libxl: remove force parameter from libxl_domain_destroy
Since a destroy is considered a forced shutdown, there''s no point in
passing a force parameter. All the occurences of this function have
been replaced with the proper syntax.
Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu>
diff -r bc90cfd8dd22 -r c0d51df66b82 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c Mon Dec 05 11:06:23 2011 +0100
+++ b/tools/libxl/libxl.c Mon Dec 05 11:06:45 2011 +0100
@@ -718,7 +718,7 @@ int libxl_event_get_disk_eject_info(libx
return 1;
}
-int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid, int force)
+int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid)
{
libxl__gc gc = LIBXL_INIT_GC(ctx);
libxl_dominfo dominfo;
@@ -767,7 +767,7 @@ int libxl_domain_destroy(libxl_ctx *ctx,
libxl__qmp_cleanup(&gc, domid);
}
- if (libxl__devices_destroy(&gc, domid, force) < 0)
+ if (libxl__devices_destroy(&gc, domid, 1) < 0)
LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "libxl__devices_destroy failed
for %d", domid);
vm_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc,
"%s/vm", dom_path));
diff -r bc90cfd8dd22 -r c0d51df66b82 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h Mon Dec 05 11:06:23 2011 +0100
+++ b/tools/libxl/libxl.h Mon Dec 05 11:06:45 2011 +0100
@@ -269,7 +269,7 @@ int libxl_domain_suspend(libxl_ctx *ctx,
uint32_t domid, int fd);
int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid);
int libxl_domain_shutdown(libxl_ctx *ctx, uint32_t domid, int req);
-int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid, int force);
+int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid);
int libxl_domain_preserve(libxl_ctx *ctx, uint32_t domid,
libxl_domain_create_info *info, const char *name_suffix, libxl_uuid new_uuid);
/* get max. number of cpus supported by hypervisor */
diff -r bc90cfd8dd22 -r c0d51df66b82 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c Mon Dec 05 11:06:23 2011 +0100
+++ b/tools/libxl/libxl_create.c Mon Dec 05 11:06:45 2011 +0100
@@ -646,7 +646,7 @@ static int do_domain_create(libxl__gc *g
error_out:
if (domid)
- libxl_domain_destroy(ctx, domid, 0);
+ libxl_domain_destroy(ctx, domid);
return ret;
}
diff -r bc90cfd8dd22 -r c0d51df66b82 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c Mon Dec 05 11:06:23 2011 +0100
+++ b/tools/libxl/libxl_dm.c Mon Dec 05 11:06:45 2011 +0100
@@ -917,7 +917,7 @@ int libxl__destroy_device_model(libxl__g
goto out;
}
LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Device model is a stubdom,
domid=%d", stubdomid);
- ret = libxl_domain_destroy(ctx, stubdomid, 0);
+ ret = libxl_domain_destroy(ctx, stubdomid);
if (ret)
goto out;
} else {
diff -r bc90cfd8dd22 -r c0d51df66b82 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Mon Dec 05 11:06:23 2011 +0100
+++ b/tools/libxl/xl_cmdimpl.c Mon Dec 05 11:06:45 2011 +0100
@@ -1200,7 +1200,7 @@ static int handle_domain_death(libxl_ctx
/* fall-through */
case LIBXL_ACTION_ON_SHUTDOWN_DESTROY:
LOG("Domain %d needs to be cleaned up: destroying the
domain", domid);
- libxl_domain_destroy(ctx, domid, 0);
+ libxl_domain_destroy(ctx, domid);
break;
case LIBXL_ACTION_ON_SHUTDOWN_COREDUMP_DESTROY:
@@ -1698,7 +1698,7 @@ start:
error_out:
release_lock();
if (libxl_domid_valid_guest(domid))
- libxl_domain_destroy(ctx, domid, 0);
+ libxl_domain_destroy(ctx, domid);
out:
if (logfile != 2)
@@ -2168,7 +2168,7 @@ static void destroy_domain(const char *p
fprintf(stderr, "Cannot destroy privileged domain 0.\n\n");
exit(-1);
}
- rc = libxl_domain_destroy(ctx, domid, 0);
+ rc = libxl_domain_destroy(ctx, domid);
if (rc) { fprintf(stderr,"destroy failed (rc=%d)\n",rc);
exit(-1); }
}
@@ -2414,7 +2414,7 @@ static int save_domain(const char *p, co
if (checkpoint)
libxl_domain_unpause(ctx, domid);
else
- libxl_domain_destroy(ctx, domid, 0);
+ libxl_domain_destroy(ctx, domid);
exit(0);
}
@@ -2647,7 +2647,7 @@ static void migrate_domain(const char *d
}
fprintf(stderr, "migration sender: Target reports successful
startup.\n");
- libxl_domain_destroy(ctx, domid, 1); /* bang! */
+ libxl_domain_destroy(ctx, domid); /* bang! */
fprintf(stderr, "Migration successful.\n");
exit(0);
@@ -2764,7 +2764,7 @@ static void migrate_receive(int debug, i
if (rc) {
fprintf(stderr, "migration target: Failure, destroying our
copy.\n");
- rc2 = libxl_domain_destroy(ctx, domid, 1);
+ rc2 = libxl_domain_destroy(ctx, domid);
if (rc2) {
fprintf(stderr, "migration target: Failed to destroy our
copy"
" (code %d).\n", rc2);
diff -r bc90cfd8dd22 -r c0d51df66b82 tools/python/xen/lowlevel/xl/xl.c
--- a/tools/python/xen/lowlevel/xl/xl.c Mon Dec 05 11:06:23 2011 +0100
+++ b/tools/python/xen/lowlevel/xl/xl.c Mon Dec 05 11:06:45 2011 +0100
@@ -437,10 +437,10 @@ static PyObject *pyxl_domain_shutdown(Xl
static PyObject *pyxl_domain_destroy(XlObject *self, PyObject *args)
{
- int domid, force = 1;
- if ( !PyArg_ParseTuple(args, "i|i", &domid, &force) )
+ int domid;
+ if ( !PyArg_ParseTuple(args, "i", &domid) )
return NULL;
- if ( libxl_domain_destroy(self->ctx, domid, force) ) {
+ if ( libxl_domain_destroy(self->ctx, domid) ) {
PyErr_SetString(xl_error_obj, "cannot destroy domain");
return NULL;
}
Ian Campbell
2011-Dec-05 10:24 UTC
Re: [PATCH 2 of 2] libxl: remove force parameter from libxl_domain_destroy
Did patch 1/2 get stuck somewhere? I''ve not seen it yet. On Mon, 2011-12-05 at 10:10 +0000, Roger Pau Monne wrote:> # HG changeset patch > # User Roger Pau Monne <roger.pau@entel.upc.edu> > # Date 1323079605 -3600 > # Node ID c0d51df66b829995c4eb3902b5b9914c710a6c01 > # Parent bc90cfd8dd220d69d09cf94a3d39ff3cef76d021 > libxl: remove force parameter from libxl_domain_destroy > > Since a destroy is considered a forced shutdown, there''s no point in > passing a force parameter. All the occurences of this function have > been replaced with the proper syntax.I''m a little concerned with the change in libxl__destroy_device_model, mostly because I don'';t know what the expected semantics of a stub dom shutdown are. Perhaps it is fine to shoot such a domain in the head without previously giving an opportunity to shutdown?> > Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu> > > diff -r bc90cfd8dd22 -r c0d51df66b82 tools/libxl/libxl.c > --- a/tools/libxl/libxl.c Mon Dec 05 11:06:23 2011 +0100 > +++ b/tools/libxl/libxl.c Mon Dec 05 11:06:45 2011 +0100 > @@ -718,7 +718,7 @@ int libxl_event_get_disk_eject_info(libx > return 1; > } > > -int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid, int force) > +int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid) > { > libxl__gc gc = LIBXL_INIT_GC(ctx); > libxl_dominfo dominfo; > @@ -767,7 +767,7 @@ int libxl_domain_destroy(libxl_ctx *ctx, > > libxl__qmp_cleanup(&gc, domid); > } > - if (libxl__devices_destroy(&gc, domid, force) < 0) > + if (libxl__devices_destroy(&gc, domid, 1) < 0)If I''m not missing something this seems to be the only caller of libxl__device_destroy. We could keep pushing this change down and remove the force param here too which in turns removes a bunch of code from libxl__devices_destroy and makes it behave like its name suggests.> LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "libxl__devices_destroy failed for %d", domid); > > vm_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, "%s/vm", dom_path)); > diff -r bc90cfd8dd22 -r c0d51df66b82 tools/libxl/libxl.h > --- a/tools/libxl/libxl.h Mon Dec 05 11:06:23 2011 +0100 > +++ b/tools/libxl/libxl.h Mon Dec 05 11:06:45 2011 +0100 > @@ -269,7 +269,7 @@ int libxl_domain_suspend(libxl_ctx *ctx, > uint32_t domid, int fd); > int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid); > int libxl_domain_shutdown(libxl_ctx *ctx, uint32_t domid, int req); > -int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid, int force); > +int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid); > int libxl_domain_preserve(libxl_ctx *ctx, uint32_t domid, libxl_domain_create_info *info, const char *name_suffix, libxl_uuid new_uuid); > > /* get max. number of cpus supported by hypervisor */ > diff -r bc90cfd8dd22 -r c0d51df66b82 tools/libxl/libxl_create.c > --- a/tools/libxl/libxl_create.c Mon Dec 05 11:06:23 2011 +0100 > +++ b/tools/libxl/libxl_create.c Mon Dec 05 11:06:45 2011 +0100 > @@ -646,7 +646,7 @@ static int do_domain_create(libxl__gc *g > > error_out: > if (domid) > - libxl_domain_destroy(ctx, domid, 0); > + libxl_domain_destroy(ctx, domid); > > return ret; > } > diff -r bc90cfd8dd22 -r c0d51df66b82 tools/libxl/libxl_dm.c > --- a/tools/libxl/libxl_dm.c Mon Dec 05 11:06:23 2011 +0100 > +++ b/tools/libxl/libxl_dm.c Mon Dec 05 11:06:45 2011 +0100 > @@ -917,7 +917,7 @@ int libxl__destroy_device_model(libxl__g > goto out; > } > LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Device model is a stubdom, domid=%d", stubdomid); > - ret = libxl_domain_destroy(ctx, stubdomid, 0); > + ret = libxl_domain_destroy(ctx, stubdomid); > if (ret) > goto out; > } else { > diff -r bc90cfd8dd22 -r c0d51df66b82 tools/libxl/xl_cmdimpl.c > --- a/tools/libxl/xl_cmdimpl.c Mon Dec 05 11:06:23 2011 +0100 > +++ b/tools/libxl/xl_cmdimpl.c Mon Dec 05 11:06:45 2011 +0100 > @@ -1200,7 +1200,7 @@ static int handle_domain_death(libxl_ctx > /* fall-through */ > case LIBXL_ACTION_ON_SHUTDOWN_DESTROY: > LOG("Domain %d needs to be cleaned up: destroying the domain", domid); > - libxl_domain_destroy(ctx, domid, 0); > + libxl_domain_destroy(ctx, domid); > break; > > case LIBXL_ACTION_ON_SHUTDOWN_COREDUMP_DESTROY: > @@ -1698,7 +1698,7 @@ start: > error_out: > release_lock(); > if (libxl_domid_valid_guest(domid)) > - libxl_domain_destroy(ctx, domid, 0); > + libxl_domain_destroy(ctx, domid); > > out: > if (logfile != 2) > @@ -2168,7 +2168,7 @@ static void destroy_domain(const char *p > fprintf(stderr, "Cannot destroy privileged domain 0.\n\n"); > exit(-1); > } > - rc = libxl_domain_destroy(ctx, domid, 0); > + rc = libxl_domain_destroy(ctx, domid); > if (rc) { fprintf(stderr,"destroy failed (rc=%d)\n",rc); exit(-1); } > } > > @@ -2414,7 +2414,7 @@ static int save_domain(const char *p, co > if (checkpoint) > libxl_domain_unpause(ctx, domid); > else > - libxl_domain_destroy(ctx, domid, 0); > + libxl_domain_destroy(ctx, domid); > > exit(0); > } > @@ -2647,7 +2647,7 @@ static void migrate_domain(const char *d > } > > fprintf(stderr, "migration sender: Target reports successful startup.\n"); > - libxl_domain_destroy(ctx, domid, 1); /* bang! */ > + libxl_domain_destroy(ctx, domid); /* bang! */ > fprintf(stderr, "Migration successful.\n"); > exit(0); > > @@ -2764,7 +2764,7 @@ static void migrate_receive(int debug, i > if (rc) { > fprintf(stderr, "migration target: Failure, destroying our copy.\n"); > > - rc2 = libxl_domain_destroy(ctx, domid, 1); > + rc2 = libxl_domain_destroy(ctx, domid); > if (rc2) { > fprintf(stderr, "migration target: Failed to destroy our copy" > " (code %d).\n", rc2); > diff -r bc90cfd8dd22 -r c0d51df66b82 tools/python/xen/lowlevel/xl/xl.c > --- a/tools/python/xen/lowlevel/xl/xl.c Mon Dec 05 11:06:23 2011 +0100 > +++ b/tools/python/xen/lowlevel/xl/xl.c Mon Dec 05 11:06:45 2011 +0100 > @@ -437,10 +437,10 @@ static PyObject *pyxl_domain_shutdown(Xl > > static PyObject *pyxl_domain_destroy(XlObject *self, PyObject *args) > { > - int domid, force = 1; > - if ( !PyArg_ParseTuple(args, "i|i", &domid, &force) ) > + int domid; > + if ( !PyArg_ParseTuple(args, "i", &domid) ) > return NULL; > - if ( libxl_domain_destroy(self->ctx, domid, force) ) { > + if ( libxl_domain_destroy(self->ctx, domid) ) { > PyErr_SetString(xl_error_obj, "cannot destroy domain"); > return NULL; > } > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel
Ian Campbell
2011-Dec-05 10:36 UTC
Re: [PATCH 2 of 2] libxl: remove force parameter from libxl_domain_destroy
On Mon, 2011-12-05 at 10:24 +0000, Ian Campbell wrote:> Did patch 1/2 get stuck somewhere? I''ve not seen it yet.NM. Got it now, thanks.
Roger Pau Monné
2011-Dec-05 10:48 UTC
Re: [PATCH 2 of 2] libxl: remove force parameter from libxl_domain_destroy
2011/12/5 Ian Campbell <Ian.Campbell@citrix.com>:> Did patch 1/2 get stuck somewhere? I've not seen it yet. > > On Mon, 2011-12-05 at 10:10 +0000, Roger Pau Monne wrote: >> # HG changeset patch >> # User Roger Pau Monne <roger.pau@entel.upc.edu> >> # Date 1323079605 -3600 >> # Node ID c0d51df66b829995c4eb3902b5b9914c710a6c01 >> # Parent bc90cfd8dd220d69d09cf94a3d39ff3cef76d021 >> libxl: remove force parameter from libxl_domain_destroy >> >> Since a destroy is considered a forced shutdown, there's no point in >> passing a force parameter. All the occurences of this function have >> been replaced with the proper syntax. > > I'm a little concerned with the change in libxl__destroy_device_model, > mostly because I don';t know what the expected semantics of a stub dom > shutdown are. Perhaps it is fine to shoot such a domain in the head > without previously giving an opportunity to shutdown?I'm sorry, but I don't know about stubdoms, they are not working under NetBSD *yet*, I will probably get with this once I finish porting libxl (or a userspace blktap implementation, that's also quite outstanding).>> >> Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu> >> >> diff -r bc90cfd8dd22 -r c0d51df66b82 tools/libxl/libxl.c >> --- a/tools/libxl/libxl.c Mon Dec 05 11:06:23 2011 +0100 >> +++ b/tools/libxl/libxl.c Mon Dec 05 11:06:45 2011 +0100 >> @@ -718,7 +718,7 @@ int libxl_event_get_disk_eject_info(libx >> return 1; >> } >> >> -int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid, int force) >> +int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid) >> { >> libxl__gc gc = LIBXL_INIT_GC(ctx); >> libxl_dominfo dominfo; >> @@ -767,7 +767,7 @@ int libxl_domain_destroy(libxl_ctx *ctx, >> >> libxl__qmp_cleanup(&gc, domid); >> } >> - if (libxl__devices_destroy(&gc, domid, force) < 0) >> + if (libxl__devices_destroy(&gc, domid, 1) < 0) > > If I'm not missing something this seems to be the only caller of > libxl__device_destroy. We could keep pushing this change down and remove > the force param here too which in turns removes a bunch of code from > libxl__devices_destroy and makes it behave like its name suggests.I've already created another patch that changes the semantics of libxl__device_destroy and removes the force flag and all the unnecessary code. If no one has an objection about this change I will resend the series later with this patch. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel