From patchwork Tue Jun 21 23:40:27 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Cole Robinson X-Patchwork-Id: 70595 Delivered-To: patch@linaro.org Received: by 10.140.28.4 with SMTP id 4csp2271081qgy; Tue, 21 Jun 2016 16:50:40 -0700 (PDT) X-Received: by 10.28.168.7 with SMTP id r7mr5489472wme.9.1466553040388; Tue, 21 Jun 2016 16:50:40 -0700 (PDT) Return-Path: Received: from mx4-phx2.redhat.com (mx4-phx2.redhat.com. [209.132.183.25]) by mx.google.com with ESMTPS id p129si7420782wmp.104.2016.06.21.16.50.39 (version=TLS1 cipher=AES128-SHA bits=128/128); Tue, 21 Jun 2016 16:50:40 -0700 (PDT) Received-SPF: pass (google.com: domain of libvir-list-bounces@redhat.com designates 209.132.183.25 as permitted sender) client-ip=209.132.183.25; Authentication-Results: mx.google.com; spf=pass (google.com: domain of libvir-list-bounces@redhat.com designates 209.132.183.25 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by mx4-phx2.redhat.com (8.13.8/8.13.8) with ESMTP id u5LNlrt9031344; Tue, 21 Jun 2016 19:47:54 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id u5LNeT4h019627 for ; Tue, 21 Jun 2016 19:40:29 -0400 Received: from [10.10.116.65] (ovpn-116-65.rdu2.redhat.com [10.10.116.65]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u5LNeRsG016215; Tue, 21 Jun 2016 19:40:28 -0400 To: Tomasz Flendrich , Laine Stump , Martin Kletzander References: <1466381920-2117-1-git-send-email-t.flendrich@gmail.com> <448D3D11-CA56-4D09-AA6C-F2CCCDD75763@gmail.com> From: Cole Robinson Message-ID: <7fd644b9-c977-4228-01ec-d43a0ef56304@redhat.com> Date: Tue, 21 Jun 2016 19:40:27 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <448D3D11-CA56-4D09-AA6C-F2CCCDD75763@gmail.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-loop: libvir-list@redhat.com Cc: libvir-list@redhat.com Subject: Re: [libvirt] [PATCH 0/4] Move ccwaddrs and pciaddrs to domainDef X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com On 06/20/2016 10:26 PM, Tomasz Flendrich wrote: > >> Apologies if I'm missing something, I didn't look too closely at this series, >> however have you seen this thread? >> >> http://www.redhat.com/archives/libvir-list/2016-May/msg01071.html > I haven’t noticed that some work has been done on that, thank you! > >> My understanding of the current code is that the cached vioserial/ccw/pciaddrs >> lists in qemu aren't actually required…they were at one point to handle > >> older qemu, but we dropped that support. Maybe you can pick up my patches and >> finish off dropping of pciaddrs and ccwaddrs? I suspect the pciaddrs cache in >> bhyve can be dropped as well, I don't think it was ever strictly required, the >> code just followed the qemu example > If we could do without the caching, it would make the current code simpler. > There wouldn’t be those booleans in qemu_hotplug.c that remember whether > an address has to be deleted or not in case something fails. We could > delete qemuDomainReleaseDeviceAddress() and a few more functions. > > I examined vioserial and pci addresses and it looks like > it could be done. However, I'm not an expert on qemu_hotplug yet > and this is where the interesting stuff happens with addresses, > so I am not entirely sure yet. > I also don't know what the plans are for device addresses in the future. > Perhaps there are some features that will require caching them. > In a way they are currently cached twice: once in these lists, and once in the runtime VM XML. That's what my vioserial series did, was replace the hotplug usage of the cached list with just fetching it on demand from the running VM XML, so at least at a high level these cached lists seem redundant because the info is tracked elsewhere. > I think that recalculation may change the current behavior of ccw addresses. > Function virDomainCCWAddressReleaseAddr() modifies addrs->next > only when the address being released was the address most recently > assigned. > Hmm, that's interesting. Not sure what that is about. I tried adding this diff: return ret; And the test suite doesn't break, so at least it's not something completely obvious. But then again this seems to only be via hotunplug call path, and our test suite coverage for hotplug isn't that great. This logic may in fact just be an artifact of maintaining a persistent set of addresses. If we are generating an address set on demand from the runtime XML, we probably don't need to worry about 'releasing' an address from that set at all, so it may just side step whatever issue this logic was trying to address. > Laine, you know a lot about PCI addresses and you also mentioned that > you want to modify them in the future. What do you think? > Laine did have some patches that did touch some bits in this area, but only as a side effect. Thanks, Cole --- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 794270d..7d39b69 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -792,14 +792,7 @@ virDomainCCWAddressReleaseAddr(virDomainCCWAddressSetPtr addrs, if (!addr) return -1; - if ((ret = virHashRemoveEntry(addrs->defined, addr)) == 0 && - dev->addr.ccw.cssid == addrs->next.cssid && - dev->addr.ccw.ssid == addrs->next.ssid && - dev->addr.ccw.devno < addrs->next.devno) { - addrs->next.devno = dev->addr.ccw.devno; - addrs->next.assigned = false; - } - + ret = virHashRemoveEntry(addrs->defined, addr); VIR_FREE(addr);