Re: [Xen-devel] [PATCH 1/2] libxl: modify domain config when moving domain to another cpupool

2018-10-03 Thread George Dunlap
On Wed, Oct 3, 2018 at 12:29 PM Wei Liu  wrote:
>
> On Wed, Oct 03, 2018 at 12:02:24PM +0100, George Dunlap wrote:
> > On Tue, Oct 2, 2018 at 3:20 PM Juergen Gross  wrote:
> > >
> > > Today the domain config info contains the cpupool name the domain was
> > > started in only if the cpupool was specified at domain creation. Moving
> > > the domain to another cpupool later won't change that information.
> > >
> > > Correct that by modifying the domain config accordingly.
> > >
> > > Signed-off-by: Juergen Gross 
> >
> > Would it be better to do this the same way the scheduling parameters
> > was done -- by adding this to libxl_retrieve_domain_configuration()?
> > That way the cpupool would show up in `xl list -l` as well (I think).
>
> This already modifies the saved state file, there will not be mismatch
> between the saved state and the state in hypervisor. `xl list -l` should
> work just fine.

If you do it Juergens way, `xl list -l` will show things you have
*changed*, but not the defaults.  If you do it the way the scheduling
parameters was done, the pool name will be shown even if there was no
pool specified in the config file, nor the vm migrated from the
default pool to a different one.

 -George

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

Re: [Xen-devel] [PATCH 1/2] libxl: modify domain config when moving domain to another cpupool

2018-10-03 Thread Wei Liu
On Wed, Oct 03, 2018 at 12:02:24PM +0100, George Dunlap wrote:
> On Tue, Oct 2, 2018 at 3:20 PM Juergen Gross  wrote:
> >
> > Today the domain config info contains the cpupool name the domain was
> > started in only if the cpupool was specified at domain creation. Moving
> > the domain to another cpupool later won't change that information.
> >
> > Correct that by modifying the domain config accordingly.
> >
> > Signed-off-by: Juergen Gross 
> 
> Would it be better to do this the same way the scheduling parameters
> was done -- by adding this to libxl_retrieve_domain_configuration()?
> That way the cpupool would show up in `xl list -l` as well (I think).

This already modifies the saved state file, there will not be mismatch
between the saved state and the state in hypervisor. `xl list -l` should
work just fine.

Wei.

> 
>  -George

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

Re: [Xen-devel] [PATCH 1/2] libxl: modify domain config when moving domain to another cpupool

2018-10-03 Thread George Dunlap
On Tue, Oct 2, 2018 at 3:20 PM Juergen Gross  wrote:
>
> Today the domain config info contains the cpupool name the domain was
> started in only if the cpupool was specified at domain creation. Moving
> the domain to another cpupool later won't change that information.
>
> Correct that by modifying the domain config accordingly.
>
> Signed-off-by: Juergen Gross 

Would it be better to do this the same way the scheduling parameters
was done -- by adding this to libxl_retrieve_domain_configuration()?
That way the cpupool would show up in `xl list -l` as well (I think).

 -George

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

Re: [Xen-devel] [PATCH 1/2] libxl: modify domain config when moving domain to another cpupool

2018-10-03 Thread Wei Liu
On Tue, Oct 02, 2018 at 04:19:33PM +0200, Juergen Gross wrote:
> Today the domain config info contains the cpupool name the domain was
> started in only if the cpupool was specified at domain creation. Moving
> the domain to another cpupool later won't change that information.
> 
> Correct that by modifying the domain config accordingly.
> 
> Signed-off-by: Juergen Gross 
> ---
>  tools/libxl/libxl_cpupool.c | 28 +---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/libxl/libxl_cpupool.c b/tools/libxl/libxl_cpupool.c
> index 85b06882db..92cf29bc6b 100644
> --- a/tools/libxl/libxl_cpupool.c
> +++ b/tools/libxl/libxl_cpupool.c
> @@ -430,17 +430,39 @@ out:
>  int libxl_cpupool_movedomain(libxl_ctx *ctx, uint32_t poolid, uint32_t domid)
>  {
>  GC_INIT(ctx);
> +libxl_domain_config d_config;
> +libxl__domain_userdata_lock *lock = NULL;
>  int rc;
>  
> +libxl_domain_config_init(_config);
> +
>  rc = xc_cpupool_movedomain(ctx->xch, poolid, domid);
>  if (rc) {
>  LOGEVD(ERROR, rc, domid, "Error moving domain to cpupool");
> -GC_FREE;
> -return ERROR_FAIL;
> +rc = ERROR_FAIL;
> +goto out;
> +}
> +
> +lock = libxl__lock_domain_userdata(gc, domid);
> +if (!lock) {
> +rc = ERROR_LOCK_FAIL;
> +goto out;
>  }

It is better to move the lock before calling xc_cpupool_movedomain to
avoid races when there are multiple callers of libxl_cpupool_movedomain.

Wei.

>  
> +rc = libxl__get_domain_configuration(gc, domid, _config);
> +if (rc)
> +goto out;
> +
> +free(d_config.c_info.pool_name);
> +d_config.c_info.pool_name = libxl_cpupoolid_to_name(ctx, poolid);
> +
> +rc = libxl__set_domain_configuration(gc, domid, _config);
> +
> +out:
> +if (lock) libxl__unlock_domain_userdata(lock);
> +libxl_domain_config_dispose(_config);
>  GC_FREE;
> -return 0;
> +return rc;
>  }
>  
>  /*
> -- 
> 2.16.4
> 

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

[Xen-devel] [PATCH 1/2] libxl: modify domain config when moving domain to another cpupool

2018-10-02 Thread Juergen Gross
Today the domain config info contains the cpupool name the domain was
started in only if the cpupool was specified at domain creation. Moving
the domain to another cpupool later won't change that information.

Correct that by modifying the domain config accordingly.

Signed-off-by: Juergen Gross 
---
 tools/libxl/libxl_cpupool.c | 28 +---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl_cpupool.c b/tools/libxl/libxl_cpupool.c
index 85b06882db..92cf29bc6b 100644
--- a/tools/libxl/libxl_cpupool.c
+++ b/tools/libxl/libxl_cpupool.c
@@ -430,17 +430,39 @@ out:
 int libxl_cpupool_movedomain(libxl_ctx *ctx, uint32_t poolid, uint32_t domid)
 {
 GC_INIT(ctx);
+libxl_domain_config d_config;
+libxl__domain_userdata_lock *lock = NULL;
 int rc;
 
+libxl_domain_config_init(_config);
+
 rc = xc_cpupool_movedomain(ctx->xch, poolid, domid);
 if (rc) {
 LOGEVD(ERROR, rc, domid, "Error moving domain to cpupool");
-GC_FREE;
-return ERROR_FAIL;
+rc = ERROR_FAIL;
+goto out;
+}
+
+lock = libxl__lock_domain_userdata(gc, domid);
+if (!lock) {
+rc = ERROR_LOCK_FAIL;
+goto out;
 }
 
+rc = libxl__get_domain_configuration(gc, domid, _config);
+if (rc)
+goto out;
+
+free(d_config.c_info.pool_name);
+d_config.c_info.pool_name = libxl_cpupoolid_to_name(ctx, poolid);
+
+rc = libxl__set_domain_configuration(gc, domid, _config);
+
+out:
+if (lock) libxl__unlock_domain_userdata(lock);
+libxl_domain_config_dispose(_config);
 GC_FREE;
-return 0;
+return rc;
 }
 
 /*
-- 
2.16.4


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