On Thu, Mar 16, 2017 at 08:32:14PM +0100, Radim Krčmář wrote: > 2017-03-16 14:04+0800, Peter Xu: > > On Wed, Mar 15, 2017 at 12:15:24PM +0100, David Hildenbrand wrote: > >> Am 15.03.2017 um 09:59 schrieb Peter Xu: > >> > On Wed, Mar 15, 2017 at 09:47:14AM +0100, David Hildenbrand wrote: > >> >> Am 15.03.2017 um 09:01 schrieb Peter Xu: > >> >>> v2: > >> >>> - add one more patch (patch 2 in v2) to check pic/ioapic's > >> >>> existance before destroying them > >> >> > >> >> Patch 2+3 without 1 should work, too, right? > >> > > >> > We may need that? When destroy VM, it's: > >> > > >> > kvm_destroy_vm() > >> > foreach (bus) do kvm_io_bus_destroy() > >> > kfree(bus) <--- [1] > >> > kvm_arch_destroy_vm() > >> > kvm_pic_destroy() > >> > kvm_io_bus_unregister_dev() <--- [2] > >> > > >> > >> Wouldn't the natural way be > >> > >> 1. kvm_arch_destroy_vm() > >> 2. foreach (bus) do kvm_io_bus_destroy() > >> kfree(bus) > >> > >> Then we wouldn't have to deal with suddenly removed buses. But maybe > >> that order is of importance here ... > > > > I'll leave this question for Paolo/Radim, or anyone knows this better > > than me. :-) > > I'd like if kvm_destroy_vm() looked like kvm_create_vm() in reverse and > the current order is closer to that. > > I don't think there is dependency between those two and we can easily > reverse them in kvm_create_vm(), so go ahead if you get better code out > of it in kvm_arch_destroy_vm(). :) Thanks. :) IMHO in all cases we can consider having the first two patches, which should be something nice to have. And if so, I'll prefer patch 3 comparing to reordering of VM init/destroy to avoid any possiblilty of breakage, until one day we really need to touch them (I guess this cleanup series is not a good enough reason :). Thanks, -- peterx
