Re: [PATCH v5 11/15] libxl_dm: Pre-open QMP socket for QEMU

From: Ian Jackson <ian.jackson@citrix.com>
To: Anthony PERARD <anthony.perard@citrix.com>
Cc: xen-devel@lists.xenproject.org,
	Julien Grall <julien.grall@arm.com>,
	Wei Liu <wei.liu2@citrix.com>
Subject: Re: [PATCH v5 11/15] libxl_dm: Pre-open QMP socket for QEMU
Date: Mon, 12 Nov 2018 15:54:29 +0000	[thread overview]
Message-ID: <23529.41525.219125.82880@mariner.uk.xensource.com> (raw)
In-Reply-To: <20181112152208.GH1302@perard.uk.xensource.com>

Anthony PERARD writes ("Re: [PATCH v5 11/15] libxl_dm: Pre-open QMP socket for QEMU"):
> On Mon, Nov 12, 2018 at 03:14:30PM +0000, Ian Jackson wrote:
> > Yes, it would have to be initialised along with the other members of
> > libxl__domain_build_state.
> 
> I didn't manage to findout where this might be. There is
> libxl__build_pre() but I don't know if it's always called. Is this the
> right place, or is it initialised somewhere else?

Eerrrgh.

Just went off on an absurd wild goose chase to try to answer this
question.  I think libxl__domain_build_state predates the current more
rigorous approach to async functions; indeed, I think it predates any
async stuff at all.

I did git-log -G and found this
  59f8f46a491c9bdc1ad3e0c5ae4f8b48068d13cd
  tools: libxl: remove libxl_domain_build_state from the IDL

AFAICT in that commit it works like this:

* There is a libxl__domain_build_state as a local variable in
  each of do_domain_create and libxl__create_stubdom, which are
  synchronous functions.

* In each case, the struct is passed by reference into
  libxl__domain_build.  libxl__domain_build does not initialise it
  before passing it to libxl__build_pre.  It is filled in as we go.

At some point, libxl_domain_build_state became a member of various
parent state structures.  Those parent state structures (and the loose
state) are all initialised to zero at allocation time.

In 50f8ba84a50ebf80dd22067a04062dbaaf2621ff
  libxl: arm: Fix build after c/s 74fd984ae
libxl__domain_make started to get a pointer to the
libxl__domain_build_state.

AFAICT I think currently libxl__domain_make is actually the first
thing that touches anything in libxl__domain_build_state.  So it
should initialise it.

The layering here is ... a mess.

> Also, now I would probably need to call libxl__pre_open_qmp_socket()
> from libxl__spawn_local_dm() as the d_state (libxl_domain_build_info) is
> const when passed to libxl__build_device_model_args. But that is
> probably fine.

d_state is libxl_domain_build_state isn't it ?
libxl_domain_build_info is d_info or some such.

Ian.

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