Re: [Xen-devel] Xen 4.2 Release Plan / TODO [and 1 more messages]

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Campbell writes ("Re: Xen 4.2 Release Plan / TODO"):
> On Wed, 2012-04-11 at 17:13 +0100, Ian Jackson wrote:
> > It may be that we can add dummy ao_hows to these functions which might
> > be good enough for now, although particularly for domain creation
> > (which includes spawning) it might complicate the efforts of users to
> > use the new API.
> 
> How close is your half baked series to do it properly?

Another weeks or two maybe, if I don't get too badly sucked into doing
anything else.

> > Currently any use of subprocesses inside libxl which is not dealt with
> > by the new event machinery will experience problems if the event loop
> > is also used, because the event loop will reap the children.
> 
> You've covered the bootloader one in existing patches and mentioned the
> libxl_$CONSOLE_exec style ones in your last mail. The only other one is
> the DM fork which is covered by your rewrite of libxl__spawn?

Yes, I think so.  That's why I've been targeting those.


Ian Campbell writes ("Re: Xen 4.2 Release Plan / TODO"):
> On Wed, 2012-04-11 at 17:11 +0100, Ian Jackson wrote:
> > I took a look at libxl.h and came up with the comments below.  Firstly
> > general things I tripped over, and then the list of things which need
> > converting to the new event system.
> 
> A slightly worrying list at this stage in the game.

Yes.

> > Other:
> > ======
> > 
> > ]   int libxl_wait_for_free_memory(libxl_ctx *ctx, uint32_t domid, 
> > ]   /* wait for the memory target of a domain to be reached */
> > ]   int libxl_wait_for_memory_target(libxl_ctx *ctx, uint32_t domid
> > 
> > This whole memory interface is a bit of a dog's breakfast.
> 
> I think we can defer this to 4.3? The existing interface may be pants
> but at least the name is pretty explicit that it will block. I think
> this should then be easy enough to sort out in a backward compatible
> manner in 4.3 since I expect the name of the function would change and
> we could retain the old name in terms of the new for compat.

The problem isn't just that it blocks but also that the semantics are
wrong.  This is all related to the problems of allocating memory.  We
had a recent conversation where we concluded that we needed hypervisor
changes to do memory assignment in a race-free way.  This is not 4.3
material.

I suggest that the right answer is to make a note that this interface
is considered substandard and not to rely on it too heavily; we expect
to replace it in the future and at that point we will provide an
emulation which we intend will works by and large well enough for
existing callers.

> > ]   int libxl_vncviewer_exec(libxl_ctx *ctx, uint32_t domid, int autopass);
...
> > These functions should not exec things for you; they should tell you
> > the parameters.  Any execing helpers should be in libxlu.
> 
> It's not enough for them to just use libxl__exec?
> 
> It'd be reasonably easy to make this return a libxl_string_list or
> similar and to write a libxlu function which takes one of those.

But what if your vnc viewer doesn't have the command line options
these functions expect ?  libxl_vncviewer_* should give you an idl
struct containing the ip address (or perhaps the domain name), port
number, and password.

The command lines should be built in xlu.

> > Need to be ao/eventified:
> > =========================
...
> > ]   typedef int (*libxl_console_ready)(libxl_ctx *ctx, uint32_t domi
> > ]   int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config 
> > ]   int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_con
> 
> You are on the case with these?

Yes for the creation.

> > ]   typedef struct {
> > ]   #define XL_SUSPEND_DEBUG 1
> > ]   #define XL_SUSPEND_LIVE 2
> > ]       int flags;
> > ]       int (*suspend_callback)(void *, int);
> > ]   } libxl_domain_suspend_info;
> > ...
> > ]   int libxl_domain_suspend(libxl_ctx *ctx, libxl_domain_suspend_i
> > ]                            uint32_t domid, int fd);

No for these, which I haven't looked at at all.

> > ]   int libxl_domain_shutdown(libxl_ctx *ctx, uint32_t domid);
> > ]   int libxl_domain_reboot(libxl_ctx *ctx, uint32_t domid);
> > 
> > Are these now merely initiations ?
> 
> I think so yes.
> 
> Does a non-transaction write make a function "slow"? That's all these
> actually do. If they are currently "fast" then we could likely get away
> with a dummy ao_how. (I think it is valid for a function which is "fast"
> to take an ao_how and always run sync?)

All xenstore operations apart from waiting for watches are fast, even
transactional read/modify/writes.  So these are OK.

> > ]   int libxl_domain_core_dump(libxl_ctx *ctx, uint32_t domid, const char 
> > *filename);
> > 
> > Might become long-running in the future.
> 
> But is currently fast? Dummy ao_how?

That's probably correct, yes.

> > ]   int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl
> 
> Roger makes this async in his hotplug series.

Yes.

> > ]   /*
> > ]    * Insert a CD-ROM device. A device corresponding to disk must already
> > ]    * be attached to the guest.
> > ]    */
> > ]   int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_de
> 
> Were you looking at this one? I know you mentioned it at one point.

No, I haven't, but it shouldn't be too hard.

> > ]   /*
> > ]    * Make a disk available in this (the control) domain. Returns path to
> > ]    * a device.
> > ]    */
> > ]   char * libxl_device_disk_local_attach(libxl_ctx *ctx, libxl_dev
> > ]   int libxl_device_disk_local_detach(libxl_ctx *ctx, libxl_device
> > 
> > Does this even need to be public at this stage ?
> 
> I think Stefano internalises them in his qdisk/dom0-attach series.

Oh good.

> > ]   /* Network Interfaces */
> > ]   int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_
> > 
> > ]   /* Keyboard */
> > ]   int libxl_device_vkb_add(libxl_ctx *ctx, uint32_t domid, libxl_
> > 
> > ]   /* Framebuffer */
> > ]   int libxl_device_vfb_add(libxl_ctx *ctx, uint32_t domid, libxl_
> 
> These ought to be pretty trivial conversions?

Yes.  For the NICs I expect Roger will end up adding an ao_how for
hotplug script invocation.

> > ]   /* PCI Passthrough */
> > ]   int libxl_device_pci_add(libxl_ctx *ctx, uint32_t domid, libxl_de
> > ]   int libxl_device_pci_remove(libxl_ctx *ctx, uint32_t domid, libxl
> 
> I'm less confident that this one will be trivial to make async :-(

It's not trivial but it doesn't seem to contain any significant
difficulties.  It's all just xenstore stuff.  You split something like
do_pci_remove into a setup and a callback.

> > ]   typedef struct libxl__xen_console_reader libxl_xen_console_reader;
> > ]
> > ]   libxl_xen_console_reader *
> > ]       libxl_xen_console_read_start(libxl_ctx *ctx, int clear);
> > ]   int libxl_xen_console_read_line(libxl_ctx *ctx,
> > ]                                   libxl_xen_console_reader *cr,
> > ]                                   char **line_r);
> > ]   void libxl_xen_console_read_finish(libxl_ctx *ctx,
> > ]                                      libxl_xen_console_reader *cr);
> 
> This is basically "xl dmesg". I'm not sure what interface makes sense
> here, really it is just dumping the current ring, so each call is
> "fast".

OK, good.

Is it possible to wait for more input ?

> I'm not sure there is a need for an event driven "new-line-in-log"
> callback style thing, i.e. a need to implement a "tail -f" style thing. 
> Firstly I'm not sure that Xen actually produces an event which would
> allow this to be implemented without polling and secondly if you want
> that you can configure xenconsoled to log the hypervisor output and then
> tail the logfile.

Right.

> Perhaps this interface is OK?

I think I'm sold on it being supportable.

Ian.

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