Xen 4.2 Release Plan / TODO

Xen 4.2 Release Plan / TODO

[Ian Campbell]

Plan for a 4.2 release:
http://lists.xen.org/archives/html/xen-devel/2012-03/msg00793.html

The time line is as follows:

19 March        -- TODO list locked down
2 April         -- Feature Freeze               << WE ARE HERE
Mid/Late April  -- First release candidate
Weekly          -- RCN+1 until it is ready

The updated TODO list follows. Per the release plan a strong case will
now have to be made as to why new items should be added to the list,
especially as a blocker, rather than deferred to 4.3.

We have now entered Feature Freeze. Patches which have been posted
before or which address something on the list below are still acceptable
(for now, we will gradually be getting stricter about this), everything
else will be deferred until 4.3 by default.

hypervisor, blockers:
      * NONE?
 
tools, blockers:
      * libxl stable API -- we would like 4.2 to define a stable API
        which downstream's can start to rely on not changing. Aspects of
        this are:
              * Safe fork vs. fd handling hooks. Involves API changes
                (Ian J, patches posted)
              * locking/serialization, e.g., for domain creation. As of
                now,  nothing is provided for this purpose, and
                downstream toolstacks have to put their own mechanisms
                in place. E.g., xl uses a fcntl() lock around the whole
                process of domain creation. It should OTOH be libxl job
                to offer a proper set of hooks --placed at the proper
                spots during the domain creation process-- for the
                downstreams to  fill with the proper callbacks. (Dario
                Faggioli)
              * agree & document compatibility guarantees + associated
                technical measures (Ian C, patches posted)
      * xl compatibility with xm:
              * feature parity wrt driver domain support (George Dunlap)
              * xl support for "rtc_timeoffset" and "localtime" (Lin
                Ming, Patches posted)
      * More formally deprecate xm/xend. Manpage patches already in
        tree. Needs release noting and communication around -rc1 to
        remind people to test xl.
      * Domain 0 block attach & general hotplug when using qdisk backend
        (need to start qemu as necessary etc) (Stefano S)
      * file:// backend performance. qemu-xen-tradition's qdisk is quite
        slow & blktap2 not available in upstream kernels. Need to
        consider our options:
              * qemu-xen's qdisk is thought to be well performing but
                qemu-xen is not yet the default. Complexity arising from
                splitting qemu-for-qdisk out from qemu-for-dm and
                running N qemu's.
              * potentially fully userspace blktap could be ready for
                4.2
              * use /dev/loop+blkback. This requires loop driver AIO and
                O_DIRECT patches which are not (AFAIK) yet upstream.
              * Leverage XCP's blktap2 DKMS work.
              * Other ideas?
                      * In general we should recommend blkback (e.g.
                        with LVM) since it always out performs other
                        solutions, although at the expense of
                        flexibility regarding image formats.
        Stefano has done a lot of work here and has proposed some
        performance improvements for qdisk in both qemu-xen and
        qemu-xen-traditional which make them a plausible alternative for
        Xen 4.2.
      * Improved Hotplug script support (Roger Pau Monné, patches
        posted)
      * Block script support -- follows on from hotplug script (Roger
        Pau Monné)

hypervisor, nice to have:
      * solid implementation of sharing/paging/mem-events (using work
        queues) (Tim Deegan, Olaf Herring et al -- patches posted)
              * "The last patch to use a waitqueue in
                __get_gfn_type_access() from Tim works.  However, there
                are a few users who call __get_gfn_type_access with the
                domain_lock held. This part needs to be addressed in
                some way."
      * Sharing support for AMD (Tim, Andres).
      * PoD performance improvements (George Dunlap)

tools, nice to have:
      * Configure/control paging via xl/libxl (Olaf Herring, lots of
        discussion around interface, general consensus reached on what
        it should look like. Olaf has let me know that he is probably
        not going to have time to do this for 4.2, will likely slip to
        4.3 unless someone else want to pick up that work)
      * Upstream qemu feature patches:
              * Upstream qemu PCI passthrough support (Anthony Perard,
                patches sent)
              * Upstream qemu save restore (Anthony Perard, Stefano
                Stabellini, qemu patches applied, waiting for libxl etc
                side which has been recently reposted)
      * Nested-virtualisation. Currently "experimental". Likely to
        release that way.
              * Nested SVM. Tested in a variety of configurations but
                still some issues with the most important use case (w7
                XP mode) [0]  (Christoph Egger)
              * Nested VMX. Needs nested EPT to be genuinely useful.
                Need more data on testedness etc (Intel)
      * Initial xl support for Remus (memory checkpoint, blackholing)
        (Shriram, patches reposted recently, was blocked behind qemu
        save restore patches which are now in)
      * xl compatibility with xm:
              * xl support for autospawning vncviewer (vncviewer=1 or
                otherwise) (Goncalo Gomes, patches posted)
              * support for vif "rate" parameter (Mathieu Gagné, patches
                posted)
              * expose PCI back "permissive" parameter (George Dunlap)

[0] http://lists.xen.org/archives/html/xen-devel/2012-03/msg00883.html


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

Re: Xen 4.2 Release Plan / TODO

[David Vrabel]

On 02/04/12 11:26, Ian Campbell wrote:
> 
> We have now entered Feature Freeze. Patches which have been posted
> before or which address something on the list below are still acceptable
> (for now, we will gradually be getting stricter about this), everything
> else will be deferred until 4.3 by default.

How does this affect ARM patches?  Are they similarly restricted?

David

Re: Xen 4.2 Release Plan / TODO

[Ian Campbell]

On Mon, 2012-04-02 at 11:39 +0100, David Vrabel wrote:
> On 02/04/12 11:26, Ian Campbell wrote:
> > 
> > We have now entered Feature Freeze. Patches which have been posted
> > before or which address something on the list below are still acceptable
> > (for now, we will gradually be getting stricter about this), everything
> > else will be deferred until 4.3 by default.
> 
> How does this affect ARM patches?  Are they similarly restricted?

Given that ARM support in 4.2 is going to be experimental in nature I
suggest that, at least for the time being, ARM patches which do not have
an impact on generic or non-ARM code continue to be accepted.

Any objections?

Ian.

Re: Xen 4.2 Release Plan / TODO

[George Dunlap]

