[v3] hostdev: handle usb detach/attach on node

[libvirt] [PATCH v3 REBASE 2 00/12] hostdev: handle usb detach/attach on node

Posted by Nikolay Shirokovskiy 4 years, 5 months ago

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

Can be applied on:

commit bf0e7bdeeb790bc6ba5732623be0d9ff26a5961a
Author: Peter Krempa <pkrempa@redhat.com>
Date:   Thu Oct 24 15:50:50 2019 +0200

    util: xml: Make virXMLFormatElement void


[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

Re: [libvirt] [PATCH v3 REBASE 2 00/12] hostdev: handle usb detach/attach on node

Posted by Dave Allan 4 years, 5 months ago

On Tue, Oct 29, 2019 at 11:17:51AM +0300, 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).

I did some basic testing today, and I'm seeing the device appear and disappear in the guest, which is great and much nicer than my ugly udev rule hack.  I did find what I think is a bug though: if the USB device is plugged in at domain start, unplugging it while the domain is running does not cause it to disappear, and subsqeuently replugging it into the host causes a second instance of the device to appear in the guest.

>Can be applied on:
>
>commit bf0e7bdeeb790bc6ba5732623be0d9ff26a5961a
>Author: Peter Krempa <pkrempa@redhat.com>
>Date:   Thu Oct 24 15:50:50 2019 +0200
>
>    util: xml: Make virXMLFormatElement void
>[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
>
>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 REBASE 2 00/12] hostdev: handle usb detach/attach on node

Posted by Nikolay Shirokovskiy 4 years, 5 months ago


On 30.10.2019 23:21, Dave Allan wrote:
> On Tue, Oct 29, 2019 at 11:17:51AM +0300, 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).
> 
> I did some basic testing today, and I'm seeing the device appear and disappear in the guest, which is great and much nicer than my ugly udev rule hack.  I did find what I think is a bug though: if the USB device is plugged in at domain start, unplugging it while the domain is running does not cause it to disappear, and subsqeuently replugging it into the host causes a second instance of the device to appear in the guest.

Hi.

Hmm. Looks like you're using startupPolicy=optional otherwise it is not 
possible to start domain without a device. But in this case the whole replug thing
is disabled and further I don't understand how the second instance of the device can appear.

If device is present at domain start then replug is in play. And I cannot reproduce the bug.

Can you clarify on you use case?

Nikolay

> 
>> Can be applied on:
>>
>> commit bf0e7bdeeb790bc6ba5732623be0d9ff26a5961a
>> Author: Peter Krempa <pkrempa@redhat.com>
>> Date:   Thu Oct 24 15:50:50 2019 +0200
>>
>>    util: xml: Make virXMLFormatElement void
>> [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
>>
>>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 REBASE 2 00/12] hostdev: handle usb detach/attach on node

Posted by Dave Allan 4 years, 5 months ago

On Thu, Oct 31, 2019 at 10:40:42AM +0000, Nikolay Shirokovskiy wrote:
>
>
>On 30.10.2019 23:21, Dave Allan wrote:
>> On Tue, Oct 29, 2019 at 11:17:51AM +0300, 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).
>>
>> I did some basic testing today, and I'm seeing the device appear and disappear in the guest, which is great and much nicer than my ugly udev rule hack.  I did find what I think is a bug though: if the USB device is plugged in at domain start, unplugging it while the domain is running does not cause it to disappear, and subsqeuently replugging it into the host causes a second instance of the device to appear in the guest.
>
>Hi.
>
>Hmm. Looks like you're using startupPolicy=optional otherwise it is not
>possible to start domain without a device. But in this case the whole replug thing
>is disabled and further I don't understand how the second instance of the device can appear.
>
>If device is present at domain start then replug is in play. And I cannot reproduce the bug.

I am using startupPolicy optional, and I'm seeing the device attached and detached 100% of the time.  I can start the domain with or without the device attached to the host, plug and unplug it and it appears and disappears from the guest, which is exactly the behavior I want.  My domain XML is attached.

I tested a few more times just now, and I saw the duplicate device appear in the guest the first time I tried, but I have not been able to reproduce it again after that.

>Can you clarify on you use case?

My usecase is that I'm flashing the USB device with a program running in the VM.  I keep the VM running, and when I need to reflash, I connect the device, so having the startupPolicy optional is useful.

>Nikolay
>
>>
>>> Can be applied on:
>>>
>>> commit bf0e7bdeeb790bc6ba5732623be0d9ff26a5961a
>>> Author: Peter Krempa <pkrempa@redhat.com>
>>> Date:   Thu Oct 24 15:50:50 2019 +0200
>>>
>>>    util: xml: Make virXMLFormatElement void
>>> [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
>>>
>>>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 REBASE 2 00/12] hostdev: handle usb detach/attach on node

Posted by Jiri Denemark 4 years, 5 months ago

Hi Dave,

Time flies, doesn't it? :-)

My reply is not related to this patch series, but rather your use
case...

On Thu, Oct 31, 2019 at 12:50:28 -0400, Dave Allan wrote:
> My usecase is that I'm flashing the USB device with a program running
> in the VM.  I keep the VM running, and when I need to reflash, I
> connect the device, so having the startupPolicy optional is useful.

Have you tried using redirdev for this? It might be much more
convenient. You can dynamically attach a USB device to your host while
the domain is running and select it to be passed to the guest in your
spice client (both virt-manager and virt-viewer support this).

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 REBASE 2 00/12] hostdev: handle usb detach/attach on node

Posted by Dave Allan 4 years, 5 months ago

On Tue, Nov 05, 2019 at 03:54:09PM +0100, Jiri Denemark wrote:
>Hi Dave,
>
>Time flies, doesn't it? :-)
>
>My reply is not related to this patch series, but rather your use
>case...
>
>On Thu, Oct 31, 2019 at 12:50:28 -0400, Dave Allan wrote:
>> My usecase is that I'm flashing the USB device with a program running
>> in the VM.  I keep the VM running, and when I need to reflash, I
>> connect the device, so having the startupPolicy optional is useful.
>
>Have you tried using redirdev for this? It might be much more
>convenient. You can dynamically attach a USB device to your host while
>the domain is running and select it to be passed to the guest in your
>spice client (both virt-manager and virt-viewer support this).

Hi Jiri,

It has been a while!  Thanks for the suggestion, I'll give redirdev a try.

Dave

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list