xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Roger Pau Monne <roger.pau@citrix.com>
To: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>
Subject: Re: [PATCH v9 01/17] libxl: change ao_device_remove to ao_device
Date: Thu, 19 Jul 2012 16:15:13 +0100	[thread overview]
Message-ID: <50082481.60703@citrix.com> (raw)
In-Reply-To: <20486.58490.865137.374939@mariner.uk.xensource.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|
 *                                            +--------+
 */

  reply	other threads:[~2012-07-19 15:15 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-13  9:44 [PATCH v9 00/15] execute hotplug scripts from libxl Roger Pau Monne
2012-07-13  9:44 ` [PATCH v9 01/17] libxl: change ao_device_remove to ao_device Roger Pau Monne
2012-07-18 16:29   ` Ian Jackson
2012-07-19 15:15     ` Roger Pau Monne [this message]
2012-07-13  9:44 ` [PATCH v9 02/17] libxl: move device model creation prototypes Roger Pau Monne
2012-07-13  9:44 ` [PATCH v9 03/17] libxl: convert libxl_domain_destroy to an async op Roger Pau Monne
2012-07-13  9:44 ` [PATCH v9 04/17] libxl: move bootloader data strucutres and prototypes Roger Pau Monne
2012-07-13  9:44 ` [PATCH v9 05/17] libxl: refactor disk addition to take a helper Roger Pau Monne
2012-07-17 16:46   ` Ian Jackson
2012-07-20 10:38     ` Roger Pau Monne
2012-07-13  9:44 ` [PATCH v9 06/17] libxl: convert libxl__device_disk_local_attach to an async op Roger Pau Monne
2012-07-19 16:16   ` Ian Jackson
2012-07-20  9:48     ` Roger Pau Monne
2012-07-20 11:27       ` Ian Jackson
2012-07-20 12:52         ` Roger Pau Monne
2012-07-20 17:45         ` Ian Jackson
2012-07-13  9:44 ` [PATCH v9 07/17] libxl: rename vifs to nics Roger Pau Monne
2012-07-19 15:12   ` Ian Jackson
2012-07-13  9:44 ` [PATCH v9 08/17] libxl: convert libxl_device_disk_add to an async op Roger Pau Monne
2012-07-19 15:58   ` Ian Jackson
2012-07-19 16:14     ` Roger Pau Monne
2012-07-20 10:43       ` Ian Jackson
2012-07-20  9:41   ` Ian Campbell
2012-07-20 10:54     ` Roger Pau Monne
2012-07-20 11:28       ` Ian Jackson
2012-07-13  9:44 ` [PATCH v9 09/17] libxl: convert libxl_device_nic_add to an async operation Roger Pau Monne
2012-07-13  9:44 ` [PATCH v9 10/17] libxl: add option to choose who executes hotplug scripts Roger Pau Monne
2012-07-13  9:44 ` [PATCH v9 11/17] libxl: rename _IOEMU nic type to VIF_IOEMU Roger Pau Monne
2012-07-13  9:44 ` [PATCH v9 12/17] libxl: set correct nic type depending on the guest Roger Pau Monne
2012-07-19 16:29   ` Ian Jackson
2012-07-13  9:44 ` [PATCH v9 13/17] libxl: use libxl__xs_path_cleanup on device_destroy Roger Pau Monne
2012-07-13  9:44 ` [PATCH v9 14/17] libxl: call hotplug scripts for disk devices from libxl Roger Pau Monne
2012-07-13  9:44 ` [PATCH v9 15/17] libxl: call hotplug scripts for nic " Roger Pau Monne
2012-07-13  9:44 ` [PATCH v9 16/17] libxl: convert libxl_device_vkb_add to an async operation Roger Pau Monne
2012-07-19 16:35   ` Ian Jackson
2012-07-19 16:41     ` Roger Pau Monne
2012-07-20 10:49       ` Ian Jackson
2012-07-20 11:00         ` Roger Pau Monne
2012-07-13  9:44 ` [PATCH v9 17/17] libxl: convert libxl_device_vfb_add " Roger Pau Monne
2012-07-26 10:42   ` Ian Jackson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50082481.60703@citrix.com \
    --to=roger.pau@citrix.com \
    --cc=Ian.Jackson@eu.citrix.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).