On Mon, Apr 2, 2012 at 11:26 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> Plan for a 4.2 release:
> http://lists.xen.org/archives/html/xen-devel/2012-03/msg00793.html
>
> The time line is as follows:
>
> 19 March        -- TODO list locked down
> 2 April         -- Feature Freeze               << WE ARE HERE
> Mid/Late April  -- First release candidate
> Weekly          -- RCN+1 until it is ready
>
> The updated TODO list follows. Per the release plan a strong case will
> now have to be made as to why new items should be added to the list,
> especially as a blocker, rather than deferred to 4.3.
>
> We have now entered Feature Freeze. Patches which have been posted
> before or which address something on the list below are still acceptable
> (for now, we will gradually be getting stricter about this), everything
> else will be deferred until 4.3 by default.
>
> hypervisor, blockers:
>      * NONE?
>
> tools, blockers:
>      * libxl stable API -- we would like 4.2 to define a stable API
>        which downstream's can start to rely on not changing. Aspects of
>        this are:
>              * Safe fork vs. fd handling hooks. Involves API changes
>                (Ian J, patches posted)
>              * locking/serialization, e.g., for domain creation. As of
>                now,  nothing is provided for this purpose, and
>                downstream toolstacks have to put their own mechanisms
>                in place. E.g., xl uses a fcntl() lock around the whole
>                process of domain creation. It should OTOH be libxl job
>                to offer a proper set of hooks --placed at the proper
>                spots during the domain creation process-- for the
>                downstreams to  fill with the proper callbacks. (Dario
>                Faggioli)
>              * agree & document compatibility guarantees + associated
>                technical measures (Ian C, patches posted)
>      * xl compatibility with xm:
>              * feature parity wrt driver domain support (George Dunlap)

The only thing missing is the pci "permissive" flag.  Patches posted.

>              * xl support for "rtc_timeoffset" and "localtime" (Lin
>                Ming, Patches posted)
>      * More formally deprecate xm/xend. Manpage patches already in
>        tree. Needs release noting and communication around -rc1 to
>        remind people to test xl.
>      * Domain 0 block attach & general hotplug when using qdisk backend
>        (need to start qemu as necessary etc) (Stefano S)
>      * file:// backend performance. qemu-xen-tradition's qdisk is quite
>        slow & blktap2 not available in upstream kernels. Need to
>        consider our options:
>              * qemu-xen's qdisk is thought to be well performing but
>                qemu-xen is not yet the default. Complexity arising from
>                splitting qemu-for-qdisk out from qemu-for-dm and
>                running N qemu's.
>              * potentially fully userspace blktap could be ready for
>                4.2
>              * use /dev/loop+blkback. This requires loop driver AIO and
>                O_DIRECT patches which are not (AFAIK) yet upstream.
>              * Leverage XCP's blktap2 DKMS work.
>              * Other ideas?
>                      * In general we should recommend blkback (e.g.
>                        with LVM) since it always out performs other
>                        solutions, although at the expense of
>                        flexibility regarding image formats.
>        Stefano has done a lot of work here and has proposed some
>        performance improvements for qdisk in both qemu-xen and
>        qemu-xen-traditional which make them a plausible alternative for
>        Xen 4.2.
>      * Improved Hotplug script support (Roger Pau Monné, patches
>        posted)
>      * Block script support -- follows on from hotplug script (Roger
>        Pau Monné)
>
> hypervisor, nice to have:
>      * solid implementation of sharing/paging/mem-events (using work
>        queues) (Tim Deegan, Olaf Herring et al -- patches posted)
>              * "The last patch to use a waitqueue in
>                __get_gfn_type_access() from Tim works.  However, there
>                are a few users who call __get_gfn_type_access with the
>                domain_lock held. This part needs to be addressed in
>                some way."
>      * Sharing support for AMD (Tim, Andres).
>      * PoD performance improvements (George Dunlap)
>
> tools, nice to have:
>      * Configure/control paging via xl/libxl (Olaf Herring, lots of
>        discussion around interface, general consensus reached on what
>        it should look like. Olaf has let me know that he is probably
>        not going to have time to do this for 4.2, will likely slip to
>        4.3 unless someone else want to pick up that work)
>      * Upstream qemu feature patches:
>              * Upstream qemu PCI passthrough support (Anthony Perard,
>                patches sent)
>              * Upstream qemu save restore (Anthony Perard, Stefano
>                Stabellini, qemu patches applied, waiting for libxl etc
>                side which has been recently reposted)
>      * Nested-virtualisation. Currently "experimental". Likely to
>        release that way.
>              * Nested SVM. Tested in a variety of configurations but
>                still some issues with the most important use case (w7
>                XP mode) [0]  (Christoph Egger)
>              * Nested VMX. Needs nested EPT to be genuinely useful.
>                Need more data on testedness etc (Intel)
>      * Initial xl support for Remus (memory checkpoint, blackholing)
>        (Shriram, patches reposted recently, was blocked behind qemu
>        save restore patches which are now in)
>      * xl compatibility with xm:
>              * xl support for autospawning vncviewer (vncviewer=1 or
>                otherwise) (Goncalo Gomes, patches posted)
>              * support for vif "rate" parameter (Mathieu Gagné, patches
>                posted)
>              * expose PCI back "permissive" parameter (George Dunlap)
>
> [0] http://lists.xen.org/archives/html/xen-devel/2012-03/msg00883.html
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: Xen 4.2 Release Plan / TODO

[Stefano Stabellini]

On Mon, 2 Apr 2012, Ian Campbell wrote:
>       * file:// backend performance. qemu-xen-tradition's qdisk is quite
>         slow & blktap2 not available in upstream kernels. Need to
>         consider our options:
>               * qemu-xen's qdisk is thought to be well performing but
>                 qemu-xen is not yet the default. Complexity arising from
>                 splitting qemu-for-qdisk out from qemu-for-dm and
>                 running N qemu's.
>               * potentially fully userspace blktap could be ready for
>                 4.2
>               * use /dev/loop+blkback. This requires loop driver AIO and
>                 O_DIRECT patches which are not (AFAIK) yet upstream.
>               * Leverage XCP's blktap2 DKMS work.
>               * Other ideas?
>                       * In general we should recommend blkback (e.g.
>                         with LVM) since it always out performs other
>                         solutions, although at the expense of
>                         flexibility regarding image formats.
>         Stefano has done a lot of work here and has proposed some
>         performance improvements for qdisk in both qemu-xen and
>         qemu-xen-traditional which make them a plausible alternative for
>         Xen 4.2.

Using O_DIRECT rather than the default write-through cache policy solves
the performance problem in QEMU.
I sent patches to xen-devel to enable O_DIRECT for QDISK on
qemu-xen-traditional. I also sent patches to qemu-devel to enable
O_DIRECT and native AIO for QDISK on upstream QEMU.

Upstream QEMU PV backends are as good as the ones in
qemu-xen-traditional, but upstream QDISK performs better because it can
use native AIO. Thus I sent a patch to xen-devel to enable upstream QEMU
as default to provide userspace backends to PV guests.

I wrote and sent a patch series to fix the m2p override in Linux in case
the frontend and backend are both in the same domain.
Then I sent a patch series to xen-devel to make
libxl__device_disk_local_attach work with QDISK: the implementation uses
a frontend/backend pair in dom0.

As a result, with all these patches applied, the disk performances with
file based disk images are good and one can use a qcow2 file to store
a PV guest disk image and boot from it using pygrub.

Re: Xen 4.2 Release Plan / TODO

[Ian Jackson]

Ian Campbell writes ("Xen 4.2 Release Plan / TODO"):
> Plan for a 4.2 release:
> http://lists.xen.org/archives/html/xen-devel/2012-03/msg00793.html
...
> tools, blockers:
>       * libxl stable API -- we would like 4.2 to define a stable API
>         which downstream's can start to rely on not changing. Aspects of
>         this are:

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.

