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

[Laine Stump]

On 3/26/19 3:45 AM, Peter Krempa wrote:
> 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.


Yeah, I started to look at it and decided it was too complicated to try 
and pull together in a few minutes while I was rushed, so I just wrote 
it on my to-do list.


>
>> Signed-off-by: Laine Stump <laine at laine.org>
>> ---
>>
>> 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