[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] xen_pt: fix failure of attaching & detaching a
From: |
Chang, JianzhongX |
Subject: |
Re: [Qemu-devel] [PATCH] xen_pt: fix failure of attaching & detaching a PCI device to VM repeatedly |
Date: |
Mon, 7 Dec 2015 06:28:40 +0000 |
>> An error message will be reported like this:
>> "libxl: error: libxl_qmp.c:287:qmp_handle_error_response: received an
>> error message from QMP server: Duplicate ID 'pci-pt-07_10.1' for device"
>>
>> When xen_pt_region_add/del() is called, MemoryRegion may not belong to
>> the XenPCIPassthroughState.
>> xen_pt_region_update() checks it but memory_region_ref/unref() does not.
>> This case causes obj->ref issue and affects the release of related objects.
>> So, the detection operation is moved from xen_pt_region_update to
>> xen_pt_region_add/del.
>>
>> Signed-off-by: Jianzhong,Chang <address@hidden>
>> ---
>> hw/xen/xen_pt.c | 23 ++++++++++++++++++++---
>> 1 files changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c index aa96288..95b4970
>> 100644
>> --- a/hw/xen/xen_pt.c
>> +++ b/hw/xen/xen_pt.c
>> @@ -523,6 +523,14 @@ static int
>> xen_pt_bar_from_region(XenPCIPassthroughState *s, MemoryRegion *mr)
>> }
>> return -1;
>> }
>> +static bool xen_pt_region_in_state(XenPCIPassthroughState *s,
>> +MemoryRegion *mr) {
>> + int bar = xen_pt_bar_from_region(s, mr);
>> + if (-1 == bar && (!s->msix || &s->msix->mmio != mr)) {
>> + return false;
>> + }
>> + return true;
>> +}
>>
>> /*
>> * This function checks if an io_region overlaps an io_region from
>> another @@ -587,9 +595,6 @@ static void
>> xen_pt_region_update(XenPCIPassthroughState *s,
>> };
>>
>> bar = xen_pt_bar_from_region(s, mr);
>> - if (bar == -1 && (!s->msix || &s->msix->mmio != mr)) {
>> - return;
>> - }
>>
>> if (s->msix && &s->msix->mmio == mr) {
>> if (adding) {
>> @@ -642,6 +647,9 @@ static void xen_pt_region_add(MemoryListener *l,
>> MemoryRegionSection *sec)
>> XenPCIPassthroughState *s = container_of(l, XenPCIPassthroughState,
>> memory_listener);
>>
>> + if (!xen_pt_region_in_state(s, sec->mr)) {
>> + return;
>> + }
>> memory_region_ref(sec->mr);
>> xen_pt_region_update(s, sec, true); } @@ -651,6 +659,9 @@ static
>> void xen_pt_region_del(MemoryListener *l, MemoryRegionSection *sec)
>> XenPCIPassthroughState *s = container_of(l, XenPCIPassthroughState,
>> memory_listener);
>>
>> + if (!xen_pt_region_in_state(s, sec->mr)) {
>> + return;
>> + }
>> xen_pt_region_update(s, sec, false);
>> memory_region_unref(sec->mr);
>> }
>> @@ -660,6 +671,9 @@ static void xen_pt_io_region_add(MemoryListener *l,
>> MemoryRegionSection *sec)
>> XenPCIPassthroughState *s = container_of(l, XenPCIPassthroughState,
>> io_listener);
>>
>> + if (!xen_pt_region_in_state(s, sec->mr)) {
>> + return;
>> + }
>> memory_region_ref(sec->mr);
>> xen_pt_region_update(s, sec, true); } @@ -669,6 +683,9 @@ static
>> void xen_pt_io_region_del(MemoryListener *l, MemoryRegionSection *sec)
>> XenPCIPassthroughState *s = container_of(l, XenPCIPassthroughState,
>> io_listener);
>>
>> + if (!xen_pt_region_in_state(s, sec->mr)) {
>> + return;
>> + }
>> xen_pt_region_update(s, sec, false);
>> memory_region_unref(sec->mr);
>> }
>
>Wouldn't it make more sense to move the
>memory_region_ref/memory_region_unref calls inside xen_pt_region_update?
Move the operation of mr->ref inside xen_pt_region_update can also fix this
bug.
For the convenience of maintenance, another patch can be created.