Ian.


Other:
======

]   int libxl_wait_for_free_memory(libxl_ctx *ctx, uint32_t domid, uint32_t memory_kb, int wait_secs);
]   /* wait for the memory target of a domain to be reached */
]   int libxl_wait_for_memory_target(libxl_ctx *ctx, uint32_t domid, int wait_secs);

This whole memory interface is a bit of a dog's breakfast.

]   int libxl_vncviewer_exec(libxl_ctx *ctx, uint32_t domid, int autopass);
]   int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num, libxl_console_type type);
]   /* libxl_primary_console_exec finds the domid and console number
]    * corresponding to the primary console of the given vm, then calls
]    * libxl_console_exec with the right arguments (domid might be different
]    * if the guest is using stubdoms).
]    * This function can be called after creating the device model, in
]    * case of HVM guests, and before libxl_run_bootloader in case of PV
]    * guests using pygrub. */
]   int libxl_primary_console_exec(libxl_ctx *ctx, uint32_t domid_vm);

These functions should not exec things for you; they should tell you
the parameters.  Any execing helpers should be in libxlu.

]   /* common paths */
]   const char *libxl_sbindir_path(void);
]   const char *libxl_bindir_path(void);
]   const char *libxl_libexec_path(void);
]   const char *libxl_libdir_path(void);
]   const char *libxl_sharedir_path(void);
]   const char *libxl_private_bindir_path(void);
]   const char *libxl_xenfirmwaredir_path(void);
]   const char *libxl_xen_config_dir_path(void);
]   const char *libxl_xen_script_dir_path(void);
]   const char *libxl_lock_dir_path(void);
]   const char *libxl_run_dir_path(void);
]   const char *libxl_xenpaging_dir_path(void);

Surely these should be private ?


Need to be ao/eventified:
=========================

]   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_info *info,
]                            uint32_t domid, int fd);

]   typedef int (*libxl_console_ready)(libxl_ctx *ctx, uint32_t domid, void *priv);
]   int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config, libxl_console_ready cb, void *priv, uint32_t *domid);
]   int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config *d_config, libxl_console_ready cb, void *priv, uint32_t *domid, int restore_fd);

]   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 ?

]   int libxl_domain_core_dump(libxl_ctx *ctx, uint32_t domid, const char *filename);

Might become long-running in the future.

]   int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk);

]   /*
]    * 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_device_disk *disk);

]   /*
]    * Make a disk available in this (the control) domain. Returns path to
]    * a device.
]    */
]   char * libxl_device_disk_local_attach(libxl_ctx *ctx, libxl_device_disk *disk);
]   int libxl_device_disk_local_detach(libxl_ctx *ctx, libxl_device_disk *disk);

Does this even need to be public at this stage ?

]   /* Network Interfaces */
]   int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic);

]   /* Keyboard */
]   int libxl_device_vkb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb);

]   /* Framebuffer */
]   int libxl_device_vfb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb *vfb);

]   /* PCI Passthrough */
]   int libxl_device_pci_add(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev);
]   int libxl_device_pci_remove(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev);

]   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);

]   char *libxl_tmem_list(libxl_ctx *ctx, uint32_t domid, int use_long);
]   int libxl_tmem_freeze(libxl_ctx *ctx, uint32_t domid);
]   int libxl_tmem_destroy(libxl_ctx *ctx, uint32_t domid);
]   int libxl_tmem_thaw(libxl_ctx *ctx, uint32_t domid);
]   int libxl_tmem_set(libxl_ctx *ctx, uint32_t domid, char* name,
]                      uint32_t set);
]   int libxl_tmem_shared_auth(libxl_ctx *ctx, uint32_t domid, char* uuid,
]                              int auth);
]   int libxl_tmem_freeable(libxl_ctx *ctx);

Not sure about the tmem calls.

And from libxl_utils.h:

]   pid_t libxl_fork(libxl_ctx *ctx);

This function is going to have to go away.

-- 

Re: Xen 4.2 Release Plan / TODO

[Ian Jackson]

Ian Jackson writes ("Re: Xen 4.2 Release Plan / TODO"):
> Ian Campbell writes ("Xen 4.2 Release Plan / TODO"):
> > Plan for a 4.2 release:
> > http://lists.xen.org/archives/html/xen-devel/2012-03/msg00793.html
> ...
> > tools, blockers:
> >       * libxl stable API -- we would like 4.2 to define a stable API
> >         which downstream's can start to rely on not changing. Aspects of
> >         this are:
> 
> 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.

And I should mention that in my event series I have two
as-yet-unposted half-backed patches to rewrite libxl__spawn_* and
libxl_domain_create_*.

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.

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.

Ian.

Re: Xen 4.2 Release Plan / TODO

[Ian Campbell]

On Wed, 2012-04-11 at 17:11 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Xen 4.2 Release Plan / TODO"):
> > Plan for a 4.2 release:
> > http://lists.xen.org/archives/html/xen-devel/2012-03/msg00793.html
> ...
> > tools, blockers:
> >       * libxl stable API -- we would like 4.2 to define a stable API
> >         which downstream's can start to rely on not changing. Aspects of
> >         this are:
> 
> 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.

> 
> Ian.
> 
> 
> Other:
> ======
> 
> ]   int libxl_wait_for_free_memory(libxl_ctx *ctx, uint32_t domid, uint32_t memory_kb, int wait_secs);
> ]   /* wait for the memory target of a domain to be reached */
> ]   int libxl_wait_for_memory_target(libxl_ctx *ctx, uint32_t domid, int wait_secs);
> 
> 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.

> ]   int libxl_vncviewer_exec(libxl_ctx *ctx, uint32_t domid, int autopass);
> ]   int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num, libxl_console_type type);
> ]   /* libxl_primary_console_exec finds the domid and console number
> ]    * corresponding to the primary console of the given vm, then calls
> ]    * libxl_console_exec with the right arguments (domid might be different
> ]    * if the guest is using stubdoms).
> ]    * This function can be called after creating the device model, in
> ]    * case of HVM guests, and before libxl_run_bootloader in case of PV
> ]    * guests using pygrub. */
> ]   int libxl_primary_console_exec(libxl_ctx *ctx, uint32_t domid_vm);
> 
> 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.

> ]   /* common paths */
> ]   const char *libxl_sbindir_path(void);
> ]   const char *libxl_bindir_path(void);
> ]   const char *libxl_libexec_path(void);
> ]   const char *libxl_libdir_path(void);
> ]   const char *libxl_sharedir_path(void);
> ]   const char *libxl_private_bindir_path(void);
> ]   const char *libxl_xenfirmwaredir_path(void);
> ]   const char *libxl_xen_config_dir_path(void);
> ]   const char *libxl_xen_script_dir_path(void);
> ]   const char *libxl_lock_dir_path(void);
> ]   const char *libxl_run_dir_path(void);
> ]   const char *libxl_xenpaging_dir_path(void);
> 
> Surely these should be private ?

As far as I can grep, yes.

> Need to be ao/eventified:
> =========================
> 
> ]   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_info *info,
> ]                            uint32_t domid, int fd);
> 
> ]   typedef int (*libxl_console_ready)(libxl_ctx *ctx, uint32_t domid, void *priv);
> ]   int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config, libxl_console_ready cb, void *priv, uint32_t *domid);
> ]   int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config *d_config, libxl_console_ready cb, void *priv, uint32_t *domid, int restore_fd);

You are on the case with these?

> ]   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?)

> ]   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?

> ]   int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk);

Roger makes this async in his hotplug series.

> ]   /*
> ]    * 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_device_disk *disk);

Were you looking at this one? I know you mentioned it at one point.

> ]   /*
> ]    * Make a disk available in this (the control) domain. Returns path to
> ]    * a device.
> ]    */
> ]   char * libxl_device_disk_local_attach(libxl_ctx *ctx, libxl_device_disk *disk);
> ]   int libxl_device_disk_local_detach(libxl_ctx *ctx, libxl_device_disk *disk);
> 
> Does this even need to be public at this stage ?

I think Stefano internalises them in his qdisk/dom0-attach series.

> ]   /* Network Interfaces */
> ]   int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic);
> 
> ]   /* Keyboard */
> ]   int libxl_device_vkb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb);
> 
> ]   /* Framebuffer */
> ]   int libxl_device_vfb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb *vfb);

These ought to be pretty trivial conversions?

> ]   /* PCI Passthrough */
> ]   int libxl_device_pci_add(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev);
> ]   int libxl_device_pci_remove(libxl_ctx *ctx, uint32_t domid, libxl_device_pci *pcidev);

I'm less confident that this one will be trivial to make async :-(

> ]   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".

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.

Perhaps this interface is OK?

> ]   char *libxl_tmem_list(libxl_ctx *ctx, uint32_t domid, int use_long);
> ]   int libxl_tmem_freeze(libxl_ctx *ctx, uint32_t domid);
> ]   int libxl_tmem_destroy(libxl_ctx *ctx, uint32_t domid);
> ]   int libxl_tmem_thaw(libxl_ctx *ctx, uint32_t domid);
> ]   int libxl_tmem_set(libxl_ctx *ctx, uint32_t domid, char* name,
> ]                      uint32_t set);
> ]   int libxl_tmem_shared_auth(libxl_ctx *ctx, uint32_t domid, char* uuid,
> ]                              int auth);
> ]   int libxl_tmem_freeable(libxl_ctx *ctx);
> 
> Not sure about the tmem calls.

Me neither.

> And from libxl_utils.h:
> 
> ]   pid_t libxl_fork(libxl_ctx *ctx);
> 
> This function is going to have to go away.

Great.

Maybe things aren't as bad as I feared.

Re: Xen 4.2 Release Plan / TODO

[Ian Campbell]

On Wed, 2012-04-11 at 17:13 +0100, Ian Jackson wrote:
> Ian Jackson writes ("Re: Xen 4.2 Release Plan / TODO"):
> > Ian Campbell writes ("Xen 4.2 Release Plan / TODO"):
> > > Plan for a 4.2 release:
> > > http://lists.xen.org/archives/html/xen-devel/2012-03/msg00793.html
> > ...
> > > tools, blockers:
> > >       * libxl stable API -- we would like 4.2 to define a stable API
> > >         which downstream's can start to rely on not changing. Aspects of
> > >         this are:
> > 
> > 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.
> 
> And I should mention that in my event series I have two
> as-yet-unposted half-backed patches to rewrite libxl__spawn_* and
> libxl_domain_create_*.
> 
> 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?

> 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?

Ian.

Re: Xen 4.2 Release Plan / TODO

[Ian Campbell]

On Thu, 2012-04-12 at 08:35 +0100, Ian Campbell wrote:
> 
> > ]   char *libxl_tmem_list(libxl_ctx *ctx, uint32_t domid, int
> use_long);
> > ]   int libxl_tmem_freeze(libxl_ctx *ctx, uint32_t domid);
> > ]   int libxl_tmem_destroy(libxl_ctx *ctx, uint32_t domid);
> > ]   int libxl_tmem_thaw(libxl_ctx *ctx, uint32_t domid);
> > ]   int libxl_tmem_set(libxl_ctx *ctx, uint32_t domid, char* name,
> > ]                      uint32_t set);
> > ]   int libxl_tmem_shared_auth(libxl_ctx *ctx, uint32_t domid, char*
> uuid,
> > ]                              int auth);
> > ]   int libxl_tmem_freeable(libxl_ctx *ctx);
> > 
> > Not sure about the tmem calls.
> 
> Me neither. 

Dan,

We want to declare the libxl 4.2 API as "stable" so we are trying to
determine whether any of these functions need to be made potentially
asynchronous or not, i.e. if they may be "slow" per the definition under
the comment "Machinery for asynchronous operations ("ao")" in
tools/libxl/libxl_internal.h, effectively if they may block for extended
periods.

If they were then we would possibly want to change the API to take an
"ao_how" as described under "Some libxl operations can take a long time"
in tools/libxl/libxl.h

If they are "fast" today but could potentially be slow in the future
then we may be able to make the trivial API change but keep the
synchronous implementation (depending on the specifics). It's quite late
in the day so if the functions are "slow" then this would be the
preferred option at this stage.

Otherwise the alternative is that we have to maintain these interfaces
going forward (for compat) and perhaps be forced introduce a new
parallel async interface in the future. Annoying but not the end of the
world.

Ian.

Re: Xen 4.2 Release Plan / TODO

[Ian Campbell]

On Thu, 2012-04-12 at 08:35 +0100, Ian Campbell wrote:
> > ]   /* common paths */
> > ]   const char *libxl_sbindir_path(void);
> > ]   const char *libxl_bindir_path(void);
> > ]   const char *libxl_libexec_path(void);
> > ]   const char *libxl_libdir_path(void);
> > ]   const char *libxl_sharedir_path(void);
> > ]   const char *libxl_private_bindir_path(void);
> > ]   const char *libxl_xenfirmwaredir_path(void);
> > ]   const char *libxl_xen_config_dir_path(void);
> > ]   const char *libxl_xen_script_dir_path(void);
> > ]   const char *libxl_lock_dir_path(void);
> > ]   const char *libxl_run_dir_path(void);
> > ]   const char *libxl_xenpaging_dir_path(void);
> > 
> > Surely these should be private ?
> 
> As far as I can grep, yes. 

All but two it turns out. Not sure about those. The following patch
moves the rest.

libxl_lock_dir_path seems like it ought to be part of xl not libxl, or
at a stretch libxlu.

config_dir_path is only actually used by xl but an argument could be
made that it is more generally useful?

8<-----------------------------------------------

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1334218378 -3600
# Node ID 14cac8170e122115ef090cb390775b8c356ae643
# Parent  86b4ff3e201dc81c76ac91f8a9f134669294b3ba
libxl: make most libxl_FOO_path() functions internal.

Only libxl_xen_config_dir_path and libxl_lock_dir_path are used outside the
library. Also bindir, sbindir, sharedir and xenpagingdir appeared to be
completely unused so nuke them.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 86b4ff3e201d -r 14cac8170e12 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Thu Apr 12 09:02:05 2012 +0100
+++ b/tools/libxl/libxl.c	Thu Apr 12 09:12:58 2012 +0100
@@ -1126,7 +1126,7 @@ out:
 int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num, libxl_console_type type)
 {
     GC_INIT(ctx);
-    char *p = libxl__sprintf(gc, "%s/xenconsole", libxl_private_bindir_path());
+    char *p = libxl__sprintf(gc, "%s/xenconsole", libxl__private_bindir_path());
     char *domid_s = libxl__sprintf(gc, "%d", domid);
     char *cons_num_s = libxl__sprintf(gc, "%d", cons_num);
     char *cons_type_s;
@@ -1767,7 +1767,7 @@ int libxl__device_nic_setdefault(libxl__
         if (!nic->bridge) return ERROR_NOMEM;
     }
     if ( !nic->script && asprintf(&nic->script, "%s/vif-bridge",
-                                  libxl_xen_script_dir_path()) < 0 )
+                                  libxl__xen_script_dir_path()) < 0 )
         return ERROR_FAIL;
     if (!nic->nictype)
         nic->nictype = LIBXL_NIC_TYPE_IOEMU;
@@ -1837,7 +1837,7 @@ int libxl_device_nic_add(libxl_ctx *ctx,
         flexarray_append(back, "script");
         flexarray_append(back, nic->script[0]=='/' ? nic->script
                          : libxl__sprintf(gc, "%s/%s",
-                                          libxl_xen_script_dir_path(),
+                                          libxl__xen_script_dir_path(),
                                           nic->script));
     }
 
diff -r 86b4ff3e201d -r 14cac8170e12 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Thu Apr 12 09:02:05 2012 +0100
+++ b/tools/libxl/libxl.h	Thu Apr 12 09:12:58 2012 +0100
@@ -787,18 +787,8 @@ int libxl_flask_setenforce(libxl_ctx *ct
 int libxl_flask_loadpolicy(libxl_ctx *ctx, void *policy, uint32_t size);
 
 /* common paths */
-const char *libxl_sbindir_path(void);
-const char *libxl_bindir_path(void);
-const char *libxl_libexec_path(void);
-const char *libxl_libdir_path(void);
-const char *libxl_sharedir_path(void);
-const char *libxl_private_bindir_path(void);
-const char *libxl_xenfirmwaredir_path(void);
 const char *libxl_xen_config_dir_path(void);
-const char *libxl_xen_script_dir_path(void);
 const char *libxl_lock_dir_path(void);
-const char *libxl_run_dir_path(void);
-const char *libxl_xenpaging_dir_path(void);
 
 /* misc */
 
diff -r 86b4ff3e201d -r 14cac8170e12 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Thu Apr 12 09:02:05 2012 +0100
+++ b/tools/libxl/libxl_dm.c	Thu Apr 12 09:12:58 2012 +0100
@@ -24,7 +24,7 @@ static const char *libxl_tapif_script(li
 #ifdef __linux__
     return libxl__strdup(gc, "no");
 #else
-    return libxl__sprintf(gc, "%s/qemu-ifup", libxl_xen_script_dir_path());
+    return libxl__sprintf(gc, "%s/qemu-ifup", libxl__xen_script_dir_path());
 #endif
 }
 
@@ -47,10 +47,10 @@ const char *libxl__domain_device_model(l
     } else {
         switch (info->device_model_version) {
         case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
-            dm = libxl__abs_path(gc, "qemu-dm", libxl_libexec_path());
+            dm = libxl__abs_path(gc, "qemu-dm", libxl__libexec_path());
             break;
         case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
-            dm = libxl__abs_path(gc, "qemu-system-i386", libxl_libexec_path());
+            dm = libxl__abs_path(gc, "qemu-system-i386", libxl__libexec_path());
             break;
         default:
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
@@ -337,7 +337,7 @@ static char ** libxl__build_device_model
     flexarray_append(dm_args,
                      libxl__sprintf(gc, "socket,id=libxl-cmd,"
                                     "path=%s/qmp-libxl-%d,server,nowait",
-                                    libxl_run_dir_path(), guest_domid));
+                                    libxl__run_dir_path(), guest_domid));
 
     flexarray_append(dm_args, "-mon");
     flexarray_append(dm_args, "chardev=libxl-cmd,mode=control");
@@ -708,7 +708,7 @@ static int libxl__create_stubdom(libxl__
     dm_config.b_info.target_memkb = dm_config.b_info.max_memkb;
 
     dm_config.b_info.u.pv.kernel.path = libxl__abs_path(gc, "ioemu-stubdom.gz",
-                                              libxl_xenfirmwaredir_path());
+                                              libxl__xenfirmwaredir_path());
     dm_config.b_info.u.pv.cmdline = libxl__sprintf(gc, " -d %d", guest_domid);
     dm_config.b_info.u.pv.ramdisk.path = "";
     dm_config.b_info.u.pv.features = "";
diff -r 86b4ff3e201d -r 14cac8170e12 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Thu Apr 12 09:02:05 2012 +0100
+++ b/tools/libxl/libxl_dom.c	Thu Apr 12 09:12:58 2012 +0100
@@ -349,7 +349,7 @@ static const char *libxl__domain_firmwar
             break;
         }
     }
-    return libxl__abs_path(gc, firmware, libxl_xenfirmwaredir_path());
+    return libxl__abs_path(gc, firmware, libxl__xenfirmwaredir_path());
 }
 
 int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
diff -r 86b4ff3e201d -r 14cac8170e12 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Thu Apr 12 09:02:05 2012 +0100
+++ b/tools/libxl/libxl_internal.h	Thu Apr 12 09:12:58 2012 +0100
@@ -1374,6 +1374,14 @@ int libxl__carefd_close(libxl__carefd*);
 /* You may pass NULL in which case the answer is -1. */
 int libxl__carefd_fd(const libxl__carefd*);
 
+/* common paths */
+_hidden const char *libxl__libexec_path(void);
+_hidden const char *libxl__private_bindir_path(void);
+_hidden const char *libxl__xenfirmwaredir_path(void);
+_hidden const char *libxl__xen_config_dir_path(void);
+_hidden const char *libxl__xen_script_dir_path(void);
+_hidden const char *libxl__lock_dir_path(void);
+_hidden const char *libxl__run_dir_path(void);
 
 /*
  * Convenience macros.
diff -r 86b4ff3e201d -r 14cac8170e12 tools/libxl/libxl_paths.c
--- a/tools/libxl/libxl_paths.c	Thu Apr 12 09:02:05 2012 +0100
+++ b/tools/libxl/libxl_paths.c	Thu Apr 12 09:12:58 2012 +0100
@@ -15,37 +15,17 @@
 #include "libxl_osdeps.h" /* must come before any other headers */
 #include "libxl_internal.h"
 
-const char *libxl_sbindir_path(void)
-{
-    return SBINDIR;
-}
-
-const char *libxl_bindir_path(void)
-{
-    return BINDIR;
-}
-
-const char *libxl_libexec_path(void)
+const char *libxl__libexec_path(void)
 {
     return LIBEXEC;
 }
 
-const char *libxl_libdir_path(void)
-{
-    return LIBDIR;
-}
-
-const char *libxl_sharedir_path(void)
-{
-    return SHAREDIR;
-}
-
-const char *libxl_private_bindir_path(void)
+const char *libxl__private_bindir_path(void)
 {
     return PRIVATE_BINDIR;
 }
 
-const char *libxl_xenfirmwaredir_path(void)
+const char *libxl__xenfirmwaredir_path(void)
 {
     return XENFIRMWAREDIR;
 }
@@ -55,7 +35,7 @@ const char *libxl_xen_config_dir_path(vo
     return XEN_CONFIG_DIR;
 }
 
-const char *libxl_xen_script_dir_path(void)
+const char *libxl__xen_script_dir_path(void)
 {
     return XEN_SCRIPT_DIR;
 }
@@ -65,16 +45,11 @@ const char *libxl_lock_dir_path(void)
     return XEN_LOCK_DIR;
 }
 
-const char *libxl_run_dir_path(void)
+const char *libxl__run_dir_path(void)
 {
     return XEN_RUN_DIR;
 }
 
-const char *libxl_xenpaging_dir_path(void)
-{
-    return XEN_PAGING_DIR;
-}
-
 /*
  * Local variables:
  * mode: C
diff -r 86b4ff3e201d -r 14cac8170e12 tools/libxl/libxl_qmp.c
--- a/tools/libxl/libxl_qmp.c	Thu Apr 12 09:02:05 2012 +0100
+++ b/tools/libxl/libxl_qmp.c	Thu Apr 12 09:12:58 2012 +0100
@@ -633,7 +633,7 @@ libxl__qmp_handler *libxl__qmp_initializ
     qmp = qmp_init_handler(gc, domid);
 
     qmp_socket = libxl__sprintf(gc, "%s/qmp-libxl-%d",
-                                libxl_run_dir_path(), domid);
+                                libxl__run_dir_path(), domid);
     if ((ret = qmp_open(qmp, qmp_socket, QMP_SOCKET_CONNECT_TIMEOUT)) < 0) {
         LIBXL__LOG_ERRNO(qmp->ctx, LIBXL__LOG_ERROR, "Connection error");
         qmp_free_handler(qmp);
@@ -671,7 +671,7 @@ void libxl__qmp_cleanup(libxl__gc *gc, u
     char *qmp_socket;
 
     qmp_socket = libxl__sprintf(gc, "%s/qmp-libxl-%d",
-                                libxl_run_dir_path(), domid);
+                                libxl__run_dir_path(), domid);
     if (unlink(qmp_socket) == -1) {
         if (errno != ENOENT) {
             LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,

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

[Ian Jackson]

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.

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

[Ian Campbell]

On Thu, 2012-04-12 at 12:10 +0100, Ian Jackson wrote:
> 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.

OK, great.

> 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.

That sounds fine, do you want to propose text which you are happy with
as a patch adding a comment?

> 
> > > ]   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.

Hrm, this seems like 4.3 material at this stage.

I'd expect that the functions which behaved this way would not be
called ..._exec (perhaps ..._get_params or so) so implementing the
proper interface in 4.3 won't cause a compat issue.

> > > 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.

Any volunteers for this one? I suspect that a dummy ao_how isn't going
to cut it in this case.

What's the pattern for handling long running things which actually
happen in our process? I suppose libxl needs to start a thread or fork
or something?

> > > ]   /*
> > > ]    * 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.

I'll leave this to you then.

> > > ]   /* 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.

He also adds libxl__device_add_initiate (or so) which makes the others
look pretty trivial to convert also.

> > > ]   /* 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.

I'll have a poke at this then I think.

> > > ]   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 don't believe so.
> 
> > 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.

Phew!

Ian.

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

[Ian Jackson]

Ian Campbell writes ("Re: Xen 4.2 Release Plan / TODO [and 1 more messages]"):
> > > > ]   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.
> 
> Any volunteers for this one? I suspect that a dummy ao_how isn't going
> to cut it in this case.
> 
> What's the pattern for handling long running things which actually
> happen in our process? I suppose libxl needs to start a thread or fork
> or something?

The intended pattern in the new libxl event universe is to use event
callbacks.  So for example for outgoing migration you'd have an fd
writeable event which called into xc.  But that's not really doable in
this case because the libxc function is entirely monolithic and
synchronous.

The options for libxl are to fork, or to start a thread.

Ian.

Re: Xen 4.2 Release Plan / TODO

[Dan Magenheimer]

> From: Ian Campbell [mailto:Ian.Campbell@citrix.com]
> Sent: Thursday, April 12, 2012 1:59 AM
> To: Ian Jackson; Dan Magenheimer
> Cc: xen-devel
> Subject: Re: [Xen-devel] Xen 4.2 Release Plan / TODO
> 
> On Thu, 2012-04-12 at 08:35 +0100, Ian Campbell wrote:
> >
> > > ]   char *libxl_tmem_list(libxl_ctx *ctx, uint32_t domid, int
> > use_long);
> > > ]   int libxl_tmem_freeze(libxl_ctx *ctx, uint32_t domid);
> > > ]   int libxl_tmem_destroy(libxl_ctx *ctx, uint32_t domid);
> > > ]   int libxl_tmem_thaw(libxl_ctx *ctx, uint32_t domid);
> > > ]   int libxl_tmem_set(libxl_ctx *ctx, uint32_t domid, char* name,
> > > ]                      uint32_t set);
> > > ]   int libxl_tmem_shared_auth(libxl_ctx *ctx, uint32_t domid, char*
> > uuid,
> > > ]                              int auth);
> > > ]   int libxl_tmem_freeable(libxl_ctx *ctx);
> > >
> > > Not sure about the tmem calls.
> >
> > Me neither.
> 
> Dan,
> 
> We want to declare the libxl 4.2 API as "stable" so we are trying to
> determine whether any of these functions need to be made potentially
> asynchronous or not, i.e. if they may be "slow" per the definition under
> the comment "Machinery for asynchronous operations ("ao")" in
> tools/libxl/libxl_internal.h, effectively if they may block for extended
> periods.
> 
> If they were then we would possibly want to change the API to take an
> "ao_how" as described under "Some libxl operations can take a long time"
> in tools/libxl/libxl.h
> 
> If they are "fast" today but could potentially be slow in the future
> then we may be able to make the trivial API change but keep the
> synchronous implementation (depending on the specifics). It's quite late
> in the day so if the functions are "slow" then this would be the
> preferred option at this stage.
> 
> Otherwise the alternative is that we have to maintain these interfaces
> going forward (for compat) and perhaps be forced introduce a new
> parallel async interface in the future. Annoying but not the end of the
> world.

Hi Ian(s) --

After reading libxl.h, I'm not absolutely positive I understand
all the conditions that would cause you to label a function as
"slow" but I believe all the libxl_tmem_* functions are "fast".
All of them are strictly "call the hypervisor, wait for it to
return" and none of the hypercalls (actually which are variations of
the one tmem hypercall) require a callback to dom0 or to the
calling guest... they all complete entirely inside the hypervisor.

Libxl_tmem_destroy may take a long time as it has to walk
through and free some potentially very large data structures,
but it is only used at domain destruction.

Libxl_tmem_list does allocate some memory in userland that the
hypercall fills synchronously (with ascii-formatted statistics/counters
maintained entirely by the tmem code in the hypervisor).

If any of the above raises any alarms/concerns, let me know,
else no need to asynchronizify any of the tmem functions in libxl.

(Zhigang cc'ed since he's more familiar with the libxl layer than I.)

Dan

Re: Xen 4.2 Release Plan / TODO

[Ian Campbell]

On Thu, 2012-04-12 at 17:37 +0100, Dan Magenheimer wrote:
> > From: Ian Campbell [mailto:Ian.Campbell@citrix.com]
> > Sent: Thursday, April 12, 2012 1:59 AM
> > To: Ian Jackson; Dan Magenheimer
> > Cc: xen-devel
> > Subject: Re: [Xen-devel] Xen 4.2 Release Plan / TODO
> > 
> > On Thu, 2012-04-12 at 08:35 +0100, Ian Campbell wrote:
> > >
> > > > ]   char *libxl_tmem_list(libxl_ctx *ctx, uint32_t domid, int
> > > use_long);
> > > > ]   int libxl_tmem_freeze(libxl_ctx *ctx, uint32_t domid);
> > > > ]   int libxl_tmem_destroy(libxl_ctx *ctx, uint32_t domid);
> > > > ]   int libxl_tmem_thaw(libxl_ctx *ctx, uint32_t domid);
> > > > ]   int libxl_tmem_set(libxl_ctx *ctx, uint32_t domid, char* name,
> > > > ]                      uint32_t set);
> > > > ]   int libxl_tmem_shared_auth(libxl_ctx *ctx, uint32_t domid, char*
> > > uuid,
> > > > ]                              int auth);
> > > > ]   int libxl_tmem_freeable(libxl_ctx *ctx);
> > > >
> > > > Not sure about the tmem calls.
> > >
> > > Me neither.
> > 
> > Dan,
> > 
> > We want to declare the libxl 4.2 API as "stable" so we are trying to
> > determine whether any of these functions need to be made potentially
> > asynchronous or not, i.e. if they may be "slow" per the definition under
> > the comment "Machinery for asynchronous operations ("ao")" in
> > tools/libxl/libxl_internal.h, effectively if they may block for extended
> > periods.
> > 
> > If they were then we would possibly want to change the API to take an
> > "ao_how" as described under "Some libxl operations can take a long time"
> > in tools/libxl/libxl.h
> > 
> > If they are "fast" today but could potentially be slow in the future
> > then we may be able to make the trivial API change but keep the
> > synchronous implementation (depending on the specifics). It's quite late
> > in the day so if the functions are "slow" then this would be the
> > preferred option at this stage.
> > 
> > Otherwise the alternative is that we have to maintain these interfaces
> > going forward (for compat) and perhaps be forced introduce a new
> > parallel async interface in the future. Annoying but not the end of the
> > world.
> 
> Hi Ian(s) --
> 
> After reading libxl.h, I'm not absolutely positive I understand
> all the conditions that would cause you to label a function as
> "slow" but I believe all the libxl_tmem_* functions are "fast".
> All of them are strictly "call the hypervisor, wait for it to
> return" and none of the hypercalls (actually which are variations of
> the one tmem hypercall) require a callback to dom0 or to the
> calling guest... they all complete entirely inside the hypervisor.

OK, this sounds good, thanks.

> Libxl_tmem_destroy may take a long time as it has to walk
> through and free some potentially very large data structures,
> but it is only used at domain destruction.

Should libxl_tmem_destroy be a public function or should it solely be
called from libxl_domain_destroy? I notice that there is an xl
tmem-destroy so I presume the former?

We might consider adding a dummy ao_how to this one I suppose.

> Libxl_tmem_list does allocate some memory in userland that the
> hypercall fills synchronously (with ascii-formatted statistics/counters
> maintained entirely by the tmem code in the hypervisor).
> 
> If any of the above raises any alarms/concerns, let me know,
> else no need to asynchronizify any of the tmem functions in libxl.

It all sounds fine, I more curious about tmem_destroy than worried.

Ian.

> 
> (Zhigang cc'ed since he's more familiar with the libxl layer than I.)
> 
> Dan

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

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 1416 bytes --]

On Thu, 2012-04-12 at 12:10 +0100, Ian Jackson wrote: 
> > 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.  
>
I remember that. :-)

This is basically the reason why I said, earlier in this thread, that
warning too much about moving/reworking the locking in xl is not that
important, as it we won't be able to solve _this_ issue anyway just
playing with it.

> This is not 4.3
> material.
>
Just to be sure I get it, you think we need to have the
creation-vs-ballooning potential race solved *before* 4.2 ?

Thanks and Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

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

[Ian Campbell]

On Thu, 2012-04-12 at 22:48 +0100, Dario Faggioli wrote:
> On Thu, 2012-04-12 at 12:10 +0100, Ian Jackson wrote: 

> > This is not 4.3
> > material.
> >
> Just to be sure I get it, you think we need to have the
> creation-vs-ballooning potential race solved *before* 4.2 ?

I initially misread it as "This is not 4.2 material", which I agreed
with...

Ian.

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

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 921 bytes --]

On Fri, 2012-04-13 at 08:25 +0100, Ian Campbell wrote: 
> On Thu, 2012-04-12 at 22:48 +0100, Dario Faggioli wrote:
> > On Thu, 2012-04-12 at 12:10 +0100, Ian Jackson wrote: 
> 
> > > This is not 4.3
> > > material.
> > >
> > Just to be sure I get it, you think we need to have the
> > creation-vs-ballooning potential race solved *before* 4.2 ?
> 
> I initially misread it as "This is not 4.2 material", which I agreed
> with...
> 
Yeah, I was under that impression to, and given what follows that is
probably the case, but the possibility of a typo didn't come to my
mind... Sorry for *not* misreading this. :-P

Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

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

[Ian Jackson]

Dario Faggioli writes ("Re: [Xen-devel] Xen 4.2 Release Plan / TODO [and 1 more messages]"):
> On Thu, 2012-04-12 at 12:10 +0100, Ian Jackson wrote: 
> > This is not 4.3
> > material.
> 
> Just to be sure I get it, you think we need to have the
> creation-vs-ballooning potential race solved *before* 4.2 ?

No, I meant the opposite.  Half time I say 4.2 I mean 4.3 and vice
versa :-/.

Ian.

Re: Xen 4.2 Release Plan / TODO

[Ian Jackson]

Dan Magenheimer writes ("RE: [Xen-devel] Xen 4.2 Release Plan / TODO"):
> After reading libxl.h, I'm not absolutely positive I understand
> all the conditions that would cause you to label a function as
> "slow" but I believe all the libxl_tmem_* functions are "fast".

Thanks for your reply.

There are a few operations that make a function necessarily have to be
slow in the libxl api sense.  These are: xenstore watches; spawning
subprocesses; anything with a timeout.

More broadly any function which is sufficiently slow that a caller
might reasonably want to initiate it, and then carry on doing
something else while the function completes.  So this includes any
operation which a toolstack might want to parallelise.

> All of them are strictly "call the hypervisor, wait for it to
> return" and none of the hypercalls (actually which are variations of
> the one tmem hypercall) require a callback to dom0 or to the
> calling guest... they all complete entirely inside the hypervisor.

Right, that sounds good.  I guess you also mean that this will always
be the case.

> Libxl_tmem_destroy may take a long time as it has to walk
> through and free some potentially very large data structures,
> but it is only used at domain destruction.

How long a time are we talking about ?  Would it be a scalability or
performance problem if an entire host's management toolstack had to
block, and no other management operations could be performed on any
domain for any reason, while the tmem destroy takes place ?

> Libxl_tmem_list does allocate some memory in userland that the
> hypercall fills synchronously (with ascii-formatted statistics/counters
> maintained entirely by the tmem code in the hypervisor).

Memory allocation in userland is fine.  I guess we're not talking
about megabytes here.

Ian.

Re: Xen 4.2 Release Plan / TODO

[Dan Magenheimer]

> From: Ian Campbell [mailto:Ian.Campbell@citrix.com]
> > >
> 
> > Libxl_tmem_destroy may take a long time as it has to walk
> > through and free some potentially very large data structures,
> > but it is only used at domain destruction.
> 
> Should libxl_tmem_destroy be a public function or should it solely be
> called from libxl_domain_destroy? I notice that there is an xl
> tmem-destroy so I presume the former?
> 
> We might consider adding a dummy ao_how to this one I suppose.

Bearing in mind that I haven't poked around in this
code for over 2 years, I *think* the only reason tmem_destroy
needs to exist at all is if tmem is broken.  The normal domain
termination process destroys any persistent tmem pools and
any ephemeral tmem pools will eventually "age out" as the
last of the pools' pages fall off the end of the LRU.

I think the tmem_destroy functionality pre-dates the
existence of tmem "freeable" memory* and was a way for
a toolset to force the hypervisor to free up the hypervisor
memory used by some or all ephemeral tmem pools.  Once the
tmem allocation/free process was directly linked into
alloc_heap_pages() in the hypervisor (see call to
tmem_relinquish_pages()), this forcing function was
no longer needed.

So, bottom line, I *think* it can be ripped out, or at least
for now removed from the definition of the stable xl API/UI.
The libxl.c routine libxl_tmem_destroy() could also be
removed if you like, but I guess I'd prefer to leave the
lower level droppings in xc.c and in the hypervisor in case
I am misremembering.

* http://xenbits.xensource.com/hg/xen-unstable.hg/rev/03063e309356

Re: Xen 4.2 Release Plan / TODO

[Dan Magenheimer]

> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Subject: RE: [Xen-devel] Xen 4.2 Release Plan / TODO
> 
> Dan Magenheimer writes ("RE: [Xen-devel] Xen 4.2 Release Plan / TODO"):
> > After reading libxl.h, I'm not absolutely positive I understand
> > all the conditions that would cause you to label a function as
> > "slow" but I believe all the libxl_tmem_* functions are "fast".
> 
> There are a few operations that make a function necessarily have to be
> slow in the libxl api sense.  These are: xenstore watches; spawning
> subprocesses; anything with a timeout.
> 
> More broadly any function which is sufficiently slow that a caller
> might reasonably want to initiate it, and then carry on doing
> something else while the function completes.  So this includes any
> operation which a toolstack might want to parallelise.

Got it.  Thanks.  This is a bit clearer than the comment in libxl.h.

> > All of them are strictly "call the hypervisor, wait for it to
> > return" and none of the hypercalls (actually which are variations of
> > the one tmem hypercall) require a callback to dom0 or to the
> > calling guest... they all complete entirely inside the hypervisor.
> 
> Right, that sounds good.  I guess you also mean that this will always
> be the case.

Yes AFAICT.

> > Libxl_tmem_destroy may take a long time as it has to walk
> > through and free some potentially very large data structures,
> > but it is only used at domain destruction.
> 
> How long a time are we talking about ?  Would it be a scalability or
> performance problem if an entire host's management toolstack had to
> block, and no other management operations could be performed on any
> domain for any reason, while the tmem destroy takes place ?

See previous reply to IanC... this is moot since (I think)
tmem_destroy will go away.

> > Libxl_tmem_list does allocate some memory in userland that the
> > hypercall fills synchronously (with ascii-formatted statistics/counters
> > maintained entirely by the tmem code in the hypervisor).
> 
> Memory allocation in userland is fine.  I guess we're not talking
> about megabytes here.

A reasonable bound would be on the order of 1K per tmem-enabled guest.
The current code in pyxc_tmem_control enforces a 32K buffer limit.

Dan

Re: Xen 4.2 Release Plan / TODO

[Ian Jackson]

Dan Magenheimer writes ("RE: [Xen-devel] Xen 4.2 Release Plan / TODO"):
> > From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
...
> > There are a few operations that make a function necessarily have to be
> > slow in the libxl api sense.  These are: xenstore watches; spawning
> > subprocesses; anything with a timeout.
> > 
> > More broadly any function which is sufficiently slow that a caller
> > might reasonably want to initiate it, and then carry on doing
> > something else while the function completes.  So this includes any
> > operation which a toolstack might want to parallelise.
> 
> Got it.  Thanks.  This is a bit clearer than the comment in libxl.h.

The internal-facing documentation, which is for people who might be
implementing libxl facilities and API designers, is in
libxl_internal.h.  But the comment there is less comprehensive.  I
will prepare a patch to update it.

Ian.

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

[Ian Campbell]

> > > > ]   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.
> 
> Hrm, this seems like 4.3 material at this stage.
> 
> I'd expect that the functions which behaved this way would not be
> called ..._exec (perhaps ..._get_params or so) so implementing the
> proper interface in 4.3 won't cause a compat issue.

Just looking at this again, does this argument only really apply to
vncviewer (which is a 3rd party component)?

The other libxl_FOO_exec are both variations of execing xenconsole which
is supplied as part of the xen install and is not really subject to 3rd
party replacements (at least we can decree that they are cmdline
compatible).

Re: Xen 4.2 Release Plan / TODO

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] Xen 4.2 Release Plan / TODO"):
> libxl: make most libxl_FOO_path() functions internal.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>