[v2] Fix hotplug of unassigned hostdevs

[PATCH v2 0/3] Fix hotplug of unassigned hostdevs

Posted by Michal Privoznik 2 years, 3 months ago

v2 of:

https://listman.redhat.com/archives/libvir-list/2022-January/msg01111.html

Patches 1/3 and 2/3 were ACKed but I'm sending them for completeness.

diff to v1:
- Reworked patch 3/3

Michal Prívozník (3):
  domain_validate: Refuse VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED
  qemuDomainAttachHostPCIDevice: Handle hostevs with unassigned type of
    address
  qemuDomainDetachDeviceLive: Handle hostevs with unassigned type of
    address

 src/conf/domain_validate.c | 45 ++++++++++++++++++++++++++++++++++++++
 src/qemu/qemu_hotplug.c    | 11 ++++++++++
 2 files changed, 56 insertions(+)

-- 
2.34.1

Re: [PATCH v2 0/3] Fix hotplug of unassigned hostdevs

Posted by Daniel Henrique Barboza 2 years, 2 months ago


On 1/26/22 06:29, Michal Privoznik wrote:
> v2 of:
> 
> https://listman.redhat.com/archives/libvir-list/2022-January/msg01111.html
> 
> Patches 1/3 and 2/3 were ACKed but I'm sending them for completeness.


Back when I've added this UNASSIGNED address type I was also pursuing a PCI multifunction
hotplug/unplug feature in Libvirt and, apparently, I didn't mind testing hotplug/unplug
of non-multifunction unassigned hostdevs like the bug mentioned in patch 1 did.


Thanks for fixing this up.


Daniel

> 
> diff to v1:
> - Reworked patch 3/3
> 
> Michal Prívozník (3):
>    domain_validate: Refuse VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED
>    qemuDomainAttachHostPCIDevice: Handle hostevs with unassigned type of
>      address
>    qemuDomainDetachDeviceLive: Handle hostevs with unassigned type of
>      address
> 
>   src/conf/domain_validate.c | 45 ++++++++++++++++++++++++++++++++++++++
>   src/qemu/qemu_hotplug.c    | 11 ++++++++++
>   2 files changed, 56 insertions(+)
>

Re: [PATCH v2 0/3] Fix hotplug of unassigned hostdevs

Posted by Michal Prívozník 2 years, 2 months ago

On 1/27/22 14:57, Daniel Henrique Barboza wrote:
> 
> 
> On 1/26/22 06:29, Michal Privoznik wrote:
>> v2 of:
>>
>> https://listman.redhat.com/archives/libvir-list/2022-January/msg01111.html
>>
>>
>> Patches 1/3 and 2/3 were ACKed but I'm sending them for completeness.
> 
> 
> Back when I've added this UNASSIGNED address type I was also pursuing a
> PCI multifunction
> hotplug/unplug feature in Libvirt and, apparently, I didn't mind testing
> hotplug/unplug
> of non-multifunction unassigned hostdevs like the bug mentioned in patch
> 1 did.
> 
> 
> Thanks for fixing this up.

yeah, this is more of a workaround than a real fix, because IIUC
unassigned PCI address makes sense for devices within one IOMMU group. I
mean, you want to passthrough one specific PCI device but because it's
in IOMMU group with another one, both have to be detached from the host.

Well, and for that we would need the hotplug/hotunplug API understand
that and attach/detach two or more devices at once. With my patches you
can still hotplug/hotunplug just a single device, but hey, at least we
don't error out!

Michal

Re: [PATCH v2 0/3] Fix hotplug of unassigned hostdevs

Posted by Daniel Henrique Barboza 2 years, 2 months ago

On 1/27/22 11:05, Michal Prívozník wrote:
> On 1/27/22 14:57, Daniel Henrique Barboza wrote:
>>
>>
>> On 1/26/22 06:29, Michal Privoznik wrote:
>>> v2 of:
>>>
>>> https://listman.redhat.com/archives/libvir-list/2022-January/msg01111.html
>>>
>>>
>>> Patches 1/3 and 2/3 were ACKed but I'm sending them for completeness.
>>
>>
>> Back when I've added this UNASSIGNED address type I was also pursuing a
>> PCI multifunction
>> hotplug/unplug feature in Libvirt and, apparently, I didn't mind testing
>> hotplug/unplug
>> of non-multifunction unassigned hostdevs like the bug mentioned in patch
>> 1 did.
>>
>>
>> Thanks for fixing this up.
> 
> yeah, this is more of a workaround than a real fix, because IIUC
> unassigned PCI address makes sense for devices within one IOMMU group. I
> mean, you want to passthrough one specific PCI device but because it's
> in IOMMU group with another one, both have to be detached from the host.

Yes. The context where I added this UNASSIGNED address type was to allow a whole PCI
multifunction card to be managed by Libvirt, which would detach all the functions from
the IOMMU grupo of the host and, at the same time, filter which functions would be visible
to the guest.

I didn't foresee this usage back then.

> 
> Well, and for that we would need the hotplug/hotunplug API understand
> that and attach/detach two or more devices at once. With my patches you
> can still hotplug/hotunplug just a single device, but hey, at least we
> don't error out!

You could also just forbid hotplug of UNASSIGNED non-multifunction hostdevs.

I can't think of an actual use of detaching a single hostdev from the host and doing nothing
with it. If the idea is to refrain the host from using the hostdev the user can just
nodedev-detach it.

Matter of fact, I'd say that the 'UNASSIGNED' address type makes little sense for
non-multifuction devices altogether.

Thanks,

Daniel

> 
> Michal
>