Re: [libvirt] [PATCH v2 03/14] qemu_hotplug: pull qemuDomainUpdateDeviceList out of qemuDomainDetachDeviceLive

On Mon, Mar 25, 2019 at 13:24:25 -0400, Laine Stump wrote: > qemuDomainDetachDeviceLive() is called from two places in > qemu_driver.c, and qemuDomainUpdateDeviceList() is called from the > end of qemuDomainDetachDeviceLive(), which is now in qemu_hotplug.c > > This patch replaces the single call to qemuDomainUpdateDeviceList() > with two calls to it immediately after return from > qemuDomainDetachDeviceLive(). This is only done if the return from > that function is exactly 0, in order to exactly preserve previous > behavior. > > Removing that one call from qemuDomainDetachDeviceList() will permit > us to call it from the test driver hotplug test, replacing the > separate calls to qemuDomainDetachDeviceDiskLive(), > qemuDomainDetachChrDevice(), qemuDomainDetachShmemDevice() and > qemuDomainDetachWatchdog(). We want to do this so that part of the > common functionality of those three functions (and the rest of the > device-specific Detach functions) can be pulled up into > qemuDomainDetachDeviceLive() without breaking the test. (This is done > in the next patch). > > NB: Almost certainly this is "not the best place" to call > qemuDomainUpdateDeviceList() (actually, it is provably the *wrong* > place), since it's purpose is to retrieve an "up to date" list of > aliases for all devices from qemu, and if the guest OS hasn't yet > processed the detach request, the now-being-removed device may still > be on that list. It would arguably be better to instead call > qemuDomainUpdateDevicesList() later during the response to the > DEVICE_DELETED event for the device. But removing the call from the > current point in the detach could have some unforeseen ill effect due > to changed timing, so the change to move it into > qemuDomainRemove*Device() will be done in a separate patch (in order > to make it easily revertible in case it causes a regression). Actually it probably would cause a regression. It's called only when the device is removed. That function will require some overhauling.

> Signed-off-by: Laine Stump <laine(a)laine.org&gt; > --- > > Change from V1: > > * Added the big explanation above about why I'm not completely > changing behavior by waiting unt after DEVICE_DELETED to call > qemuDomainUpdateDeviceList() > > * Only call qemuDomainUpdateDeviceList() if the Detach function > returned *exactly* 0, since there are cases where it could return > > 0, and the previous behavior was to not call it in those cases. ACK