[libvirt] [PATCH 0/2] virsh: man: Document quirks of detaching devices
Peter Krempa (2): virsh: man: Document quirks of device-detach and friends virsh: man: Document asynchronous behaviour of detach-device-alias tools/virsh.pod | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) -- 2.20.1
Mention that successful return does not equal to device being detached similarly as we do at the API level. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh.pod | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tools/virsh.pod b/tools/virsh.pod index 66e2bf24ec..39ff8cd7c9 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -3264,6 +3264,16 @@ or attempting to detach a device that is not present in the domain XML, but shares some specific attributes with one that is present, may lead to unexpected results. +B<Quirk>: Device unplug is asynchronous in most cases and requires guest +cooperation. This means that it's up to the discretion of the guest to disallow +or delay the unplug arbitrarily. As the libvirt API used in this command was +designed as synchronous it returns success after some timeout even if the device +was not unplugged yet to allow further interactions with the domain if the guest +e.g. if the guest is unresponsive. Callers which need to make sure that the +device was unplugged can use libvirt events (see virsh event) to be notified +when the device is removed. Note that the even may arrive before the command +returns. + If I<--live> is specified, affect a running domain. If I<--config> is specified, affect the next startup of a persistent domain. If I<--current> is specified, affect the current domain state. @@ -3311,6 +3321,8 @@ I<--persistent>. If B<--print-xml> is specified, then the XML which would be used to detach the disk is printed instead. +Please see documentation for B<detach-device> for known quirks. + =item B<detach-interface> I<domain> I<type> [I<--mac mac>] [[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]] @@ -3333,6 +3345,8 @@ an offline domain, and like I<--live> I<--config> for a running domain. Note that older versions of virsh used I<--config> as an alias for I<--persistent>. +Please see documentation for B<detach-device> for known quirks. + =item B<update-device> I<domain> I<file> [I<--force>] [[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]] -- 2.20.1
On 3/18/19 2:19 AM, Peter Krempa wrote:
Mention that successful return does not equal to device being detached similarly as we do at the API level.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh.pod | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/tools/virsh.pod b/tools/virsh.pod index 66e2bf24ec..39ff8cd7c9 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -3264,6 +3264,16 @@ or attempting to detach a device that is not present in the domain XML, but shares some specific attributes with one that is present, may lead to unexpected results.
+B<Quirk>: Device unplug is asynchronous in most cases and requires guest +cooperation. This means that it's up to the discretion of the guest to disallow +or delay the unplug arbitrarily. As the libvirt API used in this command was +designed as synchronous it returns success after some timeout even if the device +was not unplugged yet to allow further interactions with the domain if the guest +e.g. if the guest is unresponsive. Callers which need to make sure that the +device was unplugged can use libvirt events (see virsh event) to be notified +when the device is removed. Note that the even may arrive before the command
event
+returns. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
This command is fully async. Note that users can use virsh event to be notified of the guest actually removing the device. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh.pod | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index 39ff8cd7c9..2889e17531 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -3290,7 +3290,10 @@ I<--persistent>. =item B<detach-device-alias> I<domain> I<alias> [[[I<--live>] [I<--config>] | [I<--current>]]]] -Detach a device with given I<alias> from the I<domain>. +Detach a device with given I<alias> from the I<domain>. This command returns +successfully after the unplug request was sent to the hypervisor. The actual +removal of the device is notified asynchronously via libvirt events +(see virsh event). If I<--live> is specified, affect a running domain. If I<--config> is specified, affect the next startup of a persistent domain. -- 2.20.1
On 3/18/19 8:19 AM, Peter Krempa wrote:
This command is fully async. Note that users can use virsh event to be notified of the guest actually removing the device.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh.pod | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/virsh.pod b/tools/virsh.pod index 39ff8cd7c9..2889e17531 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -3290,7 +3290,10 @@ I<--persistent>. =item B<detach-device-alias> I<domain> I<alias> [[[I<--live>] [I<--config>] | [I<--current>]]]]
-Detach a device with given I<alias> from the I<domain>. +Detach a device with given I<alias> from the I<domain>. This command returns +successfully after the unplug request was sent to the hypervisor. The actual +removal of the device is notified asynchronously via libvirt events +(see virsh event).
This is not enterly true. Under the hood both detach-device and detach-device-alias call qemuDomainDetachDeviceLive() which takes virDomainDeviceDefPtr and which calls qemuDomainWaitForDeviceRemoval() after talking to the monitor. I'm failing to see why this note is here for detach-device-alias and not for plain detach-device. Michal
On Mon, Mar 18, 2019 at 13:24:35 +0100, Michal Privoznik wrote:
On 3/18/19 8:19 AM, Peter Krempa wrote:
This command is fully async. Note that users can use virsh event to be notified of the guest actually removing the device.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh.pod | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/virsh.pod b/tools/virsh.pod index 39ff8cd7c9..2889e17531 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -3290,7 +3290,10 @@ I<--persistent>. =item B<detach-device-alias> I<domain> I<alias> [[[I<--live>] [I<--config>] | [I<--current>]]]]
-Detach a device with given I<alias> from the I<domain>. +Detach a device with given I<alias> from the I<domain>. This command returns +successfully after the unplug request was sent to the hypervisor. The actual +removal of the device is notified asynchronously via libvirt events +(see virsh event).
This is not enterly true. Under the hood both detach-device and detach-device-alias call qemuDomainDetachDeviceLive() which takes virDomainDeviceDefPtr and which calls qemuDomainWaitForDeviceRemoval() after talking to the monitor.
detach-device-alias sets the @async parameter of qemuDomainDetachDeviceLive to true which very specifically skips qemuDomainWaitForDeviceRemoval in any subsequent worker thus no synchronous behaviour is done. Note that e.g. in case of memory unplug if the guest is quick enough detach-device can even report an error when the device unplug is rejected. detach-device-alias can't do that as it does not call qemuDomainWaitForDeviceRemoval contrary to what you claim above.
I'm failing to see why this note is here for detach-device-alias and not for plain detach-device.
Because the aynchronous behaviour is not expected for detach-device and thus it requires more explanation whereas detach-device-alias was properly designed as fully async.
On Mon, Mar 18, 2019 at 08:19:48AM +0100, Peter Krempa wrote:
Peter Krempa (2): virsh: man: Document quirks of device-detach and friends virsh: man: Document asynchronous behaviour of detach-device-alias
tools/virsh.pod | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) Reviewed-by: Erik Skultety <eskultet@redhat.com>
participants (4)
-
Eric Blake -
Erik Skultety -
Michal Privoznik -
Peter Krempa