[libvirt] [PATCH 4/4] xen: Fix virDomain{At,De}tachDevice

[Jim Fehlig]

Jiri Denemark wrote:
> According to API documentation virDomain{At,De}tachDevice calls are
> supposed to only work on active guests for device hotplug. For anything
> beyond that, their *Flags variants have to be used.
>
> Despite the variant which was acked on libvirt mailing list
> (https://www.redhat.com/archives/libvir-list/2010-January/msg00385.html)
> commit ed9c14a7ef86d7a45a6d57cbfee5410fca428633 (by Jim Fehlig)
> introduced automagic behavior of these API calls for xen driver. Since
> January, these calls always change persistent configuration of a guest
> and if the guest is currently active, they also hot(un)plug the device.
>
> That change didn't follow API documentation and also broke device
> hot(un)plug for older xend implementations which do not support changing
> persistent configuration of a guest and hot(un)plugging in one step.
>   

I only tested as far back as Xen 3.2.1, which is unfortunately still
xendConfigVersion 4.  This (nearly useless) version value was last
bumped by danpb  over 3 years ago -
http://xenbits.xensource.com/xen-unstable.hg?rev/887fa548f650 :-(.

> This patch should not break anything for active guests. On the other
> hand, changing inactive guests is not supported any more.
> ---
>  src/xen/xen_driver.c |   24 ++++++++++++++++++------
>  1 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
> index 56ba41b..e9eeab9 100644
> --- a/src/xen/xen_driver.c
> +++ b/src/xen/xen_driver.c
> @@ -1437,10 +1437,16 @@ xenUnifiedDomainAttachDevice (virDomainPtr dom, const char *xml)
>  {
>      GET_PRIVATE(dom->conn);
>      int i;
> -    unsigned int flags = VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
> +    unsigned int flags = VIR_DOMAIN_DEVICE_MODIFY_LIVE;
>  
> -    if (dom->id >= 0)
> -        flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE;
> +    /*
> +     * HACK: xend with xendConfigVersion >= 3 does not support changing live
> +     * config without touching persistent config, we add the extra flag here
> +     * to make this API work
> +     */
> +    if (priv->opened[XEN_UNIFIED_XEND_OFFSET] &&
> +        priv->xendConfigVersion >= 3)
> +        flags |= VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
>  
>      for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i)
>          if (priv->opened[i] && drivers[i]->domainAttachDeviceFlags &&
> @@ -1470,10 +1476,16 @@ xenUnifiedDomainDetachDevice (virDomainPtr dom, const char *xml)
>  {
>      GET_PRIVATE(dom->conn);
>      int i;
> -    unsigned int flags = VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
> +    unsigned int flags = VIR_DOMAIN_DEVICE_MODIFY_LIVE;
>  
> -    if (dom->id >= 0)
> -        flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE;
> +    /*
> +     * HACK: xend with xendConfigVersion >= 3 does not support changing live
> +     * config without touching persistent config, we add the extra flag here
> +     * to make this API work
> +     */
> +    if (priv->opened[XEN_UNIFIED_XEND_OFFSET] &&
> +        priv->xendConfigVersion >= 3)
> +        flags |= VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
>  
>      for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i)
>          if (priv->opened[i] && drivers[i]->domainDetachDeviceFlags &&
>   

ACK.  I have tested this on Xen 4.0.0 setup, both active and inactive pv
and hvm domUs.

Regards,
Jim