WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH,RFC]: Introduce libxl_domain_create()

On Tue, 2010-12-21 at 13:28 +0000, Ian Campbell wrote:
> On Tue, 2010-12-21 at 13:01 +0000, Gianni Tedesco wrote:
> > On Tue, 2010-12-21 at 11:02 +0000, Ian Campbell wrote:
> > > On Tue, 2010-12-21 at 08:44 +0000, Gianni Tedesco wrote:
> > > > On Fri, 2010-12-17 at 09:06 +0000, Ian Campbell wrote:
> > > > > On Tue, 2010-12-14 at 19:00 +0000, Gianni Tedesco wrote:
> > > > > We probably need IDL support for enumerations and other constants.
> > > > 
> > > > Might be a good idea. We also rely on a few xc constants. In the case of
> > > > the python binding I had been adding them manually. If we did this via
> > > > IDL it'd be an idea to generate type-safety macros for that stuff too.
> > > 
> > > What sort of macros?
> > 
> > Well just to have them as typed structs and macros to get/set the
> > integer values.
> 
> Seem like overkill to me.
> 
> > Beyond that I don't see much point in putting them in
> > the IDL at all.
> 
> So that language bindings can make a variable/constant with the
> appropriate name and value available to the user, without needing to
> manually code a list in each set of bindings.

Fair enough

