Re: [Xen-devel] [PATCH V5 29/32] xl: use "libxl-json" format

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Sun, 2014-06-01 at 20:37 +0100, Wei Liu wrote:
> On Tue, May 20, 2014 at 03:23:26PM +0100, Ian Campbell wrote:
> [...]
> > > @@ -1787,13 +1770,10 @@ static int handle_domain_death(uint32_t *r_domid,
> > >          break;
> > >  
> > >      case LIBXL_ACTION_ON_SHUTDOWN_RESTART_RENAME:
> > > -        reload_domain_config(*r_domid, config_data, config_len);
> > >          restart = 2;
> > >          break;
> > >  
> > >      case LIBXL_ACTION_ON_SHUTDOWN_RESTART:
> > > -        reload_domain_config(*r_domid, config_data, config_len);
> > 
> > Why is it not equally necessary to reload the JSON config at this point
> > if it exists?
> > 
> 
> Because domain configuration is loaded in the caller of
> handle_domain_death now.

OK. Is there any way that could be refactored into a separate patch to
make this one simpler to reason about?

> > Or should we not be updating the domain config as we do the renaming at
> > the end?
> > 
> 
> I agree that libxl_domain_rename is a better place to update stored
> domain name.

We should probably all agree on which of the approachs we want to use
before you refactor (there is more than one subthread suggesting various
ways...)

> > 
> > > @@ -3102,22 +3119,18 @@ static void list_domains_details(const 
> > > libxl_dominfo *info, int nb_domain)
> > >          s = yajl_gen_status_ok;
> > >  
> > >      for (i = 0; i < nb_domain; i++) {
> > > +        libxl_domain_config_init(&d_config);
> > >          /* no detailed info available on dom0 */
> > >          if (info[i].domid == 0)
> > >              continue;
> > > -        rc = libxl_userdata_retrieve(ctx, info[i].domid, "xl", &data, 
> > > &len);
> > > +        rc = libxl_load_domain_configuration(ctx, info[i].domid, 
> > > &d_config);
> > 
> > If a domain was created with xl.old will we not now fail to list details
> > of it here?
> > 
> 
> Don't you need to restart if you install new tool? Is this a valid
> usecase?

Yes, that's true. I had my lazy developer non-rebooting hat on ;-)

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel