Re: [PATCH v9 01/17] libxl: change ao_device_remove to ao_device

[Roger Pau Monne <roger.pau@citrix.com>]

Ian Jackson wrote:
> Roger Pau Monne writes ("[PATCH v9 01/17] libxl: change ao_device_remove to ao_device"):
>> Introduce a new structure to track state of device backends, that will
>> be used in following patches on this series.
>>
>> This structure if used for both device creation and device
>> destruction and removes libxl__ao_device_remove.
>>
>> Changes since v8:
>>
>>  * Don't wait for QDISK, VKBD or VFB to disconnect, since Qemu doesn't
>>    honour the disconnection protocol.
> 
> Following discussion in front of a whiteboard (thanks also to Ian C
> and Stefano), we have concluded that this needs to be done
> differently.  Here is the comment I promised Roger I would write
> 
> Ian.
> 
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> 
> /*
>  * Algorithm for handling device removal (including domain
>  * destruction).  This is somewhat subtle because we may already have
>  * killed the domain and caused the death of qemu.
>  *
>  * In current versions of qemu there is no mechanism for ensuring that
>  * the resources used by its devices (both emulated and any PV devices
>  * provided by qemu) are freed (eg, fds closed) before it shuts down,
>  * and no confirmation from a terminating qemu back to the toolstack.
>  *
>  * This will need to be fixed in Xen 4.3.  In the meantime (Xen 4.2)
>  * we implement a bodge.
>  *
>  *      WE WANT TO UNPLUG         WE WANT TO SHUT DOWN OR DESTROY
>  *                    |                           |
>  *                    |             LIBXL SENDS SIGHUP TO QEMU
>  *                    |      .....................|........................
>  *                    |      : XEN 4.3+ PLANNED   |                       :
>  *                    |      :      QEMU TEARS DOWN ALL DEVICES           :
>  *                    |      :      FREES RESOURCES (closing fds)         :
>  *                    |      :      SETS PV BACKENDS TO STATE 5,          :
>  *                    |      :       waits for PV frontends to shut down  :
>  *                    |      :       SETS PV BACKENDS TO STATE 6          :
>  *                    |      :                    |                       :
>  *                    |      :      QEMU NOTIFIES TOOLSTACK (via          :
>  *                    |      :       xenstore) that it is exiting         :
>  *                    |      :      QEMU EXITS (parent may be init)       :
>  *                    |      :                    |                       :
>  *                    |      :        TOOLSTACK WAITS FOR QEMU            :
>  *                    |      :        notices qemu has finished           :
>  *                    |      :....................|.......................:
>  *                    |      .--------------------'
>  *                    V      V
>  *                  for each device
>  *                 we want to unplug/remove
>  *       ..................|...........................................
>  *       :                 V                       XEN 4.2 RACY BODGE :
>  *       :      device is provided by    qemu                         :
>  *       :            |            `-----------.                      :
>  *       :   something|                        V                      :
>  *       :    else, eg|             domain (that is domain for which  :
>  *       :     blkback|              this PV device is the backend,   :
>  *       :            |              which might be the stub dm)      :
>  *       :            |                is still alive?                :
>  *       :            |                  |        |                   :
>  *       :            |                  |alive   |dead               :
>  *       :            |<-----------------'        |                   :
>  *       :            |    hopefully qemu is      |                   :
>  *       :            |       still running       |                   :
>  *       :............|.................          |                   :
>  *             ,----->|                :     we may be racing         :
>  *             |    backend state?     :      with qemu's death       :
>  *             ^      |         |      :          |                   :
>  *     xenstore|      |other    |6     :      WAIT 2.0s               :
>  *     conflict|      |         |      :       TIMEOUT                :
>  *             |   WRITE B.E.   |      :          |                   :
>  *             |    STATE:=5    |      :     hopefully qemu has       :
>  *             `---'  |         |      :      gone by now and         :
>  *                    |ok       |      :      freed its resources     :
>  *                    |         |      :          |                   :
>  *              WAIT FOR        |      :     SET B.E.                 :
>  *              STATE==6        |      :      STATE:=6                :
>  *              /     |         |      :..........|...................:
>  *      timeout/    ok|         |                 |
>  *            /       |         |                 |
>  *           |    RUN HOTPLUG <-'<----------------'
>  *           |      SCRIPT
>  *           |        |
>  *           `---> NUKE
>  *                  BACKEND
>  *                    |
>  *                   DONE.
>  */

This is the diagram comment I'm planning to add on top of the callbacks
in libxl_device.c, it contains the flow of functions used for device
plug/unplug:

/*
 * This is a general flow that describes the device plug/unplug process
 * Some functions are ommited (like _cleanup) to simplify the scheme.
 *
 *   +----------------------+
 * +->initiate_device_remove+
 * | +----------------------+---------+
 * |                                  |
 * | +---------------+ NO +-----------v-----------+
 * | |wait state == 6+----+Qemu bk && domu running|
 * | +----------+---++    +-----------+-----------+
 * |            |   |                 |YES
 * |         T/O|   |OK               |T/O 2s
 * |            |   |         +-------v-----------+
 * |            |   |         |device_qemu_timeout|
 * |            |   |         |      set state = 6|
 * |            |   |         +-------+-----------+
 * |            |   |                 |
 * |            |   |     +-----------v-----------+
+---------------+T/O
 * |            |   +----->device_backend_callback<--------+wait state
== 2+--+
 * |            |         +-----------+-----------+
OK+-------------^-+  |
 * |OK       +--v-------+             |
NO|    |
 * |force = 1|disconnect|          +--v-----------+
+-----+-+  |
 * +---------+&& !force |  +-------+device_hotplug<--+-------------+Qemu
bk|  |
 *           +---+------+  |       +--------------+  |
YES+-----^-+  |
 *             NO|         |                         |
 |    |
 *               |         |                         |
 |    |
 * +-------------+         |
|+------------------+---+|
 * | +---------------------v-+       +------------+
||wait_device_connection||
 * |++get_hotplug_script_info+------->exec_hotplug|
|+----------------------+|
 * ||+-----------------------+OK     +---+------+-+  |
      |
 * ||                                 T/O|      |    |
      |
 * ||               +--------------------+      |    |
      |
 * ||+--------------v---+  +--------------------v-+  |
      |
 * |||hotplug_timeout_cb|  |hotplug_child_death_cb+--+
      |
 * |||       kill script|  |            num_exec++|OK
      |
 * ||+------------------+  +--------------------+-+
      |
 * ||                                      error|
      |
 * ||                                           |
      |
 * ||                         +-----------------v-+
      |
 *
+>------------------------->device_hotplug_done<---------------------------+
 *   error || no script left  +--------+----------+
 *                                     |
 *                         +-----------v--------+    +-----------------+
 *                         |action == disconnect+---->rm back/front end|
 *                         +-------------------++YES ++----------------+
 *                                           NO|      |
 *                                            +v------v+
 *                                            |callback|
 *                                            +--------+
 */