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