[libvirt] [PATCH v3 00/12] hostdev: handle usb detach/attach on node
Diff to v2[1] version: - add 'replug' attribute for hostdev element to allow replug semantics - avoid accuiring domain lock in event loop thread on udev events as suggested by Peter - nit picks after review by Daniel Henrique Barboza * is used to mark patches that were 'Reviewed-by' by Daniel (sometimes with very minor changes to take into account new replug flag). [1] https://www.redhat.com/archives/libvir-list/2019-September/msg00321.html Nikolay Shirokovskiy (12): conf: add replug option for usb hostdev qemu: track hostdev delete intention qemu: support host usb device unplug * qemu: support usb hostdev plugging back * qemu: handle host usb device add/del udev events qemu: handle libvirtd restart after host usb device unplug * qemu: handle race on device deletion and usb host device plugging * qemu: hotplug: update device list on device deleted event qemu: handle host usb device plug/unplug when libvirtd is down * qemu: don't mess with non mandatory hostdevs on reattaching * qemu: handle detaching of unplugged hostdev conf: parse hostdev missing flag * docs/formatdomain.html.in | 10 +- docs/schemas/domaincommon.rng | 5 + src/conf/domain_conf.c | 62 +++ src/conf/domain_conf.h | 17 + src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_conf.h | 3 + src/qemu/qemu_domain.c | 2 + src/qemu/qemu_domain.h | 2 + src/qemu/qemu_driver.c | 404 +++++++++++++++++- src/qemu/qemu_hotplug.c | 104 ++++- src/qemu/qemu_hotplug.h | 3 +- src/qemu/qemu_process.c | 60 +++ src/util/virhostdev.c | 2 + tests/qemuhotplugtest.c | 2 +- tests/qemuxml2argvdata/hostdev-usb-replug.xml | 36 ++ .../qemuxml2xmloutdata/hostdev-usb-replug.xml | 40 ++ tests/qemuxml2xmltest.c | 1 + 17 files changed, 733 insertions(+), 22 deletions(-) create mode 100644 tests/qemuxml2argvdata/hostdev-usb-replug.xml create mode 100644 tests/qemuxml2xmloutdata/hostdev-usb-replug.xml -- 2.23.0
If usb device attached to a domain is unplugged from host and then plugged back then it will no longer be available in guest. We are going to support this case so that device will be detached from qemu on unplug and attached back on replug. As sometimes this behaviour is not desirable and for backcompat too let's add 'replug' option for usb hostdev. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- docs/formatdomain.html.in | 10 ++++- docs/schemas/domaincommon.rng | 5 +++ src/conf/domain_conf.c | 30 ++++++++++++++ src/conf/domain_conf.h | 2 + tests/qemuxml2argvdata/hostdev-usb-replug.xml | 36 +++++++++++++++++ .../qemuxml2xmloutdata/hostdev-usb-replug.xml | 40 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 7 files changed, 122 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/hostdev-usb-replug.xml create mode 100644 tests/qemuxml2xmloutdata/hostdev-usb-replug.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 86a5261e47..5b0d41760b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4678,7 +4678,7 @@ <pre> ... <devices> - <hostdev mode='subsystem' type='usb'> + <hostdev mode='subsystem' type='usb' replug='yes'> <source startupPolicy='optional'> <vendor id='0x1234'/> <product id='0xbeef'/> @@ -4777,7 +4777,13 @@ <dt><code>usb</code></dt> <dd>USB devices are detached from the host on guest startup and reattached after the guest exits or the device is - hot-unplugged. + hot-unplugged. If optional <code>replug</code> + (<span class="since">since 5.8.0</span>) is "yes" then libvirt + tracks USB device unplug/plug on host. On unplug the correspondent + QEMU device will be be deleted but device stays in libvirt config. + On plug the device will be added back to QEMU. This applies only + for USB devices with product/vendor pair specified. Default value + is "no". </dd> <dt><code>pci</code></dt> <dd>For PCI devices, when <code>managed</code> is "yes" it is diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index cae3be639e..8afce21deb 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4644,6 +4644,11 @@ <attribute name="type"> <value>usb</value> </attribute> + <optional> + <attribute name="replug"> + <ref name="virYesNo"/> + </attribute> + </optional> <element name="source"> <optional> <ref name="startupPolicy"/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 848c831330..db065f6a5b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7660,6 +7660,13 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, goto out; } + if (usbsrc->replug && (!got_vendor || !got_product)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("replug is only possible if vendor/product " + "pair is specified")); + goto out; + } + ret = 0; out: return ret; @@ -8113,12 +8120,14 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, virDomainHostdevSubsysSCSIPtr scsisrc = &def->source.subsys.u.scsi; virDomainHostdevSubsysSCSIVHostPtr scsihostsrc = &def->source.subsys.u.scsi_host; virDomainHostdevSubsysMediatedDevPtr mdevsrc = &def->source.subsys.u.mdev; + virDomainHostdevSubsysUSBPtr usbsrc = &def->source.subsys.u.usb; VIR_AUTOFREE(char *) managed = NULL; VIR_AUTOFREE(char *) sgio = NULL; VIR_AUTOFREE(char *) rawio = NULL; VIR_AUTOFREE(char *) backendStr = NULL; VIR_AUTOFREE(char *) model = NULL; VIR_AUTOFREE(char *) display = NULL; + VIR_AUTOFREE(char *) replug = NULL; /* @managed can be read from the xml document - it is always an * attribute of the toplevel element, no matter what type of @@ -8134,6 +8143,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, rawio = virXMLPropString(node, "rawio"); model = virXMLPropString(node, "model"); display = virXMLPropString(node, "display"); + replug = virXMLPropString(node, "replug"); /* @type is passed in from the caller rather than read from the * xml document, because it is specified in different places for @@ -8200,6 +8210,20 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, } } + if (replug) { + if (def->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("replug is only supported for usb host device")); + return -1; + } + + if ((usbsrc->replug = virTristateBoolTypeFromString(replug)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown hostdev replug setting '%s'"), rawio); + return -1; + } + } + if (def->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV && def->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST) { if (model) { @@ -27139,6 +27163,7 @@ virDomainHostdevDefFormat(virBufferPtr buf, virDomainHostdevSubsysSCSIPtr scsisrc = &def->source.subsys.u.scsi; virDomainHostdevSubsysMediatedDevPtr mdevsrc = &def->source.subsys.u.mdev; virDomainHostdevSubsysSCSIVHostPtr scsihostsrc = &def->source.subsys.u.scsi_host; + virDomainHostdevSubsysUSBPtr usbsrc = &def->source.subsys.u.usb; const char *type; if (!mode) { @@ -27203,6 +27228,11 @@ virDomainHostdevDefFormat(virBufferPtr buf, virTristateSwitchTypeToString(mdevsrc->display)); } + if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB && + usbsrc->replug) { + virBufferAsprintf(buf, " replug='%s'", + virTristateBoolTypeToString(usbsrc->replug)); + } } virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b688ee2b83..44a339ab0c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -230,6 +230,8 @@ struct _virDomainHostdevSubsysUSB { unsigned vendor; unsigned product; + + bool replug; }; struct _virDomainHostdevSubsysPCI { diff --git a/tests/qemuxml2argvdata/hostdev-usb-replug.xml b/tests/qemuxml2argvdata/hostdev-usb-replug.xml new file mode 100644 index 0000000000..9647385667 --- /dev/null +++ b/tests/qemuxml2argvdata/hostdev-usb-replug.xml @@ -0,0 +1,36 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <hostdev mode='subsystem' type='usb' managed='no' replug='yes'> + <source> + <vendor id='0x1234'/> + <product id='0xbeef'/> + </source> + </hostdev> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/hostdev-usb-replug.xml b/tests/qemuxml2xmloutdata/hostdev-usb-replug.xml new file mode 100644 index 0000000000..5f4e39e2c4 --- /dev/null +++ b/tests/qemuxml2xmloutdata/hostdev-usb-replug.xml @@ -0,0 +1,40 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <hostdev mode='subsystem' type='usb' managed='no' replug='yes'> + <source> + <vendor id='0x1234'/> + <product id='0xbeef'/> + </source> + </hostdev> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index d5c66d8791..14b94321b0 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -439,6 +439,7 @@ mymain(void) DO_TEST("channel-unix-source-path", NONE); DO_TEST("hostdev-usb-address", NONE); + DO_TEST("hostdev-usb-replug", NONE); DO_TEST("hostdev-pci-address", NONE); DO_TEST("hostdev-pci-multifunction", NONE); DO_TEST("hostdev-vfio", NONE); -- 2.23.0
On Tue, 2019-09-17 at 16:47 +0300, Nikolay Shirokovskiy wrote:
> If usb device attached to a domain is unplugged from host and
> then plugged back then it will no longer be available in guest.
> We are going to support this case so that device will be detached
> from qemu on unplug and attached back on replug. As sometimes
> this behaviour is not desirable and for backcompat too let's
> add 'replug' option for usb hostdev.
>
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
> ---
> docs/formatdomain.html.in | 10 ++++-
> docs/schemas/domaincommon.rng | 5 +++
> src/conf/domain_conf.c | 30 ++++++++++++++
> src/conf/domain_conf.h | 2 +
> tests/qemuxml2argvdata/hostdev-usb-replug.xml | 36 +++++++++++++++++
> .../qemuxml2xmloutdata/hostdev-usb-replug.xml | 40
> +++++++++++++++++++
> tests/qemuxml2xmltest.c | 1 +
> 7 files changed, 122 insertions(+), 2 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/hostdev-usb-replug.xml
> create mode 100644 tests/qemuxml2xmloutdata/hostdev-usb-replug.xml
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 86a5261e47..5b0d41760b 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -4678,7 +4678,7 @@
> <pre>
> ...
> <devices>
> - <hostdev mode='subsystem' type='usb'>
> + <hostdev mode='subsystem' type='usb' replug='yes'>
> <source startupPolicy='optional'>
> <vendor id='0x1234'/>
> <product id='0xbeef'/>
> @@ -4777,7 +4777,13 @@
> <dt><code>usb</code></dt>
> <dd>USB devices are detached from the host on guest
> startup
> and reattached after the guest exits or the device is
> - hot-unplugged.
> + hot-unplugged. If optional <code>replug</code>
> + (<span class="since">since 5.8.0</span>) is "yes" then
> libvirt
> + tracks USB device unplug/plug on host. On unplug the
> correspondent
"corresponding" is a bit better here
> + QEMU device will be be deleted but device stays in
> libvirt config.
Do we want to mention qemu explicitly here? Presumably this
configuration option could be hypervisor indepedendant? Maybe it's
better to refer generically to the "guest" or "hypervisor" rather than
qemu?
> + On plug the device will be added back to QEMU. This
> applies only
> + for USB devices with product/vendor pair specified.
> Default value
> + is "no".
> </dd>
> <dt><code>pci</code></dt>
> <dd>For PCI devices, when <code>managed</code> is "yes" it
> is
> diff --git a/docs/schemas/domaincommon.rng
> b/docs/schemas/domaincommon.rng
> index cae3be639e..8afce21deb 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -4644,6 +4644,11 @@
> <attribute name="type">
> <value>usb</value>
> </attribute>
> + <optional>
> + <attribute name="replug">
> + <ref name="virYesNo"/>
> + </attribute>
> + </optional>
> <element name="source">
> <optional>
> <ref name="startupPolicy"/>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 848c831330..db065f6a5b 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -7660,6 +7660,13 @@
> virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
> goto out;
> }
>
> + if (usbsrc->replug && (!got_vendor || !got_product)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("replug is only possible if vendor/product
> "
> + "pair is specified"));
> + goto out;
> + }
> +
> ret = 0;
> out:
> return ret;
> @@ -8113,12 +8120,14 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr
> node,
> virDomainHostdevSubsysSCSIPtr scsisrc = &def-
> >source.subsys.u.scsi;
> virDomainHostdevSubsysSCSIVHostPtr scsihostsrc = &def-
> >source.subsys.u.scsi_host;
> virDomainHostdevSubsysMediatedDevPtr mdevsrc = &def-
> >source.subsys.u.mdev;
> + virDomainHostdevSubsysUSBPtr usbsrc = &def->source.subsys.u.usb;
> VIR_AUTOFREE(char *) managed = NULL;
> VIR_AUTOFREE(char *) sgio = NULL;
> VIR_AUTOFREE(char *) rawio = NULL;
> VIR_AUTOFREE(char *) backendStr = NULL;
> VIR_AUTOFREE(char *) model = NULL;
> VIR_AUTOFREE(char *) display = NULL;
> + VIR_AUTOFREE(char *) replug = NULL;
>
> /* @managed can be read from the xml document - it is always an
> * attribute of the toplevel element, no matter what type of
> @@ -8134,6 +8143,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr
> node,
> rawio = virXMLPropString(node, "rawio");
> model = virXMLPropString(node, "model");
> display = virXMLPropString(node, "display");
> + replug = virXMLPropString(node, "replug");
>
> /* @type is passed in from the caller rather than read from the
> * xml document, because it is specified in different places for
> @@ -8200,6 +8210,20 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr
> node,
> }
> }
>
> + if (replug) {
> + if (def->source.subsys.type !=
> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("replug is only supported for usb host
> device"));
> + return -1;
> + }
> +
> + if ((usbsrc->replug = virTristateBoolTypeFromString(replug))
> <= 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("unknown hostdev replug setting '%s'"),
> rawio);
> + return -1;
> + }
> + }
> +
> if (def->source.subsys.type !=
> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV &&
> def->source.subsys.type !=
> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST) {
> if (model) {
> @@ -27139,6 +27163,7 @@ virDomainHostdevDefFormat(virBufferPtr buf,
> virDomainHostdevSubsysSCSIPtr scsisrc = &def-
> >source.subsys.u.scsi;
> virDomainHostdevSubsysMediatedDevPtr mdevsrc = &def-
> >source.subsys.u.mdev;
> virDomainHostdevSubsysSCSIVHostPtr scsihostsrc = &def-
> >source.subsys.u.scsi_host;
> + virDomainHostdevSubsysUSBPtr usbsrc = &def->source.subsys.u.usb;
> const char *type;
>
> if (!mode) {
> @@ -27203,6 +27228,11 @@ virDomainHostdevDefFormat(virBufferPtr buf,
> virTristateSwitchTypeToString(mdev
> src->display));
> }
>
> + if (def->source.subsys.type ==
> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB &&
> + usbsrc->replug) {
> + virBufferAsprintf(buf, " replug='%s'",
> + virTristateBoolTypeToString(usbsrc-
> >replug));
> + }
> }
> virBufferAddLit(buf, ">\n");
> virBufferAdjustIndent(buf, 2);
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index b688ee2b83..44a339ab0c 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -230,6 +230,8 @@ struct _virDomainHostdevSubsysUSB {
>
> unsigned vendor;
> unsigned product;
> +
> + bool replug;
> };
>
> struct _virDomainHostdevSubsysPCI {
> diff --git a/tests/qemuxml2argvdata/hostdev-usb-replug.xml
> b/tests/qemuxml2argvdata/hostdev-usb-replug.xml
> new file mode 100644
> index 0000000000..9647385667
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/hostdev-usb-replug.xml
> @@ -0,0 +1,36 @@
> +<domain type='qemu'>
> + <name>QEMUGuest1</name>
> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> + <memory unit='KiB'>219136</memory>
> + <currentMemory unit='KiB'>219136</currentMemory>
> + <vcpu placement='static'>1</vcpu>
> + <os>
> + <type arch='i686' machine='pc'>hvm</type>
> + <boot dev='hd'/>
> + </os>
> + <clock offset='utc'/>
> + <on_poweroff>destroy</on_poweroff>
> + <on_reboot>restart</on_reboot>
> + <on_crash>destroy</on_crash>
> + <devices>
> + <emulator>/usr/bin/qemu-system-i686</emulator>
> + <disk type='block' device='disk'>
> + <driver name='qemu' type='raw'/>
> + <source dev='/dev/HostVG/QEMUGuest1'/>
> + <target dev='hda' bus='ide'/>
> + <address type='drive' controller='0' bus='0' target='0'
> unit='0'/>
> + </disk>
> + <controller type='usb' index='0'/>
> + <controller type='ide' index='0'/>
> + <controller type='pci' index='0' model='pci-root'/>
> + <input type='mouse' bus='ps2'/>
> + <input type='keyboard' bus='ps2'/>
> + <hostdev mode='subsystem' type='usb' managed='no' replug='yes'>
> + <source>
> + <vendor id='0x1234'/>
> + <product id='0xbeef'/>
> + </source>
> + </hostdev>
> + <memballoon model='none'/>
> + </devices>
> +</domain>
> diff --git a/tests/qemuxml2xmloutdata/hostdev-usb-replug.xml
> b/tests/qemuxml2xmloutdata/hostdev-usb-replug.xml
> new file mode 100644
> index 0000000000..5f4e39e2c4
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/hostdev-usb-replug.xml
> @@ -0,0 +1,40 @@
> +<domain type='qemu'>
> + <name>QEMUGuest1</name>
> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> + <memory unit='KiB'>219136</memory>
> + <currentMemory unit='KiB'>219136</currentMemory>
> + <vcpu placement='static'>1</vcpu>
> + <os>
> + <type arch='i686' machine='pc'>hvm</type>
> + <boot dev='hd'/>
> + </os>
> + <clock offset='utc'/>
> + <on_poweroff>destroy</on_poweroff>
> + <on_reboot>restart</on_reboot>
> + <on_crash>destroy</on_crash>
> + <devices>
> + <emulator>/usr/bin/qemu-system-i686</emulator>
> + <disk type='block' device='disk'>
> + <driver name='qemu' type='raw'/>
> + <source dev='/dev/HostVG/QEMUGuest1'/>
> + <target dev='hda' bus='ide'/>
> + <address type='drive' controller='0' bus='0' target='0'
> unit='0'/>
> + </disk>
> + <controller type='usb' index='0'>
> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01'
> function='0x2'/>
> + </controller>
> + <controller type='ide' index='0'>
> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01'
> function='0x1'/>
> + </controller>
> + <controller type='pci' index='0' model='pci-root'/>
> + <input type='mouse' bus='ps2'/>
> + <input type='keyboard' bus='ps2'/>
> + <hostdev mode='subsystem' type='usb' managed='no' replug='yes'>
> + <source>
> + <vendor id='0x1234'/>
> + <product id='0xbeef'/>
> + </source>
> + </hostdev>
> + <memballoon model='none'/>
> + </devices>
> +</domain>
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index d5c66d8791..14b94321b0 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -439,6 +439,7 @@ mymain(void)
> DO_TEST("channel-unix-source-path", NONE);
>
> DO_TEST("hostdev-usb-address", NONE);
> + DO_TEST("hostdev-usb-replug", NONE);
> DO_TEST("hostdev-pci-address", NONE);
> DO_TEST("hostdev-pci-multifunction", NONE);
> DO_TEST("hostdev-vfio", NONE);
On 09.10.2019 20:02, Jonathon Jongsma wrote:
On Tue, 2019-09-17 at 16:47 +0300, Nikolay Shirokovskiy wrote:
If usb device attached to a domain is unplugged from host and then plugged back then it will no longer be available in guest. We are going to support this case so that device will be detached from qemu on unplug and attached back on replug. As sometimes this behaviour is not desirable and for backcompat too let's add 'replug' option for usb hostdev.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- docs/formatdomain.html.in | 10 ++++- docs/schemas/domaincommon.rng | 5 +++ src/conf/domain_conf.c | 30 ++++++++++++++ src/conf/domain_conf.h | 2 + tests/qemuxml2argvdata/hostdev-usb-replug.xml | 36 +++++++++++++++++ .../qemuxml2xmloutdata/hostdev-usb-replug.xml | 40 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 7 files changed, 122 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/hostdev-usb-replug.xml create mode 100644 tests/qemuxml2xmloutdata/hostdev-usb-replug.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 86a5261e47..5b0d41760b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4678,7 +4678,7 @@ <pre> ... <devices> - <hostdev mode='subsystem' type='usb'> + <hostdev mode='subsystem' type='usb' replug='yes'> <source startupPolicy='optional'> <vendor id='0x1234'/> <product id='0xbeef'/> @@ -4777,7 +4777,13 @@ <dt><code>usb</code></dt> <dd>USB devices are detached from the host on guest startup and reattached after the guest exits or the device is - hot-unplugged. + hot-unplugged. If optional <code>replug</code> + (<span class="since">since 5.8.0</span>) is "yes" then libvirt + tracks USB device unplug/plug on host. On unplug the correspondent
"corresponding" is a bit better here
+ QEMU device will be be deleted but device stays in libvirt config.
Do we want to mention qemu explicitly here? Presumably this configuration option could be hypervisor indepedendant? Maybe it's better to refer generically to the "guest" or "hypervisor" rather than qemu?
Yeah, makes sense. Nikolay
So we are going to support replug of usb device on host. We need to delete device from qemu when the device is unplugged on host and to add device when the device is plugged back. As to deleting let's use same code as when we detach device from domain. But we need to keep the usb hostdev etc in libvirt config when handle DEVICE_DELETED event from qemu. For this purpose let's save delete intention in device config. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/conf/domain_conf.h | 14 ++++++++++++++ src/qemu/qemu_driver.c | 4 ++-- src/qemu/qemu_hotplug.c | 23 ++++++++++++++++++++--- src/qemu/qemu_hotplug.h | 3 ++- tests/qemuhotplugtest.c | 2 +- 5 files changed, 39 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 44a339ab0c..da005e5423 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -328,6 +328,19 @@ struct _virDomainHostdevCaps { } u; }; +typedef enum { + VIR_DOMAIN_HOSTDEV_DELETE_ACTION_NONE = 0, + /* delete associated device from libvirt config + * as intended by client API call */ + VIR_DOMAIN_HOSTDEV_DELETE_ACTION_DELETE, + /* keep associated device in libvirt config as + * qemu device is deleted as a result of unplugging + * device from host */ + VIR_DOMAIN_HOSTDEV_DELETE_ACTION_UNPLUG, + + VIR_DOMAIN_HOSTDEV_DELETE_ACTION_LAST +} virDomainHostdevDeleteActionType; + /* basic device for direct passthrough */ struct _virDomainHostdevDef { @@ -345,6 +358,7 @@ struct _virDomainHostdevDef { bool missing; bool readonly; bool shareable; + virDomainHostdevDeleteActionType deleteAction; union { virDomainHostdevSubsys subsys; virDomainHostdevCaps caps; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1e041a8bac..c208591133 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9144,7 +9144,7 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr driver, if (flags & VIR_DOMAIN_AFFECT_LIVE) { int rc; - if ((rc = qemuDomainDetachDeviceLive(vm, dev_copy, driver, false)) < 0) + if ((rc = qemuDomainDetachDeviceLive(vm, dev_copy, driver, false, false)) < 0) goto cleanup; if (rc == 0 && qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) @@ -9233,7 +9233,7 @@ qemuDomainDetachDeviceAliasLiveAndConfig(virQEMUDriverPtr driver, if (virDomainDefFindDevice(def, alias, &dev, true) < 0) goto cleanup; - if ((rc = qemuDomainDetachDeviceLive(vm, &dev, driver, true)) < 0) + if ((rc = qemuDomainDetachDeviceLive(vm, &dev, driver, true, false)) < 0) goto cleanup; if (rc == 0 && qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index cf0d46fee9..5b65963f86 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -5451,7 +5451,8 @@ qemuDomainDetachPrepController(virDomainObjPtr vm, static int qemuDomainDetachPrepHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr match, - virDomainHostdevDefPtr *detach) + virDomainHostdevDefPtr *detach, + bool unplug) { virDomainHostdevSubsysPtr subsys = &match->source.subsys; virDomainHostdevSubsysUSBPtr usbsrc = &subsys->u.usb; @@ -5523,6 +5524,20 @@ qemuDomainDetachPrepHostdev(virDomainObjPtr vm, return -1; } + /* + * Why having additional check in second branch? Suppose client + * asks for device detaching and we delete device from qemu + * but don't get DEVICE_DELETED event yet. Next USB is unplugged + * from host and we have this function called again. If we reset + * delete action to 'unplug' then device will be left in + * libvirt config after handling DEVICE_DELETED event while + * it should not as client asked to detach the device before. + */ + if (!unplug) + hostdev->deleteAction = VIR_DOMAIN_HOSTDEV_DELETE_ACTION_DELETE; + else if (hostdev->deleteAction != VIR_DOMAIN_HOSTDEV_DELETE_ACTION_DELETE) + hostdev->deleteAction = VIR_DOMAIN_HOSTDEV_DELETE_ACTION_UNPLUG; + return 0; } @@ -5824,7 +5839,8 @@ int qemuDomainDetachDeviceLive(virDomainObjPtr vm, virDomainDeviceDefPtr match, virQEMUDriverPtr driver, - bool async) + bool async, + bool unplug) { virDomainDeviceDef detach = { .type = match->type }; virDomainDeviceInfoPtr info = NULL; @@ -5869,7 +5885,8 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm, break; case VIR_DOMAIN_DEVICE_HOSTDEV: if (qemuDomainDetachPrepHostdev(vm, match->data.hostdev, - &detach.data.hostdev) < 0) { + &detach.data.hostdev, + unplug) < 0) { return -1; } break; diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 6d2cd34dbc..647f617355 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -116,7 +116,8 @@ int qemuDomainAttachRNGDevice(virQEMUDriverPtr driver, int qemuDomainDetachDeviceLive(virDomainObjPtr vm, virDomainDeviceDefPtr match, virQEMUDriverPtr driver, - bool async); + bool async, + bool unplug); void qemuDomainRemoveVcpuAlias(virQEMUDriverPtr driver, virDomainObjPtr vm, diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 3c177c6622..4e7851aa62 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -158,7 +158,7 @@ testQemuHotplugDetach(virDomainObjPtr vm, case VIR_DOMAIN_DEVICE_SHMEM: case VIR_DOMAIN_DEVICE_WATCHDOG: case VIR_DOMAIN_DEVICE_HOSTDEV: - ret = qemuDomainDetachDeviceLive(vm, dev, &driver, async); + ret = qemuDomainDetachDeviceLive(vm, dev, &driver, async, false); break; default: VIR_TEST_VERBOSE("device type '%s' cannot be detached", -- 2.23.0
Handle host usb device unplug in DEVICE_DELETED handle execution path. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_hotplug.c | 38 +++++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 5b65963f86..121e4b2821 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4465,7 +4465,8 @@ qemuDomainRemoveUSBHostDevice(virQEMUDriverPtr driver, virDomainHostdevDefPtr hostdev) { qemuHostdevReAttachUSBDevices(driver, vm->def->name, &hostdev, 1); - qemuDomainReleaseDeviceAddress(vm, hostdev->info); + if (hostdev->deleteAction != VIR_DOMAIN_HOSTDEV_DELETE_ACTION_UNPLUG) + qemuDomainReleaseDeviceAddress(vm, hostdev->info); } static void @@ -4507,6 +4508,7 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, char *drivealias = NULL; char *objAlias = NULL; bool is_vfio = false; + bool unplug = hostdev->deleteAction == VIR_DOMAIN_HOSTDEV_DELETE_ACTION_UNPLUG; VIR_DEBUG("Removing host device %s from domain %p %s", hostdev->info->alias, vm, vm->def->name); @@ -4554,16 +4556,24 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, } } - for (i = 0; i < vm->def->nhostdevs; i++) { - if (vm->def->hostdevs[i] == hostdev) { - virDomainHostdevRemove(vm->def, i); - break; + if (!unplug) { + for (i = 0; i < vm->def->nhostdevs; i++) { + if (vm->def->hostdevs[i] == hostdev) { + virDomainHostdevRemove(vm->def, i); + break; + } } } virDomainAuditHostdev(vm, hostdev, "detach", true); - if (!is_vfio && + /* + * In case of unplug the attempt to restore label will fail. But we don't + * need to restore the label! In case of separate mount namespace for the + * domain we remove device file later in this function. In case of global + * mount namespace the device file is deleted or being deleted by systemd. + */ + if (!is_vfio && !unplug && qemuSecurityRestoreHostdevLabel(driver, vm, hostdev) < 0) VIR_WARN("Failed to restore host device labelling"); @@ -4597,7 +4607,13 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, break; } - virDomainHostdevDefFree(hostdev); + if (unplug) { + virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb; + usbsrc->bus = 0; + usbsrc->device = 0; + } else { + virDomainHostdevDefFree(hostdev); + } if (net) { if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { @@ -4612,6 +4628,8 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, virDomainNetDefFree(net); } + hostdev->deleteAction = VIR_DOMAIN_HOSTDEV_DELETE_ACTION_NONE; + ret = 0; cleanup: @@ -5084,6 +5102,7 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver, virDomainDeviceInfoPtr info; virObjectEventPtr event; VIR_AUTOFREE(char *) alias = NULL; + bool unplug; /* * save the alias to use when sending a DEVICE_REMOVED event after @@ -5124,8 +5143,13 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver, return -1; break; case VIR_DOMAIN_DEVICE_HOSTDEV: + unplug = dev->data.hostdev->deleteAction == VIR_DOMAIN_HOSTDEV_DELETE_ACTION_UNPLUG; + if (qemuDomainRemoveHostDevice(driver, vm, dev->data.hostdev) < 0) return -1; + + if (unplug) + return 0; break; case VIR_DOMAIN_DEVICE_RNG: if (qemuDomainRemoveRNGDevice(driver, vm, dev->data.rng) < 0) -- 2.23.0
We are going to use qemuDomainAttachHostUSBDevice when host usb device is plugged back to node. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_hotplug.c | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 121e4b2821..780bfde002 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2524,8 +2524,18 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver, bool teardownlabel = false; bool teardowndevice = false; int ret = -1; + bool replug = false; + size_t i; + + for (i = 0; i < vm->def->nhostdevs; i++) { + if (vm->def->hostdevs[i] == hostdev) { + replug = true; + break; + } + } - if (virDomainUSBAddressEnsure(priv->usbaddrs, hostdev->info) < 0) + if (!replug && + virDomainUSBAddressEnsure(priv->usbaddrs, hostdev->info) < 0) return -1; if (qemuHostdevPrepareUSBDevices(driver, vm->def->name, &hostdev, 1, 0) < 0) @@ -2550,7 +2560,7 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver, if (!(devstr = qemuBuildUSBHostdevDevStr(vm->def, hostdev, priv->qemuCaps))) goto cleanup; - if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs+1) < 0) + if (!replug && VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs+1) < 0) goto cleanup; qemuDomainObjEnterMonitor(driver, vm); @@ -2563,7 +2573,8 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver, if (ret < 0) goto cleanup; - vm->def->hostdevs[vm->def->nhostdevs++] = hostdev; + if (!replug) + vm->def->hostdevs[vm->def->nhostdevs++] = hostdev; ret = 0; cleanup: @@ -2576,9 +2587,17 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver, if (teardowndevice && qemuDomainNamespaceTeardownHostdev(vm, hostdev) < 0) VIR_WARN("Unable to remove host device from /dev"); - if (added) + if (added) { qemuHostdevReAttachUSBDevices(driver, vm->def->name, &hostdev, 1); - virDomainUSBAddressRelease(priv->usbaddrs, hostdev->info); + + if (replug) { + virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb; + usbsrc->bus = 0; + usbsrc->device = 0; + } + } + if (!replug) + virDomainUSBAddressRelease(priv->usbaddrs, hostdev->info); } VIR_FREE(devstr); return ret; -- 2.23.0
Now when code handling attaching/detaching usb hostdev is appropriately changed use it to handle host usb device udev add/del events. As device adding/deleting needs acquiring job condition it is done in thread pool rather in event loop thread itself where udev events are delivered. However implementation in this patch is a bit different from other places as we instead of finding right domain in event loop thread and creating a single thread pool job create job for every vm. Such a strategy allows us to avoid taking domain lock in event loop thread so we have little chances to block event loop thread because some API handler grabs domain lock for a significant amount of time. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_conf.h | 3 + src/qemu/qemu_domain.c | 2 + src/qemu/qemu_domain.h | 2 + src/qemu/qemu_driver.c | 324 ++++++++++++++++++++++++++++++++++++++- 5 files changed, 332 insertions(+), 1 deletion(-) diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am index 48fd0332ec..26a686e56f 100644 --- a/src/qemu/Makefile.inc.am +++ b/src/qemu/Makefile.inc.am @@ -89,6 +89,7 @@ libvirt_driver_qemu_impl_la_CFLAGS = \ -I$(srcdir)/conf \ -I$(srcdir)/secret \ $(AM_CFLAGS) \ + $(UDEV_CFLAGS) \ $(NULL) libvirt_driver_qemu_impl_la_LDFLAGS = $(AM_LDFLAGS) libvirt_driver_qemu_impl_la_LIBADD = \ @@ -97,6 +98,7 @@ libvirt_driver_qemu_impl_la_LIBADD = \ $(LIBNL_LIBS) \ $(SELINUX_LIBS) \ $(LIBXML_LIBS) \ + $(UDEV_LIBS) \ $(NULL) libvirt_driver_qemu_impl_la_SOURCES = $(QEMU_DRIVER_SOURCES) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 8473d6d4ca..f78ebcd829 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -296,6 +296,9 @@ struct _virQEMUDriver { /* Immutable pointer, self-locking APIs */ virHashAtomicPtr migrationErrors; + + struct udev_monitor *udev_monitor; + int udev_watch; }; virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ad4db7e881..711f562754 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -15401,6 +15401,8 @@ qemuProcessEventFree(struct qemuProcessEvent *event) case QEMU_PROCESS_EVENT_SERIAL_CHANGED: case QEMU_PROCESS_EVENT_BLOCK_JOB: case QEMU_PROCESS_EVENT_MONITOR_EOF: + case QEMU_PROCESS_EVENT_USB_REMOVED: + case QEMU_PROCESS_EVENT_USB_ADDED: VIR_FREE(event->data); break; case QEMU_PROCESS_EVENT_JOB_STATUS_CHANGE: diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index f53ea146e1..abd717c025 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -537,6 +537,8 @@ typedef enum { QEMU_PROCESS_EVENT_MONITOR_EOF, QEMU_PROCESS_EVENT_PR_DISCONNECT, QEMU_PROCESS_EVENT_RDMA_GID_STATUS_CHANGED, + QEMU_PROCESS_EVENT_USB_REMOVED, + QEMU_PROCESS_EVENT_USB_ADDED, QEMU_PROCESS_EVENT_LAST } qemuProcessEventType; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c208591133..51f1d5d4a2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -34,6 +34,7 @@ #include <sys/ioctl.h> #include <sys/un.h> #include <byteswap.h> +#include <libudev.h> #include "qemu_driver.h" @@ -719,6 +720,221 @@ qemuDomainFindMaxID(virDomainObjPtr vm, } +struct qemuUdevUSBRemoveData { + unsigned int bus; + unsigned int device; +}; + +struct qemuUdevUSBAddData { + unsigned int vendor; + unsigned int product; +}; + +struct qemuUdevUSBEventData { + union { + struct qemuUdevUSBRemoveData remove; + struct qemuUdevUSBAddData add; + } data; + bool remove; +}; + +static int +qemuUdevUSBHandleEvent(virDomainObjPtr vm, void *opaque) +{ + struct qemuUdevUSBEventData *data = opaque; + struct qemuProcessEvent *event = NULL; + + if (VIR_ALLOC(event) < 0) + return 0; + + if (data->remove) { + struct qemuUdevUSBRemoveData *rm_data; + + + if (VIR_ALLOC(rm_data) < 0) + goto cleanup; + + *rm_data = data->data.remove; + event->data = rm_data; + event->eventType = QEMU_PROCESS_EVENT_USB_REMOVED; + } else { + struct qemuUdevUSBAddData *add_data; + + if (VIR_ALLOC(add_data) < 0) + goto cleanup; + + *add_data = data->data.add; + event->data = add_data; + event->eventType = QEMU_PROCESS_EVENT_USB_ADDED; + } + + event->vm = virObjectRef(vm); + + if (virThreadPoolSendJob(qemu_driver->workerPool, 0, event) < 0) { + virObjectUnref(vm); + goto cleanup; + } + + event = NULL; + + cleanup: + qemuProcessEventFree(event); + + return 0; +} + + +static void +qemuUdevEventHandleCallback(int watch ATTRIBUTE_UNUSED, + int fd ATTRIBUTE_UNUSED, + int events ATTRIBUTE_UNUSED, + void *data ATTRIBUTE_UNUSED) +{ + struct qemuUdevUSBEventData event_data; + struct udev_device *dev = NULL; + const char *action; + const char *devtype; + const char *tmp; + + /* libvirtd daemon do not run event loop before full state drivers + * initialization. Also state drivers uninitialized only after + * full stop of event loop. In short driver initialization/uninitialization + * and handling events occurs in same main loop thread. Thus we + * don't need any locking here. */ + + if (!(dev = udev_monitor_receive_device(qemu_driver->udev_monitor))) { + VIR_WARNINGS_NO_WLOGICALOP_EQUAL_EXPR + if (errno == EAGAIN || errno == EWOULDBLOCK) { + VIR_WARNINGS_RESET + return; + } + + virReportSystemError(errno, "%s", + _("failed to receive device from udev monitor")); + return; + } + + devtype = udev_device_get_devtype(dev); + + if (STRNEQ_NULLABLE(devtype, "usb_device")) + goto cleanup; + + if (!(action = udev_device_get_action(dev))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to receive action from udev monitor")); + goto cleanup; + } + + if (STREQ(action, "remove")) { + struct qemuUdevUSBRemoveData *rm_data = &event_data.data.remove; + + if (!(tmp = udev_device_get_property_value(dev, "BUSNUM"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to receive busnum from udev monitor")); + goto cleanup; + } + if (virStrToLong_ui(tmp, NULL, 10, &rm_data->bus) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to convert busnum to int")); + goto cleanup; + } + + if (!(tmp = udev_device_get_property_value(dev, "DEVNUM"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to receive devnum from udev monitor")); + goto cleanup; + } + if (virStrToLong_ui(tmp, NULL, 10, &rm_data->device) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to convert devnum to int")); + goto cleanup; + } + event_data.remove = true; + } else if (STREQ(action, "add")) { + struct qemuUdevUSBAddData *add_data = &event_data.data.add; + + if (!(tmp = udev_device_get_property_value(dev, "ID_VENDOR_ID"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to receive vendor from udev monitor")); + goto cleanup; + } + if (virStrToLong_ui(tmp, NULL, 16, &add_data->vendor) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to convert vendor to int")); + goto cleanup; + } + + if (!(tmp = udev_device_get_property_value(dev, "ID_MODEL_ID"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to receive product from udev monitor")); + goto cleanup; + } + if (virStrToLong_ui(tmp, NULL, 16, &add_data->product) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to convert product to int")); + goto cleanup; + } + event_data.remove = false; + } else { + goto cleanup; + } + + virDomainObjListForEach(qemu_driver->domains, false, qemuUdevUSBHandleEvent, &event_data); + + cleanup: + udev_device_unref(dev); +} + + +static int +qemuUdevInitialize(void) +{ + struct udev *udev; + + if (!(udev = udev_new())) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to create udev context")); + return -1; + } + + if (!(qemu_driver->udev_monitor = udev_monitor_new_from_netlink(udev, "udev"))) { + udev_unref(udev); + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("udev_monitor_new_from_netlink returned NULL")); + return -1; + } + + udev_monitor_enable_receiving(qemu_driver->udev_monitor); + + qemu_driver->udev_watch = virEventAddHandle(udev_monitor_get_fd(qemu_driver->udev_monitor), + VIR_EVENT_HANDLE_READABLE, + qemuUdevEventHandleCallback, NULL, NULL); + + if (qemu_driver->udev_watch < 0) + return -1; + + return 0; +} + + +static void +qemuUdevCleanup(void) +{ + if (qemu_driver->udev_monitor) { + struct udev *udev = udev_monitor_get_udev(qemu_driver->udev_monitor); + + udev_monitor_unref(qemu_driver->udev_monitor); + udev_unref(udev); + qemu_driver->udev_monitor = NULL; + } + + if (qemu_driver->udev_watch > 0) { + virEventRemoveHandle(qemu_driver->udev_watch); + qemu_driver->udev_watch = 0; + } +} + + /** * qemuStateInitialize: * @@ -1042,6 +1258,9 @@ qemuStateInitialize(bool privileged, if (!(qemu_driver->closeCallbacks = virCloseCallbacksNew())) goto error; + if (qemuUdevInitialize() < 0) + goto error; + /* Get all the running persistent or transient configs first */ if (virDomainObjListLoadAllConfigs(qemu_driver->domains, cfg->stateDir, @@ -1256,6 +1475,8 @@ qemuStateCleanup(void) virLockManagerPluginUnref(qemu_driver->lockManager); + qemuUdevCleanup(); + virMutexDestroy(&qemu_driver->lock); VIR_FREE(qemu_driver); @@ -5028,7 +5249,102 @@ processRdmaGidStatusChangedEvent(virDomainObjPtr vm, } -static void qemuProcessEventHandler(void *data, void *opaque) +static void +processUSBAddedEvent(virQEMUDriverPtr driver, + virDomainObjPtr vm, + struct qemuUdevUSBAddData *data) +{ + virDomainHostdevDefPtr hostdev; + virDomainHostdevSubsysUSBPtr usbsrc; + size_t i; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + return; + + if (!virDomainObjIsActive(vm)) { + VIR_DEBUG("Domain is not running"); + goto cleanup; + } + + for (i = 0; i < vm->def->nhostdevs; i++) { + hostdev = vm->def->hostdevs[i]; + + if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) + continue; + + usbsrc = &hostdev->source.subsys.u.usb; + + if (!usbsrc->replug) + continue; + + if (usbsrc->vendor == data->vendor && usbsrc->product == data->product) + break; + } + + if (i == vm->def->nhostdevs) + goto cleanup; + + if (qemuDomainAttachHostDevice(driver, vm, hostdev) < 0) + goto cleanup; + + cleanup: + qemuDomainObjEndJob(driver, vm); +} + + +static void +processUSBRemovedEvent(virQEMUDriverPtr driver, + virDomainObjPtr vm, + struct qemuUdevUSBRemoveData *data) +{ + size_t i; + virDomainHostdevDefPtr hostdev; + virDomainDeviceDef dev = { .type = VIR_DOMAIN_DEVICE_HOSTDEV }; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + return; + + if (!virDomainObjIsActive(vm)) { + VIR_DEBUG("Domain is not running"); + goto cleanup; + } + + for (i = 0; i < vm->def->nhostdevs; i++) { + virDomainHostdevSubsysUSBPtr usbsrc; + + hostdev = vm->def->hostdevs[i]; + if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) + continue; + + usbsrc = &hostdev->source.subsys.u.usb; + + if (!usbsrc->replug) + continue; + + /* don't mess with devices that don't use stable host addressing + * with respect to unplug/plug to host + */ + if (!usbsrc->vendor || !usbsrc->product) + continue; + + if (usbsrc->bus == data->bus && usbsrc->device == data->device) + break; + } + + if (i == vm->def->nhostdevs) + goto cleanup; + + dev.data.hostdev = hostdev; + if (qemuDomainDetachDeviceLive(vm, &dev, driver, true, true) < 0) + goto cleanup; + + cleanup: + qemuDomainObjEndJob(driver, vm); +} + + +static void +qemuProcessEventHandler(void *data, void *opaque) { struct qemuProcessEvent *processEvent = data; virDomainObjPtr vm = processEvent->vm; @@ -5074,6 +5390,12 @@ static void qemuProcessEventHandler(void *data, void *opaque) case QEMU_PROCESS_EVENT_RDMA_GID_STATUS_CHANGED: processRdmaGidStatusChangedEvent(vm, processEvent->data); break; + case QEMU_PROCESS_EVENT_USB_REMOVED: + processUSBRemovedEvent(driver, vm, processEvent->data); + break; + case QEMU_PROCESS_EVENT_USB_ADDED: + processUSBAddedEvent(driver, vm, processEvent->data); + break; case QEMU_PROCESS_EVENT_LAST: break; } -- 2.23.0
It is possible for libvirtd to go down before DEVICE_DELETED event is delivered upon usb hostdev unplug and to receive the event after the libvirtd is up. In order to handle this case we need to save usb hostdev deleteAction in status file. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 26 ++++++++++++++++++++++++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_driver.c | 20 ++++++++++++++++++-- 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index db065f6a5b..c2d141fc74 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1236,6 +1236,13 @@ VIR_ENUM_IMPL(virDomainShmemModel, "ivshmem-doorbell", ); +VIR_ENUM_IMPL(virDomainHostdevDeleteAction, + VIR_DOMAIN_HOSTDEV_DELETE_ACTION_LAST, + "none", + "delete", + "unplug" +); + VIR_ENUM_IMPL(virDomainLaunchSecurity, VIR_DOMAIN_LAUNCH_SECURITY_LAST, "", @@ -7546,6 +7553,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, virDomainHostdevSubsysUSBPtr usbsrc = &def->source.subsys.u.usb; VIR_AUTOFREE(char *) startupPolicy = NULL; VIR_AUTOFREE(char *) autoAddress = NULL; + VIR_AUTOFREE(char *) deleteAction = NULL; if ((startupPolicy = virXMLPropString(node, "startupPolicy"))) { def->startupPolicy = @@ -7563,6 +7571,18 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, usbsrc->autoAddress = true; } + if ((deleteAction = virXMLPropString(node, "deleteAction"))) { + def->deleteAction = + virDomainHostdevDeleteActionTypeFromString(deleteAction); + + if (def->deleteAction <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unknown deleteAction '%s'"), + deleteAction); + goto out; + } + } + /* Product can validly be 0, so we need some extra help to determine * if it is uninitialized*/ got_product = false; @@ -24979,6 +24999,12 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, if (def->missing && !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)) virBufferAddLit(buf, " missing='yes'"); + + if (def->deleteAction && (flags & VIR_DOMAIN_DEF_FORMAT_STATUS)) { + const char *deleteAction; + deleteAction = virDomainHostdevDeleteActionTypeToString(def->deleteAction); + virBufferAsprintf(buf, " deleteAction='%s'", deleteAction); + } } if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index da005e5423..d596551d3a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -340,6 +340,7 @@ typedef enum { VIR_DOMAIN_HOSTDEV_DELETE_ACTION_LAST } virDomainHostdevDeleteActionType; +VIR_ENUM_DECL(virDomainHostdevDeleteAction); /* basic device for direct passthrough */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 51f1d5d4a2..b12b39d61d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5256,10 +5256,13 @@ processUSBAddedEvent(virQEMUDriverPtr driver, { virDomainHostdevDefPtr hostdev; virDomainHostdevSubsysUSBPtr usbsrc; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); size_t i; - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) { + virObjectUnref(cfg); return; + } if (!virDomainObjIsActive(vm)) { VIR_DEBUG("Domain is not running"); @@ -5287,8 +5290,13 @@ processUSBAddedEvent(virQEMUDriverPtr driver, if (qemuDomainAttachHostDevice(driver, vm, hostdev) < 0) goto cleanup; + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) + VIR_WARN("unable to save domain status after plugging device %s", + hostdev->info->alias); + cleanup: qemuDomainObjEndJob(driver, vm); + virObjectUnref(cfg); } @@ -5300,9 +5308,12 @@ processUSBRemovedEvent(virQEMUDriverPtr driver, size_t i; virDomainHostdevDefPtr hostdev; virDomainDeviceDef dev = { .type = VIR_DOMAIN_DEVICE_HOSTDEV }; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) { + virObjectUnref(cfg); return; + } if (!virDomainObjIsActive(vm)) { VIR_DEBUG("Domain is not running"); @@ -5338,8 +5349,13 @@ processUSBRemovedEvent(virQEMUDriverPtr driver, if (qemuDomainDetachDeviceLive(vm, &dev, driver, true, true) < 0) goto cleanup; + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) + VIR_WARN("unable to save domain status after unplugging device %s", + hostdev->info->alias); + cleanup: qemuDomainObjEndJob(driver, vm); + virObjectUnref(cfg); } -- 2.23.0
Imagine host usb device is unplugged from host and as a result we send command to qemu to delete appropriate device. Then before qemu device is deleted host usb device is plugged back. Currently code supposes there is no remnant device in qemu and will try to add new device and the attempt will fail. Instead let's check the device is not yet deleted and postpone adding qemu device to device_deleted event handler. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 49 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b12b39d61d..e5594835c4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4655,6 +4655,44 @@ processGuestPanicEvent(virQEMUDriverPtr driver, } +static int +qemuCheckHostdevPlugged(virQEMUDriverPtr driver, + virDomainObjPtr vm, + const char *devAlias) +{ + virDomainHostdevDefPtr hostdev; + virDomainHostdevSubsysUSBPtr usbsrc; + virDomainDeviceDef dev; + int num; + + if (virDomainDefFindDevice(vm->def, devAlias, &dev, false) < 0) + return 0; + + if (dev.type != VIR_DOMAIN_DEVICE_HOSTDEV) + return 0; + + hostdev = dev.data.hostdev; + if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) + return 0; + + usbsrc = &hostdev->source.subsys.u.usb; + if (!usbsrc->vendor || !usbsrc->product) + return 0; + + if ((num = virUSBDeviceFindByVendor(usbsrc->vendor, usbsrc->product, + NULL, false, NULL)) < 0) + return -1; + + if (num == 0) + return 0; + + if (qemuDomainAttachHostDevice(driver, vm, hostdev) < 0) + return -1; + + return 0; +} + + static void processDeviceDeletedEvent(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -4682,6 +4720,11 @@ processDeviceDeletedEvent(virQEMUDriverPtr driver, if (qemuDomainRemoveDevice(driver, vm, &dev) < 0) goto endjob; + + /* Fall thru and save status file even on error condition because + * device is removed successfully and changed configuration need + * to be saved in status file. */ + qemuCheckHostdevPlugged(driver, vm, devAlias); } if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) @@ -5287,6 +5330,12 @@ processUSBAddedEvent(virQEMUDriverPtr driver, if (i == vm->def->nhostdevs) goto cleanup; + /* if device is not yet even deleted from qemu then handle plugging later. + * Or we failed handling host usb device unplugging, then another attempt of + * unplug/plug could help. */ + if (usbsrc->bus || usbsrc->device) + goto cleanup; + if (qemuDomainAttachHostDevice(driver, vm, hostdev) < 0) goto cleanup; -- 2.23.0
I guess this is the missing piece for [1]. It did not hurt before (like we didn't even see any errors/warns in logs) because in qemuProcessUpdateDevices function virDomainDefFindDevice does not find device deleted from libvirt config. But now in case of unpluggind usb device from host we leave device in config and thus needlessly try to call qemuDomainRemoveDevice second time. [1] 0dfb8a1b9: qemu: Unplug devices that disappeared when libvirtd was down Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/qemu/qemu_driver.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e5594835c4..cd913385f5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4715,6 +4715,9 @@ processDeviceDeletedEvent(virQEMUDriverPtr driver, if (STRPREFIX(devAlias, "vcpu")) { qemuDomainRemoveVcpuAlias(driver, vm, devAlias); } else { + if (qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) + goto cleanup; + if (virDomainDefFindDevice(vm->def, devAlias, &dev, true) < 0) goto endjob; -- 2.23.0
Somebody can easily unplug usb device from host while libvirtd is being stopped. Also usb device can be plugged or unplugged/plugged back and so forth. Let's handle such cases. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_process.c | 56 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 955ba4de4c..26113e598d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3749,6 +3749,59 @@ qemuProcessUpdateDevices(virQEMUDriverPtr driver, return ret; } + +static int +qemuProcessReattachUSBDevices(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + size_t i; + + for (i = 0; i < vm->def->nhostdevs; i++) { + virDomainHostdevDefPtr hostdev = vm->def->hostdevs[i]; + virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb; + + if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB || + !usbsrc->replug) + continue; + + /* don't mess with devices that don't use stable host addressing + * with respect to unplug/plug to host + */ + if (!usbsrc->vendor || !usbsrc->product) + continue; + + if (!usbsrc->bus && !usbsrc->device) { + int num; + + if ((num = virUSBDeviceFindByVendor(usbsrc->vendor, usbsrc->product, + NULL, false, NULL)) < 0) + return -1; + + if (num > 0 && + qemuDomainAttachHostDevice(driver, vm, hostdev) < 0) + return -1; + } else { + virUSBDevicePtr usb; + + if (virUSBDeviceFindByBus(usbsrc->bus, usbsrc->device, + NULL, false, &usb) < 0) + return -1; + + if (!usb) { + virDomainDeviceDef dev = { .type = VIR_DOMAIN_DEVICE_HOSTDEV }; + + dev.data.hostdev = hostdev; + if (qemuDomainDetachDeviceLive(vm, &dev, driver, true, true) < 0) + return -1; + } + virUSBDeviceFree(usb); + } + } + + return 0; +} + + static int qemuDomainPerfRestart(virDomainObjPtr vm) { @@ -8217,6 +8270,9 @@ qemuProcessReconnect(void *opaque) if (qemuProcessUpdateDevices(driver, obj) < 0) goto error; + if (qemuProcessReattachUSBDevices(driver, obj) < 0) + goto error; + if (qemuRefreshPRManagerState(driver, obj) < 0) goto error; -- 2.23.0
First I don't want to add code to handle dummy device that is used when host usb device is not present at the moment of starting/migrating etc. Second supporting non mandatory policies would require to handle races when host usb device is plugged to host and libvirtd starts adding device but if in the meanwhile host usb device it unplugged back then current code will use dummy device which is not desired in this case. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/qemu/qemu_driver.c | 8 ++++++++ src/qemu/qemu_process.c | 4 ++++ 2 files changed, 12 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cd913385f5..70f5c70d65 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5321,6 +5321,10 @@ processUSBAddedEvent(virQEMUDriverPtr driver, if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) continue; + if (hostdev->startupPolicy == VIR_DOMAIN_STARTUP_POLICY_OPTIONAL || + hostdev->startupPolicy == VIR_DOMAIN_STARTUP_POLICY_REQUISITE) + continue; + usbsrc = &hostdev->source.subsys.u.usb; if (!usbsrc->replug) @@ -5379,6 +5383,10 @@ processUSBRemovedEvent(virQEMUDriverPtr driver, if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) continue; + if (hostdev->startupPolicy == VIR_DOMAIN_STARTUP_POLICY_OPTIONAL || + hostdev->startupPolicy == VIR_DOMAIN_STARTUP_POLICY_REQUISITE) + continue; + usbsrc = &hostdev->source.subsys.u.usb; if (!usbsrc->replug) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 26113e598d..ab0d85de1f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3770,6 +3770,10 @@ qemuProcessReattachUSBDevices(virQEMUDriverPtr driver, if (!usbsrc->vendor || !usbsrc->product) continue; + if (hostdev->startupPolicy == VIR_DOMAIN_STARTUP_POLICY_OPTIONAL || + hostdev->startupPolicy == VIR_DOMAIN_STARTUP_POLICY_REQUISITE) + continue; + if (!usbsrc->bus && !usbsrc->device) { int num; -- 2.23.0
If hostdev is unplugged we don't need to delete it's correspondent device from qemu etc. Just remove it from config immediately. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/qemu/qemu_hotplug.c | 16 +++++++++++++++- src/util/virhostdev.c | 2 ++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 780bfde002..32ab581755 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4528,6 +4528,7 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, char *objAlias = NULL; bool is_vfio = false; bool unplug = hostdev->deleteAction == VIR_DOMAIN_HOSTDEV_DELETE_ACTION_UNPLUG; + bool unplugged = false; VIR_DEBUG("Removing host device %s from domain %p %s", hostdev->info->alias, vm, vm->def->name); @@ -4586,13 +4587,17 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, virDomainAuditHostdev(vm, hostdev, "detach", true); + if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) { + virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb; + unplugged = !usbsrc->bus && !usbsrc->device; + } /* * In case of unplug the attempt to restore label will fail. But we don't * need to restore the label! In case of separate mount namespace for the * domain we remove device file later in this function. In case of global * mount namespace the device file is deleted or being deleted by systemd. */ - if (!is_vfio && !unplug && + if (!is_vfio && !unplug && !unplugged && qemuSecurityRestoreHostdevLabel(driver, vm, hostdev) < 0) VIR_WARN("Failed to restore host device labelling"); @@ -5932,6 +5937,15 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm, unplug) < 0) { return -1; } + if (!unplug) { + virDomainHostdevDefPtr hostdev = detach.data.hostdev; + virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb; + + if (hostdev->startupPolicy != VIR_DOMAIN_STARTUP_POLICY_OPTIONAL && + hostdev->startupPolicy != VIR_DOMAIN_STARTUP_POLICY_REQUISITE && + usbsrc->device == 0 && usbsrc->bus == 0) + return qemuDomainRemoveDevice(driver, vm, &detach); + } break; case VIR_DOMAIN_DEVICE_RNG: if (qemuDomainDetachPrepRNG(vm, match->data.rng, diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 41fcab7222..ed9bc09e14 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -1793,6 +1793,8 @@ virHostdevReAttachUSBDevices(virHostdevManagerPtr mgr, continue; if (hostdev->missing) continue; + if (!usbsrc->bus && !usbsrc->device) + continue; if (!(usb = virUSBDeviceNew(usbsrc->bus, usbsrc->device, NULL))) { VIR_WARN("Unable to reattach USB device %03d.%03d on domain %s", -- 2.23.0
We want to keep this flag across libvirtd restarts. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/conf/domain_conf.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c2d141fc74..0af19e63d1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7554,6 +7554,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, VIR_AUTOFREE(char *) startupPolicy = NULL; VIR_AUTOFREE(char *) autoAddress = NULL; VIR_AUTOFREE(char *) deleteAction = NULL; + VIR_AUTOFREE(char *) missing = NULL; if ((startupPolicy = virXMLPropString(node, "startupPolicy"))) { def->startupPolicy = @@ -7583,6 +7584,11 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, } } + if ((missing = virXMLPropString(node, "missing"))) { + if (STREQ(missing, "yes")) + def->missing = true; + } + /* Product can validly be 0, so we need some extra help to determine * if it is uninitialized*/ got_product = false; -- 2.23.0
ping On 17.09.2019 16:47, Nikolay Shirokovskiy wrote:
Diff to v2[1] version: - add 'replug' attribute for hostdev element to allow replug semantics - avoid accuiring domain lock in event loop thread on udev events as suggested by Peter - nit picks after review by Daniel Henrique Barboza
* is used to mark patches that were 'Reviewed-by' by Daniel (sometimes with very minor changes to take into account new replug flag).
[1] https://www.redhat.com/archives/libvir-list/2019-September/msg00321.html
Nikolay Shirokovskiy (12): conf: add replug option for usb hostdev qemu: track hostdev delete intention qemu: support host usb device unplug * qemu: support usb hostdev plugging back * qemu: handle host usb device add/del udev events qemu: handle libvirtd restart after host usb device unplug * qemu: handle race on device deletion and usb host device plugging * qemu: hotplug: update device list on device deleted event qemu: handle host usb device plug/unplug when libvirtd is down * qemu: don't mess with non mandatory hostdevs on reattaching * qemu: handle detaching of unplugged hostdev conf: parse hostdev missing flag *
docs/formatdomain.html.in | 10 +- docs/schemas/domaincommon.rng | 5 + src/conf/domain_conf.c | 62 +++ src/conf/domain_conf.h | 17 + src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_conf.h | 3 + src/qemu/qemu_domain.c | 2 + src/qemu/qemu_domain.h | 2 + src/qemu/qemu_driver.c | 404 +++++++++++++++++- src/qemu/qemu_hotplug.c | 104 ++++- src/qemu/qemu_hotplug.h | 3 +- src/qemu/qemu_process.c | 60 +++ src/util/virhostdev.c | 2 + tests/qemuhotplugtest.c | 2 +- tests/qemuxml2argvdata/hostdev-usb-replug.xml | 36 ++ .../qemuxml2xmloutdata/hostdev-usb-replug.xml | 40 ++ tests/qemuxml2xmltest.c | 1 + 17 files changed, 733 insertions(+), 22 deletions(-) create mode 100644 tests/qemuxml2argvdata/hostdev-usb-replug.xml create mode 100644 tests/qemuxml2xmloutdata/hostdev-usb-replug.xml
-- 2.23.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (2)
-
Jonathon Jongsma -
Nikolay Shirokovskiy