[libvirt] [PATCH v2 12/14] qemu_hotplug: consolidate all common detach code in qemuDomainDetachDeviceLive

[Peter Krempa]

On Mon, Mar 25, 2019 at 13:24:34 -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
>   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 at laine.org>
> ---
> 
> Change from V1:
> 
> * netdev shutdown is *not* moved to RemoveNet function - yet! That is
>   a good idea, but it is being done in a separate patch (14/14) in
>   order to make it easy to revert if we find it causes a problem.
> 
> * 
>  src/qemu/qemu_hotplug.c | 481 ++++++++++++----------------------------
>  1 file changed, 142 insertions(+), 339 deletions(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index e9d6c8622b..c86d1fb4eb 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c

[...]

> -        goto cleanup;
> -    }
> -
> -    if (async) {
> -        ret = 0;
> -    } else {
> -        if ((ret = qemuDomainWaitForDeviceRemoval(vm)) == 1)
> -            ret = qemuDomainRemoveNetDevice(driver, vm, net);
> -    }
> -
> - cleanup:
> -    if (!async)
> -        qemuDomainResetDeviceRemoval(vm);
> -    return ret;
>  }
>  
> -

We usually do two lines between functions so there's no need to break it
here.

>  static int
>  qemuDomainDetachDeviceChr(virQEMUDriverPtr driver,
>                            virDomainObjPtr vm,

ACK
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190326/344af667/attachment-0001.sig>