[libvirt] [PATCH 2/4] xen: Fix logic bug in xenDaemon*DeviceFlags

[Eric Blake]

On 10/01/2010 02:09 PM, Jiri Denemark wrote:
> ---
>   src/xen/xend_internal.c |   15 +++++++++------
>   1 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
> index 1318bd4..4fba6af 100644
> --- a/src/xen/xend_internal.c
> +++ b/src/xen/xend_internal.c
> @@ -3904,8 +3904,9 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml,
>           /* Xen only supports modifying both live and persistent config if
>            * xendConfigVersion>= 3
>            */
> -        if (flags != (VIR_DOMAIN_DEVICE_MODIFY_LIVE |
> -                      VIR_DOMAIN_DEVICE_MODIFY_CONFIG)) {
> +        if (priv->xendConfigVersion>= 3&&
> +            (flags != (VIR_DOMAIN_DEVICE_MODIFY_LIVE |
> +                       VIR_DOMAIN_DEVICE_MODIFY_CONFIG))) {
>               virXendError(VIR_ERR_OPERATION_INVALID, "%s",
>                            _("Xend only supports modifying both live and "
>                              "persistent config"));

The comment says:

_LIVE|_CONFIG is supported only if version>=3

logically, this tells me nothing about version < 3, nor about setting 
one but not both flags.

Meanwhile, the code says:

if version >=3, _LIVE|_CONFIG is the only supported combo

Are we sure this is the right change?  What happens with version < 3?

It seems like we need a 12-way table (in fact, this is pretty much what 
I ended up resorting to with my vcpus stuff).  Here's my shot at it, 
from reading the comments (but not actually testing it); once we fix 
this attempt to an actual table, then I can answer whether the code 
matches the table.


                   _LIVE     _CONFIG      _LIVE|_CONFIG
xen 2,running     good      unsupported  unsupported
xen 2,inactive    error     good         error or silently good
xen 3,running     good      good         good
xen 3,inactive    error     good         error or silently good

where the 'error or silently good' depends on our decision of whether an 
inactive domain should silently ignore _LIVE.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org