> > 
> > > > > > +typedef struct {
> > > > [...]
> > > > > > +} libxl_domain_config;
> > > > > 
> > > > > Should be in IDL so it gets a destructor? Could require adding an 
> > > > > Array
> > > > > construct to handle the foo + num_foo style stuff.
> > > > 
> > > > I've thought about that and rejected it because C arrays don't map to
> > > > anything useful in language bindings. It makes sense to me to keep this
> > > > as a builtin and use functions to fill these domain creation related
> > > > structures in for us.
> > > 
> > > OK
> > > 
> > > > But then what you get is like two versions of:
> > > >  - libxl_device_add_(nic|block|etc)
> > > > One for a live domain and one for domain creation.
> > > > 
> > > > I have been toying with the idea of using polymorphism (is that what
> > > > it's called?) So that such a function would multiplex to different
> > > > implementations depending on whether this is a live domain or a
> > > > description of a domain for creation. It might need a bit of thinking
> > > > through as how it would be used.
> > > > 
> > > > Not sure what the others think of that?
> > > 
> > > Proper polymorphism is a bit tricky in C, since you can't have multiple
> > > variations of the same function with different parameters and you simply
> > > end up with multiple different functions -- i.e. back where you started.
> > > 
> > > The need for a version of libxl_device_add_FOO for the create case is
> > > simply to support automatically extending the array while filing in the
> > > structure etc? I don't see a useful way to have a function which works
> > > like this for both live and in-creation domains.
> > 
> > Maybe I'm using the wrong OOP term? But you would have a libxl_domain
> > struct and operations on it would be redirected via virtual methods.
> > Either to update a struct or a live domain. But the input parameters
> > would always be the same.
> 
> Hmm, so in the live domain case the struct would be mostly empty apart
> from a domid and in the domain build case it would contains all the
> various info structures?

Exactly yes.

> Polymorphic would mean you had libxl_add_nic(int domid, ...) and
> libxl_add_nic(struct libxl_domain, ...) and the compiler would pick the
> right one from the arguments you give.
> 
> I think what you describe is more like inheritance or something.
>
> > > > > > +static pid_t autoconnect_console(libxl_ctx *ctx, int domid)
> > > > > > +{
> > > > > [...]
> > > > > > +}
> > > > > 
> > > > > I think console connect should be under toolstack control (i.e. stay 
> > > > > in
> > > > > xl). exec'ing the xenconsole client is only one way of connecting the
> > > > > console, e.g. xapi might want to launch vncterm instead.
> > > > > 
> > > > > I think libxl_domain_create should take a callback function pointer to
> > > > > connect the console. It's possible that libxl might also provide a
> > > > > convenience function which launches xenconsole which is suitable for 
> > > > > use
> > > > > as this callback but ultimately it should be the toolstack's decision
> > > > > what to do here.
> > > > 
> > > > I think you're right. I had just been trying to avoid having a mega
> > > > function with 12 arguments, 6 of them callbacks.
> > > 
> > > A structure containing the callback functions is the answer there.
> > 
> > Hmm, my displeasure is not so much how to pass the arguments but that
> > API's based around a lot of callbacks tend to be difficult to understand
> > and hard to maintain. They pretend to make it such that you don't need
> > to know what order things are done in but then you end up relying on the
> > order things are done in...
> > 
> > I have already factored out the other nastiness of the API I proposed
> > (the flags) but autoconnect seems to be the final sticking point.
> > 
> > > > I had the idea that the original domain_create() was very fragile and I
> > > > didn't want to move things around. But on the other hand it seems to me
> > > > that there's no reason to start the console at two different points
> > > > depending on pv or hvm and perhaps I should just try to move the code
> > > > around.
> > > 
> > > I'm pretty certain Stefano did this deliberately when he introduced
> > > console support for HVM, I don't remember the specific constraint he
> > > found on HVM though. It seems to arise from 22100:fde833c66948 but the
> > > commit message doesn't say why just "it needs to be this way".
> > > 
> > > > Domains start paused anyway so why can't we just:
> > > > 
> > > >  libxl_domain_create();
> > > >  do_console_stuff();
> > > >  libxl_domain_unpause();
> > > 
> > > Not quite because for a PV domain we need to do the console before the
> > > bootloader runs (so the user can interact with pygrub) and the
> > > bootloader provides the input to libxl_domain_create().
> > > 
> > > So for PV it ends up as
> > >         libxl_domain_make() // returns a domid
> > >         do_console_stuff() // launches console client
> > >         libxl_run_bootloader() // potentially interact with console, 
> > > return kernel etc
> > >         libxl_domain_create() // build domain using kernel
> > >         libxl_domain_unpause() // go go go
> > > 
> > > My guess is that there is some reason you can't create the console for
> > > an HVM guest before libxl_domain_create but I don't specifically know
> > > why, perhaps qemu needs to be running?
> > > 
> > > In theory at least the do_console_stuff should cause a client to start
> > > and wait for the server end to appear, rather than insist on connecting
> > > immediately and it already behaves that way for PV guests, I don't see
> > > any fundamental reason HVM couldn't be the same.
> > 
> > Yes, the points above make sense. I think I would rather go down the
> > road of libxl callers setting up their console stuff before creating the
> > domain, providing a wait_for_console_stuff() API. Or at worst, give them
> > the control back over bootloader and do a 2/3-phase domain creation API.
> 
> That's pretty close to what we have today, isn't it?
> 
> We have (roughly):
>       libxl_domain_make()
>       do_the_pv_console()
>       libxl_run_bootloader()
>       libxl_domain_create()
>       for each device: libxl_device_blah_add
>       do_the_hvm_console()
>       libxl_domain_unpause()
> 
> Your initial patch moved all of that into libxl_domain_create but now
> you are suggesting that only the "for each device" and/or
> libxl_run_bootloader bits gets pushed down? e.g. the caller does
>       libxl_domain_make()
>       do_the_pv_console()
>       libxl_domain_create()
>               libxl_run_bootloader()
>               libxl_domain_actually_create()
>               for each device: libxl_device_blah_add
>       do_the_hvm_console()
>       libxl_domain_unpause()
> 
> seems semi plausible, although how to do the do_the_pv_console vs hvm
> console in a clean way.

Yeah I don't like it because I already think xl is doing a helluva lot
of stuff in its create_domain() function.

If we can have pv and hvm cases unified so that both consoles can be
done a lot earlier (ie. before libxl_domain_create()) then that would
let us keep the bulk of the work in libxl and we can zap:
 -  libxl_domain(make|create|restore|run_bootloader)

Then again, it may complicate the callers because then they have to do
something if domain_create() fails to clean that up.

So, domain creation really is hard. Let's go shopping :S

Gianni


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