Re: [Xen-devel] [PATCH v2] libxl: allow libxl_domain_suspend to simply suspend a domain, without saving it

2018-04-04 Thread Ian Jackson 
Marek Marczykowski-Górecki writes ("Re: [PATCH v2] libxl: allow 
libxl_domain_suspend to simply suspend a domain, without saving it"):
> Fine with me too.
> 
> Should I add also #define HAVE_DOMAIN_SUSPEND_ONLY or such?

Yes.

Thanks, and sorry to quibble about these kind of details.

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2] libxl: allow libxl_domain_suspend to simply suspend a domain, without saving it

2018-04-04 Thread Marek Marczykowski-Górecki 
On Wed, Apr 04, 2018 at 04:45:51PM +0100, Ian Jackson wrote:
> Marek Marczykowski-Górecki writes ("Re: [PATCH v2] libxl: allow 
> libxl_domain_suspend to simply suspend a domain, without saving it"):
> > On Wed, Mar 28, 2018 at 12:20:25PM +0100, Ian Jackson wrote:
> > > I agree that libxl_domain_suspend is an unfortunate name, but can't we
> > > come up with an alternative new name ?  It does seem odd to bundle
> > > this into _save.
> > 
> > libxl_domain_just_suspend ?
> 
> How about libxl_domain_suspend_only ?

Fine with me too.

Should I add also #define HAVE_DOMAIN_SUSPEND_ONLY or such?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2] libxl: allow libxl_domain_suspend to simply suspend a domain, without saving it

2018-04-04 Thread Ian Jackson 
Marek Marczykowski-Górecki writes ("Re: [PATCH v2] libxl: allow 
libxl_domain_suspend to simply suspend a domain, without saving it"):
> On Wed, Mar 28, 2018 at 12:20:25PM +0100, Ian Jackson wrote:
> > I agree that libxl_domain_suspend is an unfortunate name, but can't we
> > come up with an alternative new name ?  It does seem odd to bundle
> > this into _save.
> 
> libxl_domain_just_suspend ?

How about libxl_domain_suspend_only ?

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2] libxl: allow libxl_domain_suspend to simply suspend a domain, without saving it

2018-04-04 Thread Marek Marczykowski-Górecki 
On Wed, Mar 28, 2018 at 12:20:25PM +0100, Ian Jackson wrote:
> Wei Liu writes ("Re: [PATCH v2] libxl: allow libxl_domain_suspend to simply 
> suspend a domain, without saving it"):
> > On Wed, Mar 14, 2018 at 03:36:08PM +0100, Marek Marczykowski-Górecki wrote:
> > > When LIBXL_SUSPEND_NO_SAVE flag is set, no savefile will be written, but
> > > the domain will still be suspended (but not destroyed). The main reason
> > > for this functionality is to suspend the host while some domains are
> > > running, potentially holding PCI devices. This will give a chance to a
> > > driver in such a domain to properly suspend the device.
> > > 
> ...
> > > Signed-off-by: Marek Marczykowski-Górecki 
> > > 
> > > Signed-off-by: Marcus of Wetware Labs 
> > 
> > The code and idea look fine.
> > 
> > I would like to give Ian a chance to voice his opinion (he's currently
> > away).
> 
> The API does seem a bit odd.  The intent is then that the domain will
> be un-suspended afterwards ?  This doesn't seem to be documented
> AFAICT in your patch.

Yes, there is already libxl_domain_resume for that.

> I don't think I agree with this part of the reaoning:
> 
> > > It would be better to have a separate function for this, but in fact it
> > > should be named libxl_domain_suspend, then the current one renamed to
> > > libxl_domain_save. Since that would break API compatibility, keep it in
> > > the same function.
> 
> I agree that libxl_domain_suspend is an unfortunate name, but can't we
> come up with an alternative new name ?  It does seem odd to bundle
> this into _save.

libxl_domain_just_suspend ?
It isn't bundling it into _save. There is no _save function - that's the
problem.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2] libxl: allow libxl_domain_suspend to simply suspend a domain, without saving it

2018-03-28 Thread Ian Jackson 
Wei Liu writes ("Re: [PATCH v2] libxl: allow libxl_domain_suspend to simply 
suspend a domain, without saving it"):
> On Wed, Mar 14, 2018 at 03:36:08PM +0100, Marek Marczykowski-Górecki wrote:
> > When LIBXL_SUSPEND_NO_SAVE flag is set, no savefile will be written, but
> > the domain will still be suspended (but not destroyed). The main reason
> > for this functionality is to suspend the host while some domains are
> > running, potentially holding PCI devices. This will give a chance to a
> > driver in such a domain to properly suspend the device.
> > 
...
> > Signed-off-by: Marek Marczykowski-Górecki 
> > Signed-off-by: Marcus of Wetware Labs 
> 
> The code and idea look fine.
> 
> I would like to give Ian a chance to voice his opinion (he's currently
> away).

The API does seem a bit odd.  The intent is then that the domain will
be un-suspended afterwards ?  This doesn't seem to be documented
AFAICT in your patch.

I don't think I agree with this part of the reaoning:

> > It would be better to have a separate function for this, but in fact it
> > should be named libxl_domain_suspend, then the current one renamed to
> > libxl_domain_save. Since that would break API compatibility, keep it in
> > the same function.

I agree that libxl_domain_suspend is an unfortunate name, but can't we
come up with an alternative new name ?  It does seem odd to bundle
this into _save.

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2] libxl: allow libxl_domain_suspend to simply suspend a domain, without saving it

2018-03-21 Thread Marek Marczykowski-Górecki 
On Wed, Mar 14, 2018 at 03:36:08PM +0100, Marek Marczykowski-Górecki wrote:
> When LIBXL_SUSPEND_NO_SAVE flag is set, no savefile will be written, but
> the domain will still be suspended (but not destroyed). The main reason
> for this functionality is to suspend the host while some domains are
> running, potentially holding PCI devices. This will give a chance to a
> driver in such a domain to properly suspend the device.
> 
> It would be better to have a separate function for this, but in fact it
> should be named libxl_domain_suspend, then the current one renamed to
> libxl_domain_save. Since that would break API compatibility, keep it in
> the same function.
> 
> Signed-off-by: Marek Marczykowski-Górecki 
> Signed-off-by: Marcus of Wetware Labs 

Bump?

Any chances to get it into 4.11?

> ---
> Changes in v2:
>  - drop double initialization of dsps fields (libxl__domain_suspend_init
>is called)
>  - use LIBXL_SUSPEND_NO_SAVE flag instead of fd=-1
> ---
>  tools/libxl/libxl.h|  5 +
>  tools/libxl/libxl_domain.c | 52 
> +-
>  2 files changed, 42 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index eca0ea2c50..636db77c2b 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -1469,6 +1469,11 @@ int libxl_domain_suspend(libxl_ctx *ctx, uint32_t 
> domid, int fd,
>   LIBXL_EXTERNAL_CALLERS_ONLY;
>  #define LIBXL_SUSPEND_DEBUG 1
>  #define LIBXL_SUSPEND_LIVE 2
> +/*
> + * Just transition the domain into suspended state, do not save its state to
> + * disk and do not destroy it. fd parameter is ignored.
> + */
> +#define LIBXL_SUSPEND_NO_SAVE 4
>  
>  /* @param suspend_cancel [from xenctrl.h:xc_domain_resume( @param fast )]
>   *   If this parameter is true, use co-operative resume. The guest
> diff --git a/tools/libxl/libxl_domain.c b/tools/libxl/libxl_domain.c
> index 13b1c73d40..0e9e245ce3 100644
> --- a/tools/libxl/libxl_domain.c
> +++ b/tools/libxl/libxl_domain.c
> @@ -486,6 +486,13 @@ static void domain_suspend_cb(libxl__egc *egc,
>  
>  }
>  
> +static void domain_suspend_empty_cb(libxl__egc *egc,
> +  libxl__domain_suspend_state *dss, int rc)
> +{
> +STATE_AO_GC(dss->ao);
> +libxl__ao_complete(egc,ao,rc);
> +}
> +
>  int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd, int flags,
>   const libxl_asyncop_how *ao_how)
>  {
> @@ -498,25 +505,40 @@ int libxl_domain_suspend(libxl_ctx *ctx, uint32_t 
> domid, int fd, int flags,
>  goto out_err;
>  }
>  
> -libxl__domain_save_state *dss;
> -GCNEW(dss);
> +if (!(flags & LIBXL_SUSPEND_NO_SAVE)) {
> +libxl__domain_save_state *dss;
>  
> -dss->ao = ao;
> -dss->callback = domain_suspend_cb;
> +GCNEW(dss);
>  
> -dss->domid = domid;
> -dss->fd = fd;
> -dss->type = type;
> -dss->live = flags & LIBXL_SUSPEND_LIVE;
> -dss->debug = flags & LIBXL_SUSPEND_DEBUG;
> -dss->checkpointed_stream = LIBXL_CHECKPOINTED_STREAM_NONE;
> +dss->ao = ao;
> +dss->callback = domain_suspend_cb;
> +
> +dss->domid = domid;
> +dss->fd = fd;
> +dss->type = type;
> +dss->live = flags & LIBXL_SUSPEND_LIVE;
> +dss->debug = flags & LIBXL_SUSPEND_DEBUG;
> +dss->checkpointed_stream = LIBXL_CHECKPOINTED_STREAM_NONE;
> +
> +rc = libxl__fd_flags_modify_save(gc, dss->fd,
> + ~(O_NONBLOCK|O_NDELAY), 0,
> + >fdfl);
> +if (rc < 0) goto out_err;
>  
> -rc = libxl__fd_flags_modify_save(gc, dss->fd,
> - ~(O_NONBLOCK|O_NDELAY), 0,
> - >fdfl);
> -if (rc < 0) goto out_err;
> +libxl__domain_save(egc, dss);
> +} else {
> +libxl__domain_suspend_state *dsps;
> +
> +GCNEW(dsps);
> +dsps->ao = ao;
> +dsps->domid = domid;
> +dsps->type = type;
> +rc = libxl__domain_suspend_init(egc, dsps, type);
> +if (rc < 0) goto out_err;
> +dsps->callback_common_done = domain_suspend_empty_cb;
> +libxl__domain_suspend(egc, dsps);
> +}
>  
> -libxl__domain_save(egc, dss);
>  return AO_INPROGRESS;
>  
>   out_err:

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v2] libxl: allow libxl_domain_suspend to simply suspend a domain, without saving it

2018-03-14 Thread Marek Marczykowski-Górecki 
When LIBXL_SUSPEND_NO_SAVE flag is set, no savefile will be written, but
the domain will still be suspended (but not destroyed). The main reason
for this functionality is to suspend the host while some domains are
running, potentially holding PCI devices. This will give a chance to a
driver in such a domain to properly suspend the device.

It would be better to have a separate function for this, but in fact it
should be named libxl_domain_suspend, then the current one renamed to
libxl_domain_save. Since that would break API compatibility, keep it in
the same function.

Signed-off-by: Marek Marczykowski-Górecki 
Signed-off-by: Marcus of Wetware Labs 

---
Changes in v2:
 - drop double initialization of dsps fields (libxl__domain_suspend_init
   is called)
 - use LIBXL_SUSPEND_NO_SAVE flag instead of fd=-1
---
 tools/libxl/libxl.h|  5 +
 tools/libxl/libxl_domain.c | 52 +-
 2 files changed, 42 insertions(+), 15 deletions(-)

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index eca0ea2c50..636db77c2b 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1469,6 +1469,11 @@ int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, 
int fd,
  LIBXL_EXTERNAL_CALLERS_ONLY;
 #define LIBXL_SUSPEND_DEBUG 1
 #define LIBXL_SUSPEND_LIVE 2
+/*
+ * Just transition the domain into suspended state, do not save its state to
+ * disk and do not destroy it. fd parameter is ignored.
+ */
+#define LIBXL_SUSPEND_NO_SAVE 4
 
 /* @param suspend_cancel [from xenctrl.h:xc_domain_resume( @param fast )]
  *   If this parameter is true, use co-operative resume. The guest
diff --git a/tools/libxl/libxl_domain.c b/tools/libxl/libxl_domain.c
index 13b1c73d40..0e9e245ce3 100644
--- a/tools/libxl/libxl_domain.c
+++ b/tools/libxl/libxl_domain.c
@@ -486,6 +486,13 @@ static void domain_suspend_cb(libxl__egc *egc,
 
 }
 
+static void domain_suspend_empty_cb(libxl__egc *egc,
+  libxl__domain_suspend_state *dss, int rc)
+{
+STATE_AO_GC(dss->ao);
+libxl__ao_complete(egc,ao,rc);
+}
+
 int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd, int flags,
  const libxl_asyncop_how *ao_how)
 {
@@ -498,25 +505,40 @@ int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, 
int fd, int flags,
 goto out_err;
 }
 
-libxl__domain_save_state *dss;
-GCNEW(dss);
+if (!(flags & LIBXL_SUSPEND_NO_SAVE)) {
+libxl__domain_save_state *dss;
 
-dss->ao = ao;
-dss->callback = domain_suspend_cb;
+GCNEW(dss);
 
-dss->domid = domid;
-dss->fd = fd;
-dss->type = type;
-dss->live = flags & LIBXL_SUSPEND_LIVE;
-dss->debug = flags & LIBXL_SUSPEND_DEBUG;
-dss->checkpointed_stream = LIBXL_CHECKPOINTED_STREAM_NONE;
+dss->ao = ao;
+dss->callback = domain_suspend_cb;
+
+dss->domid = domid;
+dss->fd = fd;
+dss->type = type;
+dss->live = flags & LIBXL_SUSPEND_LIVE;
+dss->debug = flags & LIBXL_SUSPEND_DEBUG;
+dss->checkpointed_stream = LIBXL_CHECKPOINTED_STREAM_NONE;
+
+rc = libxl__fd_flags_modify_save(gc, dss->fd,
+ ~(O_NONBLOCK|O_NDELAY), 0,
+ >fdfl);
+if (rc < 0) goto out_err;
 
-rc = libxl__fd_flags_modify_save(gc, dss->fd,
- ~(O_NONBLOCK|O_NDELAY), 0,
- >fdfl);
-if (rc < 0) goto out_err;
+libxl__domain_save(egc, dss);
+} else {
+libxl__domain_suspend_state *dsps;
+
+GCNEW(dsps);
+dsps->ao = ao;
+dsps->domid = domid;
+dsps->type = type;
+rc = libxl__domain_suspend_init(egc, dsps, type);
+if (rc < 0) goto out_err;
+dsps->callback_common_done = domain_suspend_empty_cb;
+libxl__domain_suspend(egc, dsps);
+}
 
-libxl__domain_save(egc, dss);
 return AO_INPROGRESS;
 
  out_err:
-- 
2.13.6


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel