Re: [libvirt] [PATCH 20/21] qemu_hotplug: consolidate all common detach code in qemuDomainDetachDeviceLive

On Thu, Mar 21, 2019 at 18:29:00 -0400, Laine Stump wrote: > Now that all the qemuDomainDetachPrep*() functions look nearly > identical at the end, we can put one copy of that identical code in > qemuDomainDetachDeviceLive() at the point after the individual prep > functions have been called, and remove the duplicated code from all > the prep functions. The code to locate the target "detach" device > based on the "match" device remains, as do all device-type-specific > validations. > > Unfortunately there are a few things going on at once in this patch, > which makes it a bit more difficult to follow than the others; it was > just impossible to do the changes in stages and still have a > buildable/testable tree at each step. > > The other changes of note: > > * The individual prep functions no longer need their driver or async > args, so those are removed, as are the local "ret" variables, since > in all cases the functions just directly return -1 or 0. > > * Some of the prep functions were checking for a valid alias and/or > for attempts to detach a multifunction PCI device, but not all. In > fact, both checks are valid (or at least harmless) for *all* device > types, so they are removed from the prep functions, and done a > single time in the common function. > > (any attempts to *create* an alias when there isn't one has been > removed, since that is doomed to failure anyway; the only way the > device wouldn't have an alias is if 1) the domain was created by > calling virsh qemu-attach to attach an existing qemu process to > libvirt, and 2) the qemu command that started said process used "old > style" arguments for creating devices that didn't have any device > ids. Even if we constructed a device id for one of these devices, > qemu wouldn't recognize it in the device_del command anyway, so we > may as well fail earlier with "device missing alias" rather than > failing later with "couldn't delete device net0".) > > * Only one type of device has shutdown code that must not be called > until after *all* validation of the device is done (including Why don't we put that to the RemoveDevice handler for network? We teardown all related stuff usually after unplug of the frontend.

> checking for multifunction PCI and valid alias, which is done in the > toplevel common code). For this reason, the Net function has been > split in two, with the 2nd half (qemuDomainDetachShutdownNet()) > called from the common function, right before sending the delete > command to qemu. > > Signed-off-by: Laine Stump <laine(a)laine.org&gt; > --- > src/qemu/qemu_hotplug.c | 497 ++++++++++++---------------------------- > 1 file changed, 149 insertions(+), 348 deletions(-) > > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c > index 5e5ffe16d3..de7a7a2c95 100644 > --- a/src/qemu/qemu_hotplug.c > +++ b/src/qemu/qemu_hotplug.c > @@ -5208,7 +5208,7 @@ qemuDomainRemoveRedirdevDevice(virQEMUDriverPtr driver, > } > > > -static inline void > +static void Use ATTRIBUTE_UNUSED rather than inline.

> qemuDomainRemoveAuditDevice(virDomainObjPtr vm, > virDomainDeviceDefPtr detach, > bool success)