[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [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@xxxxxxxxxxxxx>
> # 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@xxxxxxxxxxxxx>
> 
> 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@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.