[libvirt] [PATCH v2 09/11] qemu: don't mess with non mandatory hostdevs on reattaching

Nikolay Shirokovskiy posted 11 patches 6 years, 5 months ago
There is a newer version of this series
[libvirt] [PATCH v2 09/11] qemu: don't mess with non mandatory hostdevs on reattaching
Posted by Nikolay Shirokovskiy 6 years, 5 months ago
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>
---
 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 b646642c99..fe5fd94ac5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5337,6 +5337,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->vendor == data->vendor && usbsrc->product == data->product)
@@ -5392,6 +5396,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;
 
         /* don't mess with devices that don't use stable host addressing
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 8bec36fe2c..d87fb637ac 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3769,6 +3769,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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 09/11] qemu: don't mess with non mandatory hostdevs on reattaching
Posted by Daniel Henrique Barboza 6 years, 4 months ago

On 9/9/19 8:33 AM, Nikolay Shirokovskiy wrote:
> 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 b646642c99..fe5fd94ac5 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -5337,6 +5337,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->vendor == data->vendor && usbsrc->product == data->product)
> @@ -5392,6 +5396,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;
>   
>           /* don't mess with devices that don't use stable host addressing
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 8bec36fe2c..d87fb637ac 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -3769,6 +3769,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;
>   

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 09/11] qemu: don't mess with non mandatory hostdevs on reattaching
Posted by Peter Krempa 6 years, 4 months ago
On Mon, Sep 09, 2019 at 14:33:12 +0300, Nikolay Shirokovskiy wrote:
> 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.

I don't think that handling startupPolicy for cases other than VM
startup makes semantic sense. Could you elaborate what's the goal? I
didn't quite get it from the commit message.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 09/11] qemu: don't mess with non mandatory hostdevs on reattaching
Posted by Nikolay Shirokovskiy 6 years, 4 months ago

On 16.09.2019 11:30, Peter Krempa wrote:
> On Mon, Sep 09, 2019 at 14:33:12 +0300, Nikolay Shirokovskiy wrote:
>> 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.
> 
> I don't think that handling startupPolicy for cases other than VM
> startup makes semantic sense. Could you elaborate what's the goal? I
> didn't quite get it from the commit message.
> 

We have different states for device that was missing on start
and device that was unplugged. In the former case we have stub
device in qemu (bus=0, device=0) while in the latter case we don't
have any correnspondent device in qemu. So I don't want add extra
logic to handle the first case on re-plug.

Nikolay

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 09/11] qemu: don't mess with non mandatory hostdevs on reattaching
Posted by Peter Krempa 6 years, 4 months ago
On Mon, Sep 16, 2019 at 08:58:10 +0000, Nikolay Shirokovskiy wrote:
> 
> 
> On 16.09.2019 11:30, Peter Krempa wrote:
> > On Mon, Sep 09, 2019 at 14:33:12 +0300, Nikolay Shirokovskiy wrote:
> >> 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.
> > 
> > I don't think that handling startupPolicy for cases other than VM
> > startup makes semantic sense. Could you elaborate what's the goal? I
> > didn't quite get it from the commit message.
> > 
> 
> We have different states for device that was missing on start
> and device that was unplugged. In the former case we have stub
> device in qemu (bus=0, device=0) while in the latter case we don't
> have any correnspondent device in qemu. So I don't want add extra
> logic to handle the first case on re-plug.

AFAIK a device whose startup policy allows it to be missing is removed
fully from the (running) definition on VM startup if it was not present
at that time.

If it is not like that it's probably a bug because having a device in
the definition which is not recognized by the VM should not happen.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 09/11] qemu: don't mess with non mandatory hostdevs on reattaching
Posted by Nikolay Shirokovskiy 6 years, 4 months ago

On 16.09.2019 12:09, Peter Krempa wrote:
> On Mon, Sep 16, 2019 at 08:58:10 +0000, Nikolay Shirokovskiy wrote:
>>
>>
>> On 16.09.2019 11:30, Peter Krempa wrote:
>>> On Mon, Sep 09, 2019 at 14:33:12 +0300, Nikolay Shirokovskiy wrote:
>>>> 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.
>>>
>>> I don't think that handling startupPolicy for cases other than VM
>>> startup makes semantic sense. Could you elaborate what's the goal? I
>>> didn't quite get it from the commit message.
>>>
>>
>> We have different states for device that was missing on start
>> and device that was unplugged. In the former case we have stub
>> device in qemu (bus=0, device=0) while in the latter case we don't
>> have any correnspondent device in qemu. So I don't want add extra
>> logic to handle the first case on re-plug.
> 
> AFAIK a device whose startup policy allows it to be missing is removed
> fully from the (running) definition on VM startup if it was not present
> at that time.
> 
> If it is not like that it's probably a bug because having a device in
> the definition which is not recognized by the VM should not happen.
> 

It is definetly a designed behaviour as we even have 'missing' attribute for
source element for this case.

Nikolay

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