* [PATCH 0 of 2] libxl: fix domain destruction with libxl hotplug script calling @ 2011-12-05 10:10 Roger Pau Monne 2011-12-05 10:10 ` [PATCH 1 of 2] libxl: set frontend status to 6 on domain destroy Roger Pau Monne 2011-12-05 10:10 ` [PATCH 2 of 2] libxl: remove force parameter from libxl_domain_destroy Roger Pau Monne 0 siblings, 2 replies; 6+ messages in thread From: Roger Pau Monne @ 2011-12-05 10:10 UTC (permalink / raw) To: xen-devel 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1 of 2] libxl: set frontend status to 6 on domain destroy 2011-12-05 10:10 [PATCH 0 of 2] libxl: fix domain destruction with libxl hotplug script calling Roger Pau Monne @ 2011-12-05 10:10 ` Roger Pau Monne 2011-12-05 10:10 ` [PATCH 2 of 2] libxl: remove force parameter from libxl_domain_destroy Roger Pau Monne 1 sibling, 0 replies; 6+ messages in thread From: Roger Pau Monne @ 2011-12-05 10:10 UTC (permalink / raw) To: xen-devel # 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); ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2 of 2] libxl: remove force parameter from libxl_domain_destroy 2011-12-05 10:10 [PATCH 0 of 2] libxl: fix domain destruction with libxl hotplug script calling Roger Pau Monne 2011-12-05 10:10 ` [PATCH 1 of 2] libxl: set frontend status to 6 on domain destroy Roger Pau Monne @ 2011-12-05 10:10 ` Roger Pau Monne 2011-12-05 10:24 ` Ian Campbell 1 sibling, 1 reply; 6+ messages in thread From: Roger Pau Monne @ 2011-12-05 10:10 UTC (permalink / raw) To: xen-devel # 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; } ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2 of 2] libxl: remove force parameter from libxl_domain_destroy 2011-12-05 10:10 ` [PATCH 2 of 2] libxl: remove force parameter from libxl_domain_destroy Roger Pau Monne @ 2011-12-05 10:24 ` Ian Campbell 2011-12-05 10:36 ` Ian Campbell 2011-12-05 10:48 ` Roger Pau Monné 0 siblings, 2 replies; 6+ messages in thread From: Ian Campbell @ 2011-12-05 10:24 UTC (permalink / raw) To: Roger Pau Monne; +Cc: xen-devel@lists.xensource.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? > > 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2 of 2] libxl: remove force parameter from libxl_domain_destroy 2011-12-05 10:24 ` Ian Campbell @ 2011-12-05 10:36 ` Ian Campbell 2011-12-05 10:48 ` Roger Pau Monné 1 sibling, 0 replies; 6+ messages in thread From: Ian Campbell @ 2011-12-05 10:36 UTC (permalink / raw) To: Roger Pau Monne; +Cc: xen-devel@lists.xensource.com 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2 of 2] libxl: remove force parameter from libxl_domain_destroy 2011-12-05 10:24 ` Ian Campbell 2011-12-05 10:36 ` Ian Campbell @ 2011-12-05 10:48 ` Roger Pau Monné 1 sibling, 0 replies; 6+ messages in thread From: Roger Pau Monné @ 2011-12-05 10:48 UTC (permalink / raw) To: Ian Campbell; +Cc: xen-devel@lists.xensource.com 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-12-05 10:48 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-12-05 10:10 [PATCH 0 of 2] libxl: fix domain destruction with libxl hotplug script calling Roger Pau Monne 2011-12-05 10:10 ` [PATCH 1 of 2] libxl: set frontend status to 6 on domain destroy Roger Pau Monne 2011-12-05 10:10 ` [PATCH 2 of 2] libxl: remove force parameter from libxl_domain_destroy Roger Pau Monne 2011-12-05 10:24 ` Ian Campbell 2011-12-05 10:36 ` Ian Campbell 2011-12-05 10:48 ` Roger Pau Monné
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.