[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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 redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]