11:08 a.m.
This is the same as the previous series, but with standardized error
reporting macro to get consistent error messages.
This is just the first part of the previous series. The second part is
much more work to convert to this new error reporting macro, so i'll
drip-feed the rest for review over next few days...
Daniel P. Berrangé (14):
util: add a virReportEnumRangeError for bad value reporting
util: handle missing switch enum cases
conf: handle missing switch enum cases
esx: handle missing switch enum cases
hyperv: handle missing switch enum cases
libxl: handle missing switch enum cases
lxc: handle missing switch enum cases
nwfilter: handle missing switch enum cases
qemu: handle missing switch enum cases
rpc: handle missing switch enum cases
security: handle missing switch enum cases
xen: handle missing switch enum cases
tools: handle missing switch enum cases
m4: enforce that all enum cases are listed in switch statements
m4/virt-compile-warnings.m4 | 7 +++--
src/conf/domain_audit.c | 1 +
src/conf/domain_conf.c | 46 +++++++++++++++++++++++++++----
src/conf/nwfilter_conf.c | 31 ++++++++++++++++++++-
src/conf/nwfilter_conf.h | 4 +--
src/esx/esx_driver.c | 1 +
src/esx/esx_vi.c | 11 +++++---
src/esx/esx_vi_types.c | 9 +++---
src/hyperv/hyperv_driver.c | 18 ++++++++++--
src/libxl/libxl_driver.c | 2 +-
src/lxc/lxc_container.c | 8 +++---
src/lxc/lxc_controller.c | 8 +++++-
src/lxc/lxc_driver.c | 38 ++++++++++++++++++++++---
src/nwfilter/nwfilter_ebiptables_driver.c | 17 ++++++++----
src/nwfilter/nwfilter_learnipaddr.c | 6 +++-
src/qemu/qemu_command.c | 26 +++++++++++------
src/qemu/qemu_domain.c | 21 ++++++++++++++
src/qemu/qemu_driver.c | 25 +++++++++++------
src/qemu/qemu_hotplug.c | 33 ++++++++++++++++++----
src/qemu/qemu_migration.c | 11 +++++++-
src/qemu/qemu_process.c | 2 ++
src/rpc/virnetclient.c | 2 ++
src/rpc/virnetclientprogram.c | 1 +
src/rpc/virnetserverprogram.c | 4 +++
src/security/security_driver.c | 1 +
src/util/virconf.c | 11 +++++++-
src/util/virerror.h | 9 +++++-
src/util/virfirewall.c | 6 ++--
src/util/virlog.c | 9 +++++-
src/util/virnetdevvportprofile.c | 10 ++++++-
src/vmx/vmx.c | 26 +++++++++++++++--
src/xenconfig/xen_common.c | 18 ++++++++++--
src/xenconfig/xen_xl.c | 7 ++++-
tools/virt-host-validate-qemu.c | 3 +-
34 files changed, 359 insertions(+), 73 deletions(-)
--
2.14.3
11:08 a.m.
New subject: [libvirt] [PATCH v2 01/14] util: add a virReportEnumRangeError for bad value reporting
To ensure we have standardized error messages when reporting problems
with enum values being out of a range, add virReportEnumRangeError().
virReportEnumRangeError(virDomainState, 34);
results in a message
"internal error: Unexpected enum value 34 for virDomainState"
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/util/virerror.h | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/util/virerror.h b/src/util/virerror.h
index cf434f45fc..1d02451604 100644
--- a/src/util/virerror.h
+++ b/src/util/virerror.h
@@ -164,7 +164,14 @@ void virReportSystemErrorFull(int domcode,
# define virReportRestrictedError(...) \
virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_OPERATION_DENIED, \
__FILE__, __FUNCTION__, __LINE__, __VA_ARGS__)
-
+/* The sizeof(typname) comparison here is a hack to catch typos
+ * in the name of the enum by triggering a compile error. It should
+ * get optimized away since sizeof() is known at compile time */
+# define virReportEnumRangeError(typname, value) \
+ virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INTERNAL_ERROR, \
+ __FILE__, __FUNCTION__, __LINE__, \
+ "Unexpected enum value %d for %s", \
+ value, sizeof(typname) != 0 ? #typname : #typname);
void virReportOOMErrorFull(int domcode,
const char *filename,
--
2.14.3
12:42 p.m.
New subject: [libvirt] [PATCH v2 01/14] util: add a virReportEnumRangeError for bad value reporting
On 02/20/2018 12:08 PM, Daniel P. Berrangé wrote:
To ensure we have standardized error messages when reporting
problems
with enum values being out of a range, add virReportEnumRangeError().
virReportEnumRangeError(virDomainState, 34);
results in a message
"internal error: Unexpected enum value 34 for virDomainState"
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/util/virerror.h | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/util/virerror.h b/src/util/virerror.h
index cf434f45fc..1d02451604 100644
--- a/src/util/virerror.h
+++ b/src/util/virerror.h
@@ -164,7 +164,14 @@ void virReportSystemErrorFull(int domcode,
# define virReportRestrictedError(...) \
virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_OPERATION_DENIED, \
__FILE__, __FUNCTION__, __LINE__, __VA_ARGS__)
-
+/* The sizeof(typname) comparison here is a hack to catch typos
+ * in the name of the enum by triggering a compile error. It should
+ * get optimized away since sizeof() is known at compile time */
+# define virReportEnumRangeError(typname, value) \
+ virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INTERNAL_ERROR, \
+ __FILE__, __FUNCTION__, __LINE__, \
+ "Unexpected enum value %d for %s", \
Need to resolve syntax-check warning about right-aligned "\"...
+ value, sizeof(typname) != 0 ? #typname :
#typname);
I saw some internal IRC over the sizeof check... Assuming Eric and you
come up with something reasonable...
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
Even though I'm not 100% on board with the whole error message for LAST
and default especially when they're a cannot get there...
void virReportOOMErrorFull(int domcode,
const char *filename,
11:08 a.m.
New subject: [libvirt] [PATCH v2 02/14] util: handle missing switch enum cases
Ensure all enum cases are listed in switch statements.
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/util/virconf.c | 11 ++++++++++-
src/util/virfirewall.c | 6 ++++--
src/util/virlog.c | 9 ++++++++-
src/util/virnetdevvportprofile.c | 10 +++++++++-
4 files changed, 31 insertions(+), 5 deletions(-)
diff --git a/src/util/virconf.c b/src/util/virconf.c
index a82a509ca3..e0a3fd12c0 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -296,7 +296,9 @@ virConfSaveValue(virBufferPtr buf, virConfValuePtr val)
virBufferAddLit(buf, " ]");
break;
}
+ case VIR_CONF_LAST:
default:
+ virReportEnumRangeError(virConfType, val->type);
return -1;
}
return 0;
@@ -986,13 +988,20 @@ int virConfGetValueStringList(virConfPtr conf,
}
ATTRIBUTE_FALLTHROUGH;
- default:
+ case VIR_CONF_LLONG:
+ case VIR_CONF_ULLONG:
+ case VIR_CONF_NONE:
virReportError(VIR_ERR_INTERNAL_ERROR,
compatString ?
_("%s: expected a string or string list for '%s'
parameter") :
_("%s: expected a string list for '%s'
parameter"),
conf->filename, setting);
return -1;
+
+ case VIR_CONF_LAST:
+ default:
+ virReportEnumRangeError(virConfType, cval->type);
+ return -1;
}
return 1;
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index ff1bb83073..10c370a2ae 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -827,9 +827,11 @@ virFirewallApplyRule(virFirewallPtr firewall,
if (virFirewallApplyRuleFirewallD(rule, ignoreErrors, &output) < 0)
return -1;
break;
+
+ case VIR_FIREWALL_BACKEND_AUTOMATIC:
+ case VIR_FIREWALL_BACKEND_LAST:
default:
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Unexpected firewall engine backend"));
+ virReportEnumRangeError(virFirewallBackend, currentBackend);
return -1;
}
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 4f66cc5e5c..6c6d7e8ded 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -1206,10 +1206,17 @@ virLogGetOutputs(void)
virLogDestinationTypeToString(dest),
virLogOutputs[i]->name);
break;
- default:
+ case VIR_LOG_TO_STDERR:
+ case VIR_LOG_TO_JOURNALD:
virBufferAsprintf(&outputbuf, "%d:%s",
virLogOutputs[i]->priority,
virLogDestinationTypeToString(dest));
+ break;
+ case VIR_LOG_TO_OUTPUT_LAST:
+ default:
+ virReportEnumRangeError(virLogDestination, dest);
+ virLogUnlock();
+ return NULL;
}
}
virLogUnlock();
diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c
index db495a7549..dc774407df 100644
--- a/src/util/virnetdevvportprofile.c
+++ b/src/util/virnetdevvportprofile.c
@@ -1071,6 +1071,9 @@ virNetDevVPortProfileOp8021Qbg(const char *ifname,
case VIR_NETDEV_VPORT_PROFILE_LINK_OP_PREASSOCIATE:
op = PORT_REQUEST_PREASSOCIATE;
break;
+ case VIR_NETDEV_VPORT_PROFILE_LINK_OP_PREASSOCIATE_RR:
+ op = PORT_REQUEST_PREASSOCIATE_RR;
+ break;
case VIR_NETDEV_VPORT_PROFILE_LINK_OP_ASSOCIATE:
op = PORT_REQUEST_ASSOCIATE;
break;
@@ -1191,10 +1194,15 @@ virNetDevVPortProfileOp8021Qbh(const char *ifname,
false);
break;
- default:
+ case VIR_NETDEV_VPORT_PROFILE_LINK_OP_PREASSOCIATE:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("operation type %d not supported"), virtPortOp);
rc = -1;
+ break;
+ default:
+ virReportEnumRangeError(virNetDevVPortProfile, virtPortOp);
+ rc = -1;
+ break;
}
cleanup:
--
2.14.3
12:44 p.m.
New subject: [libvirt] [PATCH v2 02/14] util: handle missing switch enum cases
On 02/20/2018 12:08 PM, Daniel P. Berrangé wrote:
Ensure all enum cases are listed in switch statements.
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/util/virconf.c | 11 ++++++++++-
src/util/virfirewall.c | 6 ++++--
src/util/virlog.c | 9 ++++++++-
src/util/virnetdevvportprofile.c | 10 +++++++++-
4 files changed, 31 insertions(+), 5 deletions(-)
[...]
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 4f66cc5e5c..6c6d7e8ded 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -1206,10 +1206,17 @@ virLogGetOutputs(void)
virLogDestinationTypeToString(dest),
virLogOutputs[i]->name);
break;
- default:
+ case VIR_LOG_TO_STDERR:
+ case VIR_LOG_TO_JOURNALD:
virBufferAsprintf(&outputbuf, "%d:%s",
virLogOutputs[i]->priority,
virLogDestinationTypeToString(dest));
+ break;
+ case VIR_LOG_TO_OUTPUT_LAST:
+ default:
+ virReportEnumRangeError(virLogDestination, dest);
Didn't see this before, but because this is a for loop, you'll need to
add a virBufferFreeAndReset(&outputbuf); before returning.
Existing R-b still applies
John
+ virLogUnlock();
+ return NULL;
}
}
virLogUnlock();
3:26 a.m.
New subject: [libvirt] [PATCH v2 02/14] util: handle missing switch enum cases
On Tue, Feb 20, 2018 at 01:44:31PM -0500, John Ferlan wrote:
On 02/20/2018 12:08 PM, Daniel P. Berrangé wrote:
> Ensure all enum cases are listed in switch statements.
>
> Reviewed-by: John Ferlan <jferlan(a)redhat.com>
> Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
> ---
> src/util/virconf.c | 11 ++++++++++-
> src/util/virfirewall.c | 6 ++++--
> src/util/virlog.c | 9 ++++++++-
> src/util/virnetdevvportprofile.c | 10 +++++++++-
> 4 files changed, 31 insertions(+), 5 deletions(-)
>
[...]
> diff --git a/src/util/virlog.c b/src/util/virlog.c
> index 4f66cc5e5c..6c6d7e8ded 100644
> --- a/src/util/virlog.c
> +++ b/src/util/virlog.c
> @@ -1206,10 +1206,17 @@ virLogGetOutputs(void)
> virLogDestinationTypeToString(dest),
> virLogOutputs[i]->name);
> break;
> - default:
> + case VIR_LOG_TO_STDERR:
> + case VIR_LOG_TO_JOURNALD:
> virBufferAsprintf(&outputbuf, "%d:%s",
> virLogOutputs[i]->priority,
> virLogDestinationTypeToString(dest));
> + break;
> + case VIR_LOG_TO_OUTPUT_LAST:
> + default:
> + virReportEnumRangeError(virLogDestination, dest);
Didn't see this before, but because this is a for loop, you'll need to
add a virBufferFreeAndReset(&outputbuf); before returning.
Ok
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
11:08 a.m.
New subject: [libvirt] [PATCH v2 03/14] conf: handle missing switch enum cases
Ensure all enum cases are listed in switch statements.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/conf/domain_audit.c | 1 +
src/conf/domain_conf.c | 46 ++++++++++++++++++++++++++++++++++++++++------
src/conf/nwfilter_conf.c | 31 ++++++++++++++++++++++++++++++-
3 files changed, 71 insertions(+), 7 deletions(-)
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index 723c737363..82868bca76 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -586,6 +586,7 @@ virDomainAuditTPM(virDomainObjPtr vm, virDomainTPMDefPtr tpm,
"virt=%s resrc=dev reason=%s %s uuid=%s %s",
virt, reason, vmname, uuidstr, device);
break;
+ case VIR_DOMAIN_TPM_TYPE_LAST:
default:
break;
}
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 68a8d39016..f2ddde7a36 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11567,8 +11567,20 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
def->filterparams = filterparams;
filterparams = NULL;
break;
- default:
+ case VIR_DOMAIN_NET_TYPE_USER:
+ case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+ case VIR_DOMAIN_NET_TYPE_SERVER:
+ case VIR_DOMAIN_NET_TYPE_CLIENT:
+ case VIR_DOMAIN_NET_TYPE_MCAST:
+ case VIR_DOMAIN_NET_TYPE_INTERNAL:
+ case VIR_DOMAIN_NET_TYPE_DIRECT:
+ case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+ case VIR_DOMAIN_NET_TYPE_UDP:
break;
+ case VIR_DOMAIN_NET_TYPE_LAST:
+ default:
+ virReportEnumRangeError(virDomainNetType, def->type);
+ goto error;
}
}
@@ -14703,6 +14715,12 @@ virDomainVideoDefaultRAM(const virDomainDef *def,
/* QEMU use 64M as the minimal video memory for qxl device */
return 64 * 1024;
+ case VIR_DOMAIN_VIDEO_TYPE_DEFAULT:
+ case VIR_DOMAIN_VIDEO_TYPE_VBOX:
+ case VIR_DOMAIN_VIDEO_TYPE_PARALLELS:
+ case VIR_DOMAIN_VIDEO_TYPE_VIRTIO:
+ case VIR_DOMAIN_VIDEO_TYPE_GOP:
+ case VIR_DOMAIN_VIDEO_TYPE_LAST:
default:
return 0;
}
@@ -14737,6 +14755,16 @@ virDomainVideoDefaultType(const virDomainDef *def)
return VIR_DOMAIN_VIDEO_TYPE_PARALLELS;
case VIR_DOMAIN_VIRT_BHYVE:
return VIR_DOMAIN_VIDEO_TYPE_GOP;
+ case VIR_DOMAIN_VIRT_QEMU:
+ case VIR_DOMAIN_VIRT_KQEMU:
+ case VIR_DOMAIN_VIRT_KVM:
+ case VIR_DOMAIN_VIRT_LXC:
+ case VIR_DOMAIN_VIRT_UML:
+ case VIR_DOMAIN_VIRT_OPENVZ:
+ case VIR_DOMAIN_VIRT_HYPERV:
+ case VIR_DOMAIN_VIRT_PHYP:
+ case VIR_DOMAIN_VIRT_NONE:
+ case VIR_DOMAIN_VIRT_LAST:
default:
return VIR_DOMAIN_VIDEO_TYPE_DEFAULT;
}
@@ -27878,7 +27906,7 @@ virDomainObjGetState(virDomainObjPtr dom, int *reason)
void
virDomainObjSetState(virDomainObjPtr dom, virDomainState state, int reason)
{
- int last = -1;
+ int last;
switch (state) {
case VIR_DOMAIN_NOSTATE:
@@ -27905,11 +27933,8 @@ virDomainObjSetState(virDomainObjPtr dom, virDomainState state,
int reason)
case VIR_DOMAIN_PMSUSPENDED:
last = VIR_DOMAIN_PMSUSPENDED_LAST;
break;
+ case VIR_DOMAIN_LAST:
default:
- last = -1;
- }
-
- if (last < 0) {
VIR_ERROR(_("invalid domain state: %d"), state);
return;
}
@@ -28073,6 +28098,15 @@ virDomainNetGetActualVirtPortProfile(virDomainNetDefPtr iface)
default:
return NULL;
}
+ case VIR_DOMAIN_NET_TYPE_USER:
+ case VIR_DOMAIN_NET_TYPE_ETHERNET:
+ case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+ case VIR_DOMAIN_NET_TYPE_SERVER:
+ case VIR_DOMAIN_NET_TYPE_CLIENT:
+ case VIR_DOMAIN_NET_TYPE_MCAST:
+ case VIR_DOMAIN_NET_TYPE_INTERNAL:
+ case VIR_DOMAIN_NET_TYPE_UDP:
+ case VIR_DOMAIN_NET_TYPE_LAST:
default:
return NULL;
}
diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index 5d6423e060..fd42d58c2c 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -2153,8 +2153,34 @@ virNWFilterRuleValidate(virNWFilterRuleDefPtr rule)
}
}
break;
- default:
+ case VIR_NWFILTER_RULE_PROTOCOL_NONE:
+ case VIR_NWFILTER_RULE_PROTOCOL_MAC:
+ case VIR_NWFILTER_RULE_PROTOCOL_VLAN:
+ case VIR_NWFILTER_RULE_PROTOCOL_STP:
+ case VIR_NWFILTER_RULE_PROTOCOL_ARP:
+ case VIR_NWFILTER_RULE_PROTOCOL_RARP:
+ case VIR_NWFILTER_RULE_PROTOCOL_TCP:
+ case VIR_NWFILTER_RULE_PROTOCOL_ICMP:
+ case VIR_NWFILTER_RULE_PROTOCOL_IGMP:
+ case VIR_NWFILTER_RULE_PROTOCOL_UDP:
+ case VIR_NWFILTER_RULE_PROTOCOL_UDPLITE:
+ case VIR_NWFILTER_RULE_PROTOCOL_ESP:
+ case VIR_NWFILTER_RULE_PROTOCOL_AH:
+ case VIR_NWFILTER_RULE_PROTOCOL_SCTP:
+ case VIR_NWFILTER_RULE_PROTOCOL_ALL:
+ case VIR_NWFILTER_RULE_PROTOCOL_TCPoIPV6:
+ case VIR_NWFILTER_RULE_PROTOCOL_ICMPV6:
+ case VIR_NWFILTER_RULE_PROTOCOL_UDPoIPV6:
+ case VIR_NWFILTER_RULE_PROTOCOL_UDPLITEoIPV6:
+ case VIR_NWFILTER_RULE_PROTOCOL_ESPoIPV6:
+ case VIR_NWFILTER_RULE_PROTOCOL_AHoIPV6:
+ case VIR_NWFILTER_RULE_PROTOCOL_SCTPoIPV6:
+ case VIR_NWFILTER_RULE_PROTOCOL_ALLoIPV6:
break;
+ case VIR_NWFILTER_RULE_PROTOCOL_LAST:
+ default:
+ virReportEnumRangeError(virNWFilterRuleProtocolType, rule->prtclType);
+ return -1;
}
return ret;
@@ -3064,7 +3090,10 @@ virNWFilterRuleDefDetailsFormat(virBufferPtr buf,
virBufferAddLit(buf, "false");
break;
+ case DATATYPE_IPSETNAME:
+ case DATATYPE_IPSETFLAGS:
case DATATYPE_STRING:
+ case DATATYPE_LAST:
default:
virBufferAsprintf(buf,
"UNSUPPORTED DATATYPE 0x%02x\n",
--
2.14.3
12:50 p.m.
New subject: [libvirt] [PATCH v2 03/14] conf: handle missing switch enum cases
On 02/20/2018 12:08 PM, Daniel P. Berrangé wrote:
Ensure all enum cases are listed in switch statements.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/conf/domain_audit.c | 1 +
src/conf/domain_conf.c | 46 ++++++++++++++++++++++++++++++++++++++++------
src/conf/nwfilter_conf.c | 31 ++++++++++++++++++++++++++++++-
3 files changed, 71 insertions(+), 7 deletions(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
11:08 a.m.
New subject: [libvirt] [PATCH v2 04/14] esx: handle missing switch enum cases
Ensure all enum cases are listed in switch statements, or explicitly
cast away enum type where we don't want to list all cases.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/esx/esx_driver.c | 1 +
src/esx/esx_vi.c | 11 +++++++----
src/esx/esx_vi_types.c | 9 +++++----
3 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index f575362059..c5f04efa81 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -3603,6 +3603,7 @@ esxDomainGetSchedulerParametersFlags(virDomainPtr domain,
params[i].value.i = -3;
break;
+ case esxVI_SharesLevel_Undefined:
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Shares level has unknown value %d"),
diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index edf52ff828..103f726069 100644
--- a/src/esx/esx_vi.c
+++ b/src/esx/esx_vi.c
@@ -483,7 +483,7 @@ esxVI_SharedCURL_Lock(CURL *handle ATTRIBUTE_UNUSED, curl_lock_data
data,
size_t i;
esxVI_SharedCURL *shared = userptr;
- switch (data) {
+ switch ((int)data) {
case CURL_LOCK_DATA_SHARE:
i = 0;
break;
@@ -511,7 +511,7 @@ esxVI_SharedCURL_Unlock(CURL *handle ATTRIBUTE_UNUSED, curl_lock_data
data,
size_t i;
esxVI_SharedCURL *shared = userptr;
- switch (data) {
+ switch ((int)data) {
case CURL_LOCK_DATA_SHARE:
i = 0;
break;
@@ -1563,6 +1563,7 @@ esxVI_Context_Execute(esxVI_Context *ctx, const char *methodName,
break;
+ case esxVI_Occurrence_Undefined:
default:
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Invalid argument (occurrence)"));
@@ -2280,9 +2281,11 @@ esxVI_LookupObjectContentByType(esxVI_Context *ctx,
type, root->type);
break;
+ case esxVI_Occurrence_None:
+ case esxVI_Occurrence_Undefined:
default:
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Invalid occurrence value"));
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Invalid occurrence value %d"), occurrence);
break;
}
diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c
index be35af861c..74183f8307 100644
--- a/src/esx/esx_vi_types.c
+++ b/src/esx/esx_vi_types.c
@@ -544,7 +544,7 @@ VIR_LOG_INIT("esx.esx_vi_types");
#define ESX_VI__TEMPLATE__DISPATCH(_actual_type, _actual_type_name, __type, \
_dispatch, _error_return) \
- switch (_actual_type) { \
+ switch ((int)_actual_type) { \
_dispatch \
\
case esxVI_Type_##__type: \
@@ -690,7 +690,7 @@ VIR_LOG_INIT("esx.esx_vi_types");
return -1; \
} \
\
- switch (type) { \
+ switch ((int)type) { \
_dispatch \
\
case esxVI_Type_##__type: \
@@ -967,7 +967,7 @@ esxVI_AnyType_DeepCopy(esxVI_AnyType **dest, esxVI_AnyType *src)
goto failure;
}
- switch (src->type) {
+ switch ((int)src->type) {
case esxVI_Type_Boolean:
(*dest)->boolean = src->boolean;
break;
@@ -1071,7 +1071,7 @@ esxVI_AnyType_Deserialize(xmlNodePtr node, esxVI_AnyType **anyType)
(*anyType)->_name = number; \
} while (0)
- switch ((*anyType)->type) {
+ switch ((int)(*anyType)->type) {
case esxVI_Type_Boolean:
if (STREQ((*anyType)->value, "true")) {
(*anyType)->boolean = esxVI_Boolean_True;
@@ -1876,6 +1876,7 @@ esxVI_VirtualMachinePowerState_ConvertToLibvirt
case esxVI_VirtualMachinePowerState_Suspended:
return VIR_DOMAIN_PAUSED;
+ case esxVI_VirtualMachinePowerState_Undefined:
default:
return VIR_DOMAIN_NOSTATE;
}
--
2.14.3
1:03 p.m.
New subject: [libvirt] [PATCH v2 04/14] esx: handle missing switch enum cases
On 02/20/2018 12:08 PM, Daniel P. Berrangé wrote:
Ensure all enum cases are listed in switch statements, or explicitly
cast away enum type where we don't want to list all cases.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/esx/esx_driver.c | 1 +
src/esx/esx_vi.c | 11 +++++++----
src/esx/esx_vi_types.c | 9 +++++----
3 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index f575362059..c5f04efa81 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -3603,6 +3603,7 @@ esxDomainGetSchedulerParametersFlags(virDomainPtr domain,
params[i].value.i = -3;
break;
+ case esxVI_SharesLevel_Undefined:
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Shares level has unknown value %d"),
Shouldn't this be:
virReportEnumRangeError(esxVI_SharesLevel, sharesInfo->level);
diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index edf52ff828..103f726069 100644
--- a/src/esx/esx_vi.c
+++ b/src/esx/esx_vi.c
@@ -483,7 +483,7 @@ esxVI_SharedCURL_Lock(CURL *handle ATTRIBUTE_UNUSED, curl_lock_data
data,
size_t i;
esxVI_SharedCURL *shared = userptr;
- switch (data) {
+ switch ((int)data) {
case CURL_LOCK_DATA_SHARE:
i = 0;
break;
@@ -511,7 +511,7 @@ esxVI_SharedCURL_Unlock(CURL *handle ATTRIBUTE_UNUSED, curl_lock_data
data,
size_t i;
esxVI_SharedCURL *shared = userptr;
- switch (data) {
+ switch ((int)data) {
case CURL_LOCK_DATA_SHARE:
i = 0;
break;
@@ -1563,6 +1563,7 @@ esxVI_Context_Execute(esxVI_Context *ctx, const char *methodName,
break;
+ case esxVI_Occurrence_Undefined:
default:
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Invalid argument (occurrence)"));
and:
virReportEnumRangeError(esxVI_Occurrence, occurrence);
@@ -2280,9 +2281,11 @@ esxVI_LookupObjectContentByType(esxVI_Context
*ctx,
type, root->type);
break;
+ case esxVI_Occurrence_None:
+ case esxVI_Occurrence_Undefined:
default:
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Invalid occurrence value"));
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Invalid occurrence value %d"), occurrence);
and:
virReportEnumRangeError(esxVI_Occurrence, occurrence);
break;
}
diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c
index be35af861c..74183f8307 100644
--- a/src/esx/esx_vi_types.c
+++ b/
@@ -544,7 +544,7 @@ VIR_LOG_INIT("esx.esx_vi_types");
#define ESX_VI__TEMPLATE__DISPATCH(_actual_type, _actual_type_name, __type, \
_dispatch, _error_return) \
- switch (_actual_type) { \
+ switch ((int)_actual_type) { \
_dispatch \
\
case esxVI_Type_##__type: \
@@ -690,7 +690,7 @@ VIR_LOG_INIT("esx.esx_vi_types");
return -1; \
} \
\
- switch (type) { \
+ switch ((int)type) { \
_dispatch \
\
case esxVI_Type_##__type: \
@@ -967,7 +967,7 @@ esxVI_AnyType_DeepCopy(esxVI_AnyType **dest, esxVI_AnyType *src)
goto failure;
}
- switch (src->type) {
+ switch ((int)src->type) {
case esxVI_Type_Boolean:
(*dest)->boolean = src->boolean;
break;
@@ -1071,7 +1071,7 @@ esxVI_AnyType_Deserialize(xmlNodePtr node, esxVI_AnyType
**anyType)
(*anyType)->_name = number; \
} while (0)
- switch ((*anyType)->type) {
+ switch ((int)(*anyType)->type) {
case esxVI_Type_Boolean:
if (STREQ((*anyType)->value, "true")) {
(*anyType)->boolean = esxVI_Boolean_True;
@@ -1876,6 +1876,7 @@ esxVI_VirtualMachinePowerState_ConvertToLibvirt
case esxVI_VirtualMachinePowerState_Suspended:
return VIR_DOMAIN_PAUSED;
+ case esxVI_VirtualMachinePowerState_Undefined:
Fix indent - off by 2.
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
default:
return VIR_DOMAIN_NOSTATE;
}
3:26 a.m.
New subject: [libvirt] [PATCH v2 04/14] esx: handle missing switch enum cases
On Tue, Feb 20, 2018 at 02:03:19PM -0500, John Ferlan wrote:
On 02/20/2018 12:08 PM, Daniel P. Berrangé wrote:
> Ensure all enum cases are listed in switch statements, or explicitly
> cast away enum type where we don't want to list all cases.
>
> Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
> ---
> src/esx/esx_driver.c | 1 +
> src/esx/esx_vi.c | 11 +++++++----
> src/esx/esx_vi_types.c | 9 +++++----
> 3 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
> index f575362059..c5f04efa81 100644
> --- a/src/esx/esx_driver.c
> +++ b/src/esx/esx_driver.c
> @@ -3603,6 +3603,7 @@ esxDomainGetSchedulerParametersFlags(virDomainPtr domain,
> params[i].value.i = -3;
> break;
>
> + case esxVI_SharesLevel_Undefined:
> default:
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Shares level has unknown value %d"),
Shouldn't this be:
virReportEnumRangeError(esxVI_SharesLevel, sharesInfo->level);
Yeah, I had only changed messages I added in v1, but thse can be touched
up too.
> diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
> index edf52ff828..103f726069 100644
> --- a/src/esx/esx_vi.c
> +++ b/src/esx/esx_vi.c
> @@ -483,7 +483,7 @@ esxVI_SharedCURL_Lock(CURL *handle ATTRIBUTE_UNUSED,
curl_lock_data data,
> size_t i;
> esxVI_SharedCURL *shared = userptr;
>
> - switch (data) {
> + switch ((int)data) {
> case CURL_LOCK_DATA_SHARE:
> i = 0;
> break;
> @@ -511,7 +511,7 @@ esxVI_SharedCURL_Unlock(CURL *handle ATTRIBUTE_UNUSED,
curl_lock_data data,
> size_t i;
> esxVI_SharedCURL *shared = userptr;
>
> - switch (data) {
> + switch ((int)data) {
> case CURL_LOCK_DATA_SHARE:
> i = 0;
> break;
> @@ -1563,6 +1563,7 @@ esxVI_Context_Execute(esxVI_Context *ctx, const char
*methodName,
>
> break;
>
> + case esxVI_Occurrence_Undefined:
> default:
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Invalid argument (occurrence)"));
and:
virReportEnumRangeError(esxVI_Occurrence, occurrence);
> @@ -2280,9 +2281,11 @@ esxVI_LookupObjectContentByType(esxVI_Context *ctx,
> type, root->type);
> break;
>
> + case esxVI_Occurrence_None:
> + case esxVI_Occurrence_Undefined:
> default:
> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("Invalid occurrence value"));
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Invalid occurrence value %d"),
occurrence);
and:
virReportEnumRangeError(esxVI_Occurrence, occurrence);
> break;
> }
>
> diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c
> index be35af861c..74183f8307 100644
> --- a/src/esx/esx_vi_types.c
> +++ b/
> @@ -544,7 +544,7 @@ VIR_LOG_INIT("esx.esx_vi_types");
>
> #define ESX_VI__TEMPLATE__DISPATCH(_actual_type, _actual_type_name, __type, \
> _dispatch, _error_return) \
> - switch (_actual_type) { \
> + switch ((int)_actual_type) { \
> _dispatch \
> \
> case esxVI_Type_##__type: \
> @@ -690,7 +690,7 @@ VIR_LOG_INIT("esx.esx_vi_types");
> return -1; \
> } \
> \
> - switch (type) { \
> + switch ((int)type) { \
> _dispatch \
> \
> case esxVI_Type_##__type: \
> @@ -967,7 +967,7 @@ esxVI_AnyType_DeepCopy(esxVI_AnyType **dest, esxVI_AnyType
*src)
> goto failure;
> }
>
> - switch (src->type) {
> + switch ((int)src->type) {
> case esxVI_Type_Boolean:
> (*dest)->boolean = src->boolean;
> break;
> @@ -1071,7 +1071,7 @@ esxVI_AnyType_Deserialize(xmlNodePtr node, esxVI_AnyType
**anyType)
> (*anyType)->_name = number; \
> } while (0)
>
> - switch ((*anyType)->type) {
> + switch ((int)(*anyType)->type) {
> case esxVI_Type_Boolean:
> if (STREQ((*anyType)->value, "true")) {
> (*anyType)->boolean = esxVI_Boolean_True;
> @@ -1876,6 +1876,7 @@ esxVI_VirtualMachinePowerState_ConvertToLibvirt
> case esxVI_VirtualMachinePowerState_Suspended:
> return VIR_DOMAIN_PAUSED;
>
> + case esxVI_VirtualMachinePowerState_Undefined:
Fix indent - off by 2.
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
> default:
> return VIR_DOMAIN_NOSTATE;
> }
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
11:08 a.m.
New subject: [libvirt] [PATCH v2 05/14] hyperv: handle missing switch enum cases
Ensure all enum cases are listed in switch statements. This improves
debug logging integration with openwsman.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/hyperv/hyperv_driver.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index ee94fd3511..e512b626ea 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -1656,13 +1656,27 @@ hypervDebugHandler(const char *message, debug_level_e level,
switch (level) {
case DEBUG_LEVEL_ERROR:
case DEBUG_LEVEL_CRITICAL:
- VIR_ERROR(_("openwsman error: %s"), message);
+ case DEBUG_LEVEL_ALWAYS:
+ VIR_ERROR(_("openwsman: %s"), message);
break;
case DEBUG_LEVEL_WARNING:
- VIR_WARN("openwsman warning: %s", message);
+ VIR_WARN("openwsman: %s", message);
break;
+ case DEBUG_LEVEL_MESSAGE:
+ VIR_INFO("openwsman: %s", message);
+ break;
+
+ case DEBUG_LEVEL_INFO:
+ VIR_INFO("openwsman: %s", message);
+ break;
+
+ case DEBUG_LEVEL_DEBUG:
+ VIR_DEBUG("openwsman: %s", message);
+ break;
+
+ case DEBUG_LEVEL_NONE:
default:
/* Ignore the rest */
break;
--
2.14.3
1:04 p.m.
New subject: [libvirt] [PATCH v2 05/14] hyperv: handle missing switch enum cases
On 02/20/2018 12:08 PM, Daniel P. Berrangé wrote:
Ensure all enum cases are listed in switch statements. This improves
debug logging integration with openwsman.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/hyperv/hyperv_driver.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
11:08 a.m.
New subject: [libvirt] [PATCH v2 06/14] libxl: handle missing switch enum cases
Cast away enum type for libxl schedular constants since we don't want to
cover all of them and don't want build to break when new ones are added.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/libxl/libxl_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index be11134fb2..4b52de36f5 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4497,7 +4497,7 @@ libxlDomainGetSchedulerType(virDomainPtr dom, int *nparams)
if (nparams)
*nparams = 0;
- switch (sched_id) {
+ switch ((int)sched_id) {
case LIBXL_SCHEDULER_SEDF:
name = "sedf";
break;
--
2.14.3
1:05 p.m.
New subject: [libvirt] [PATCH v2 06/14] libxl: handle missing switch enum cases
On 02/20/2018 12:08 PM, Daniel P. Berrangé wrote:
Cast away enum type for libxl schedular constants since we don't
want to
cover all of them and don't want build to break when new ones are added.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/libxl/libxl_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
3:35 a.m.
New subject: [libvirt] [PATCH v2 06/14] libxl: handle missing switch enum cases
On Tue, Feb 20, 2018 at 05:08:14PM +0000, Daniel P. Berrangé wrote:
Cast away enum type for libxl schedular constants since we don't
want to
s/schedular/scheduler/
cover all of them and don't want build to break when new ones are
added.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/libxl/libxl_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index be11134fb2..4b52de36f5 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4497,7 +4497,7 @@ libxlDomainGetSchedulerType(virDomainPtr dom, int *nparams)
if (nparams)
*nparams = 0;
- switch (sched_id) {
+ switch ((int)sched_id) {
We should get consistent with the spacing after cast and add it to HACKING.
This way I can't tell you to add a space there =)
case LIBXL_SCHEDULER_SEDF:
name = "sedf";
break;
--
2.14.3
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
3:39 a.m.
New subject: [libvirt] [PATCH v2 06/14] libxl: handle missing switch enum cases
On Wed, Feb 21, 2018 at 10:35:04AM +0100, Martin Kletzander wrote:
On Tue, Feb 20, 2018 at 05:08:14PM +0000, Daniel P. Berrangé wrote:
> Cast away enum type for libxl schedular constants since we don't want to
s/schedular/scheduler/
> cover all of them and don't want build to break when new ones are added.
>
> Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
> ---
> src/libxl/libxl_driver.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index be11134fb2..4b52de36f5 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -4497,7 +4497,7 @@ libxlDomainGetSchedulerType(virDomainPtr dom, int *nparams)
>
> if (nparams)
> *nparams = 0;
> - switch (sched_id) {
> + switch ((int)sched_id) {
We should get consistent with the spacing after cast and add it to HACKING.
This way I can't tell you to add a space there =)
Adding a space between a typecast and a variable is a-typical in general,
so I don't see why we should do it for switch() statements.
> case LIBXL_SCHEDULER_SEDF:
> name = "sedf";
> break;
> --
> 2.14.3
>
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
4 a.m.
New subject: [libvirt] [PATCH v2 06/14] libxl: handle missing switch enum cases
On Wed, Feb 21, 2018 at 09:39:07AM +0000, Daniel P. Berrangé wrote:
On Wed, Feb 21, 2018 at 10:35:04AM +0100, Martin Kletzander wrote:
> On Tue, Feb 20, 2018 at 05:08:14PM +0000, Daniel P. Berrangé wrote:
> > Cast away enum type for libxl schedular constants since we don't want to
>
> s/schedular/scheduler/
>
> > cover all of them and don't want build to break when new ones are added.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
> > ---
> > src/libxl/libxl_driver.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> > index be11134fb2..4b52de36f5 100644
> > --- a/src/libxl/libxl_driver.c
> > +++ b/src/libxl/libxl_driver.c
> > @@ -4497,7 +4497,7 @@ libxlDomainGetSchedulerType(virDomainPtr dom, int
*nparams)
> >
> > if (nparams)
> > *nparams = 0;
> > - switch (sched_id) {
> > + switch ((int)sched_id) {
>
> We should get consistent with the spacing after cast and add it to HACKING.
> This way I can't tell you to add a space there =)
Adding a space between a typecast and a variable is a-typical in general,
so I don't see why we should do it for switch() statements.
Well, I didn't use to do it, but nowadays I do since it's more clearly readable.
Plus it looks like majority casts in our code use spaces. I'll try create a
syntax-check and make some statistics, we'll see if that works or not. I didn't
mean to pollute this series with unrelated discussion.
>
> > case LIBXL_SCHEDULER_SEDF:
> > name = "sedf";
> > break;
> > --
> > 2.14.3
> >
> > --
> > libvir-list mailing list
> > libvir-list(a)redhat.com
> > https://www.redhat.com/mailman/listinfo/libvir-list
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
11:08 a.m.
New subject: [libvirt] [PATCH v2 07/14] lxc: handle missing switch enum cases
Ensure all enum cases are listed in switch statements, or cast away
enum type in places where we don't wish to cover all cases.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/lxc/lxc_container.c | 8 ++++----
src/lxc/lxc_controller.c | 8 +++++++-
src/lxc/lxc_driver.c | 38 ++++++++++++++++++++++++++++++++++----
3 files changed, 45 insertions(+), 9 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 96fceaf1b8..14928e8ece 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -2035,7 +2035,7 @@ static int lxcContainerDropCapabilities(virDomainDefPtr def,
break;
case VIR_DOMAIN_CAPABILITIES_POLICY_DEFAULT:
- switch ((virDomainCapsFeature) i) {
+ switch (i) {
case VIR_DOMAIN_CAPS_FEATURE_SYS_BOOT: /* No use of reboot */
toDrop = !keepReboot && (state != VIR_TRISTATE_SWITCH_ON);
break;
@@ -2066,10 +2066,10 @@ static int lxcContainerDropCapabilities(virDomainDefPtr def,
}
break;
+ case VIR_DOMAIN_CAPABILITIES_POLICY_LAST:
default:
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Unsupported capabilities policy: %s"),
- virDomainCapabilitiesPolicyTypeToString(policy));
+ virReportEnumRangeError(virDomainCapabilitiesPolicy, policy);
+ return -1;
}
}
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index c5e67df938..7346804c18 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -395,8 +395,14 @@ static int virLXCControllerGetNICIndexes(virLXCControllerPtr ctrl)
case VIR_DOMAIN_NET_TYPE_INTERNAL:
case VIR_DOMAIN_NET_TYPE_DIRECT:
case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unsupported net type %s"),
+ virDomainNetTypeToString(ctrl->def->nets[i]->type));
+ goto cleanup;
+ case VIR_DOMAIN_NET_TYPE_LAST:
default:
- break;
+ virReportEnumRangeError(virDomainNetType,
ctrl->def->nets[i]->type);
+ goto cleanup;
}
}
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 961baa344c..3d5b2254f2 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -3968,10 +3968,21 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn,
if (!(veth = virLXCProcessSetupInterfaceDirect(conn, vm->def, net)))
goto cleanup;
} break;
- default:
+ case VIR_DOMAIN_NET_TYPE_USER:
+ case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+ case VIR_DOMAIN_NET_TYPE_SERVER:
+ case VIR_DOMAIN_NET_TYPE_CLIENT:
+ case VIR_DOMAIN_NET_TYPE_MCAST:
+ case VIR_DOMAIN_NET_TYPE_INTERNAL:
+ case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+ case VIR_DOMAIN_NET_TYPE_UDP:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Network device type is not supported"));
goto cleanup;
+ case VIR_DOMAIN_NET_TYPE_LAST:
+ default:
+ virReportEnumRangeError(virDomainNetType, actualType);
+ goto cleanup;
}
/* Set bandwidth or warn if requested and not supported. */
actualBandwidth = virDomainNetGetActualBandwidth(net);
@@ -4011,6 +4022,15 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn,
ignore_value(virNetDevMacVLanDelete(veth));
break;
+ case VIR_DOMAIN_NET_TYPE_USER:
+ case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+ case VIR_DOMAIN_NET_TYPE_SERVER:
+ case VIR_DOMAIN_NET_TYPE_CLIENT:
+ case VIR_DOMAIN_NET_TYPE_MCAST:
+ case VIR_DOMAIN_NET_TYPE_INTERNAL:
+ case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+ case VIR_DOMAIN_NET_TYPE_UDP:
+ case VIR_DOMAIN_NET_TYPE_LAST:
default:
/* no-op */
break;
@@ -4446,13 +4466,23 @@ lxcDomainDetachDeviceNetLive(virDomainObjPtr vm,
* the host side. Further the container can change
* the mac address of NIC name, so we can't easily
* find out which guest NIC it maps to
+ */
case VIR_DOMAIN_NET_TYPE_DIRECT:
- */
-
- default:
+ case VIR_DOMAIN_NET_TYPE_USER:
+ case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+ case VIR_DOMAIN_NET_TYPE_SERVER:
+ case VIR_DOMAIN_NET_TYPE_CLIENT:
+ case VIR_DOMAIN_NET_TYPE_MCAST:
+ case VIR_DOMAIN_NET_TYPE_INTERNAL:
+ case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+ case VIR_DOMAIN_NET_TYPE_UDP:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Only bridged veth devices can be detached"));
goto cleanup;
+ case VIR_DOMAIN_NET_TYPE_LAST:
+ default:
+ virReportEnumRangeError(virDomainNetType, actualType);
+ goto cleanup;
}
virDomainAuditNet(vm, detach, NULL, "detach", true);
--
2.14.3
1:33 p.m.
New subject: [libvirt] [PATCH v2 07/14] lxc: handle missing switch enum cases
On 02/20/2018 12:08 PM, Daniel P. Berrangé wrote:
Ensure all enum cases are listed in switch statements, or cast away
enum type in places where we don't wish to cover all cases.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/lxc/lxc_container.c | 8 ++++----
src/lxc/lxc_controller.c | 8 +++++++-
src/lxc/lxc_driver.c | 38 ++++++++++++++++++++++++++++++++++----
3 files changed, 45 insertions(+), 9 deletions(-)
[...]
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index c5e67df938..7346804c18 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -395,8 +395,14 @@ static int virLXCControllerGetNICIndexes(virLXCControllerPtr ctrl)
case VIR_DOMAIN_NET_TYPE_INTERNAL:
case VIR_DOMAIN_NET_TYPE_DIRECT:
case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unsupported net type %s"),
+
virDomainNetTypeToString(ctrl->def->nets[i]->type));
+ goto cleanup;
So this will cause an error; whereas, previously there was no error for
main startup and the values essentially ignored (with nothing added to
nicindexes). Is this the "safe" thing to do? Especially for existing
configs which may now fail... Perhaps some kind of VIR_* message printed
instead?
This certainly falls into the category of probably should have been an
error all along, but "fear" (at least in my mind) causes trepidation to
just spring an error on someone now.
Feels like one of those guest disappearing things, but you understand
the LXC controller better than I - so I'll just note it and let you decide.
+ case VIR_DOMAIN_NET_TYPE_LAST:
default:
- break;
+ virReportEnumRangeError(virDomainNetType,
ctrl->def->nets[i]->type);
+ goto cleanup;
I think having an error here is certainly reasonable even though it
probably is a can't get there type thing.
}
}
[...]
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
3:22 a.m.
New subject: [libvirt] [PATCH v2 07/14] lxc: handle missing switch enum cases
On Tue, Feb 20, 2018 at 02:33:56PM -0500, John Ferlan wrote:
On 02/20/2018 12:08 PM, Daniel P. Berrangé wrote:
> Ensure all enum cases are listed in switch statements, or cast away
> enum type in places where we don't wish to cover all cases.
>
> Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
> ---
> src/lxc/lxc_container.c | 8 ++++----
> src/lxc/lxc_controller.c | 8 +++++++-
> src/lxc/lxc_driver.c | 38 ++++++++++++++++++++++++++++++++++----
> 3 files changed, 45 insertions(+), 9 deletions(-)
>
[...]
> diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
> index c5e67df938..7346804c18 100644
> --- a/src/lxc/lxc_controller.c
> +++ b/src/lxc/lxc_controller.c
> @@ -395,8 +395,14 @@ static int virLXCControllerGetNICIndexes(virLXCControllerPtr
ctrl)
> case VIR_DOMAIN_NET_TYPE_INTERNAL:
> case VIR_DOMAIN_NET_TYPE_DIRECT:
> case VIR_DOMAIN_NET_TYPE_HOSTDEV:
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Unsupported net type %s"),
> +
virDomainNetTypeToString(ctrl->def->nets[i]->type));
> + goto cleanup;
So this will cause an error; whereas, previously there was no error for
main startup and the values essentially ignored (with nothing added to
nicindexes). Is this the "safe" thing to do? Especially for existing
configs which may now fail... Perhaps some kind of VIR_* message printed
instead?
This certainly falls into the category of probably should have been an
error all along, but "fear" (at least in my mind) causes trepidation to
just spring an error on someone now.
Feels like one of those guest disappearing things, but you understand
the LXC controller better than I - so I'll just note it and let you decide.
LXC controller code only runs on guest startup, so reporting an error here
is good. The driver code should not have even launched the controller if it
saw these NIC types.
> + case VIR_DOMAIN_NET_TYPE_LAST:
> default:
> - break;
> + virReportEnumRangeError(virDomainNetType,
ctrl->def->nets[i]->type);
> + goto cleanup;
I think having an error here is certainly reasonable even though it
probably is a can't get there type thing.
> }
> }
>
[...]
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
11:08 a.m.
New subject: [libvirt] [PATCH v2 08/14] nwfilter: handle missing switch enum cases
Ensure all enum cases are listed in switch statements, or cast away
enum type in places where we don't wish to cover all cases.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/conf/nwfilter_conf.h | 4 ++--
src/nwfilter/nwfilter_ebiptables_driver.c | 17 +++++++++++------
src/nwfilter/nwfilter_learnipaddr.c | 6 +++++-
3 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/src/conf/nwfilter_conf.h b/src/conf/nwfilter_conf.h
index eeec10e67b..f960bf3d56 100644
--- a/src/conf/nwfilter_conf.h
+++ b/src/conf/nwfilter_conf.h
@@ -86,7 +86,7 @@ typedef enum {
(((data)->flags) & NWFILTER_ENTRY_ITEM_FLAG_IS_NEG)
/* datatypes appearing in rule attributes */
-enum attrDatatype {
+typedef enum attrDatatype {
DATATYPE_UINT16 = (1 << 0),
DATATYPE_UINT8 = (1 << 1),
DATATYPE_UINT16_HEX = (1 << 2),
@@ -106,7 +106,7 @@ enum attrDatatype {
DATATYPE_IPSETFLAGS = (1 << 16),
DATATYPE_LAST = (1 << 17),
-};
+} virNWFilterAttrDataType;
# define NWFILTER_MAC_BGA "01:80:c2:00:00:00"
diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c
b/src/nwfilter/nwfilter_ebiptables_driver.c
index b8682a1130..b19b07c845 100644
--- a/src/nwfilter/nwfilter_ebiptables_driver.c
+++ b/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -320,11 +320,16 @@ _printDataType(virNWFilterVarCombIterPtr vars,
VIR_FREE(flags);
break;
- default:
+ case DATATYPE_STRING:
+ case DATATYPE_STRINGCOPY:
+ case DATATYPE_BOOLEAN:
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Unhandled datatype %x"), item->datatype);
+ _("Cannot print data type %x"), item->datatype);
+ return -1;
+ case DATATYPE_LAST:
+ default:
+ virReportEnumRangeError(virNWFilterAttrDataType, item->datatype);
return -1;
- break;
}
return 0;
@@ -1183,7 +1188,7 @@ _iptablesCreateRuleInstance(virFirewallPtr fw,
PRINT_IPT_ROOT_CHAIN(chain, chainPrefix, ifname);
- switch (rule->prtclType) {
+ switch ((int)rule->prtclType) {
case VIR_NWFILTER_RULE_PROTOCOL_TCP:
case VIR_NWFILTER_RULE_PROTOCOL_TCPoIPV6:
fwrule = virFirewallAddRule(fw, layer,
@@ -1873,7 +1878,7 @@ ebtablesCreateRuleInstance(virFirewallPtr fw,
#define INST_ITEM_MASK(S, I, MASK, C) \
INST_ITEM_2PARMS(S, I, MASK, C, "/")
- switch (rule->prtclType) {
+ switch ((int)rule->prtclType) {
case VIR_NWFILTER_RULE_PROTOCOL_MAC:
fwrule = virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
"-t", "nat",
@@ -2677,7 +2682,7 @@ ebtablesCreateTmpSubChainFW(virFirewallPtr fw,
fwrule = virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
"-t", "nat", "-A",
rootchain, NULL);
- switch (protoidx) {
+ switch ((int)protoidx) {
case L2_PROTO_MAC_IDX:
break;
case L2_PROTO_STP_IDX:
diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c
index 5b95f0e613..9ca0639576 100644
--- a/src/nwfilter/nwfilter_learnipaddr.c
+++ b/src/nwfilter/nwfilter_learnipaddr.c
@@ -430,7 +430,7 @@ learnIPAddressThread(void *arg)
}
virBufferAddLit(&buf, "src port 67 and dst port 68");
break;
- default:
+ case DETECT_STATIC:
if (techdriver->applyBasicRules(req->ifname,
&req->macaddr) < 0) {
req->status = EINVAL;
@@ -438,6 +438,10 @@ learnIPAddressThread(void *arg)
}
virBufferAsprintf(&buf, "ether host %s or ether dst
ff:ff:ff:ff:ff:ff",
macaddr);
+ break;
+ default:
+ req->status = EINVAL;
+ goto done;
}
if (virBufferError(&buf)) {
--
2.14.3
1:52 p.m.
New subject: [libvirt] [PATCH v2 08/14] nwfilter: handle missing switch enum cases
On 02/20/2018 12:08 PM, Daniel P. Berrangé wrote:
Ensure all enum cases are listed in switch statements, or cast away
enum type in places where we don't wish to cover all cases.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/conf/nwfilter_conf.h | 4 ++--
src/nwfilter/nwfilter_ebiptables_driver.c | 17 +++++++++++------
src/nwfilter/nwfilter_learnipaddr.c | 6 +++++-
3 files changed, 18 insertions(+), 9 deletions(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
11:08 a.m.
New subject: [libvirt] [PATCH v2 09/14] qemu: handle missing switch enum cases
Ensure all enum cases are listed in switch statements, or cast away
enum type in places where we don't wish to cover all cases.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/qemu/qemu_command.c | 26 ++++++++++++++++++--------
src/qemu/qemu_domain.c | 21 +++++++++++++++++++++
src/qemu/qemu_driver.c | 25 +++++++++++++++++--------
src/qemu/qemu_hotplug.c | 33 ++++++++++++++++++++++++++++-----
src/qemu/qemu_migration.c | 11 ++++++++++-
src/qemu/qemu_process.c | 2 ++
6 files changed, 96 insertions(+), 22 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 34dda23288..fa0aa5d5c3 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2632,7 +2632,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
switch ((virDomainControllerModelSCSI) def->model) {
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
- switch ((virDomainDeviceAddressType) address_type) {
+ switch (address_type) {
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW:
virBufferAddLit(&buf, "virtio-scsi-ccw");
break;
@@ -2684,7 +2684,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
break;
case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL:
- switch ((virDomainDeviceAddressType) address_type) {
+ switch (address_type) {
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
virBufferAddLit(&buf, "virtio-serial-pci");
break;
@@ -3407,12 +3407,17 @@ qemuBuildNicDevStr(virDomainDefPtr def,
case VIR_DOMAIN_NET_VIRTIO_TX_MODE_TIMER:
virBufferAddLit(&buf, "timer");
break;
+
+ case VIR_DOMAIN_NET_VIRTIO_TX_MODE_DEFAULT:
+ break;
+
+ case VIR_DOMAIN_NET_VIRTIO_TX_MODE_LAST:
default:
/* this should never happen, if it does, we need
* to add another case to this switch.
*/
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("unrecognized virtio-net-pci 'tx'
option"));
+ virReportEnumRangeError(virDomainNetVirtioTxModeType,
+ net->driver.virtio.txmode);
goto error;
}
} else {
@@ -6598,7 +6603,7 @@ qemuBuildGlobalControllerCommandLine(virCommandPtr cmd,
bool cap = false;
bool machine = false;
- switch ((virDomainControllerModelPCI) cont->model) {
+ switch (cont->model) {
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
hoststr = "i440FX-pcihost";
cap = virQEMUCapsGet(qemuCaps, QEMU_CAPS_I440FX_PCI_HOLE64_SIZE);
@@ -6941,7 +6946,7 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
if (cpu_flags && !cpu) {
const char *default_model;
- switch (def->os.arch) {
+ switch ((int)def->os.arch) {
case VIR_ARCH_I686:
default_model = "qemu32";
break;
@@ -6987,7 +6992,7 @@ qemuBuildObsoleteAccelArg(virCommandPtr cmd,
bool disableKVM = false;
bool enableKVM = false;
- switch (def->virtType) {
+ switch ((int)def->virtType) {
case VIR_DOMAIN_VIRT_QEMU:
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))
disableKVM = true;
@@ -7955,8 +7960,13 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_CLIENT:
virBufferAddLit(&opt, "agent-mouse=on,");
break;
- default:
+ case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_DEFAULT:
break;
+ case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_LAST:
+ default:
+ virReportEnumRangeError(virDomainGraphicsSpiceMouseMode,
+ graphics->data.spice.mousemode);
+ goto error;
}
}
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index b1308e5a49..8b4efc82de 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2804,6 +2804,27 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
addPCIRoot = true;
break;
+ case VIR_ARCH_ARMV6L:
+ case VIR_ARCH_ARMV7B:
+ case VIR_ARCH_CRIS:
+ case VIR_ARCH_ITANIUM:
+ case VIR_ARCH_LM32:
+ case VIR_ARCH_M68K:
+ case VIR_ARCH_MICROBLAZE:
+ case VIR_ARCH_MICROBLAZEEL:
+ case VIR_ARCH_MIPS:
+ case VIR_ARCH_MIPSEL:
+ case VIR_ARCH_MIPS64:
+ case VIR_ARCH_MIPS64EL:
+ case VIR_ARCH_OR32:
+ case VIR_ARCH_PARISC:
+ case VIR_ARCH_PARISC64:
+ case VIR_ARCH_PPCLE:
+ case VIR_ARCH_UNICORE32:
+ case VIR_ARCH_XTENSA:
+ case VIR_ARCH_XTENSAEB:
+ case VIR_ARCH_NONE:
+ case VIR_ARCH_LAST:
default:
break;
}
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 416567512d..8aa293bf1b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3016,13 +3016,8 @@ qemuCompressGetCommand(virQEMUSaveFormat compression)
ret = virCommandNew(prog);
virCommandAddArg(ret, "-dc");
- switch (compression) {
- case QEMU_SAVE_FORMAT_LZOP:
+ if (compression == QEMU_SAVE_FORMAT_LZOP)
virCommandAddArg(ret, "--ignore-warn");
- break;
- default:
- break;
- }
return ret;
}
@@ -17792,11 +17787,18 @@ qemuDomainOpenGraphics(virDomainPtr dom,
case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
protocol = "spice";
break;
- default:
+ case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
+ case VIR_DOMAIN_GRAPHICS_TYPE_RDP:
+ case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Can only open VNC or SPICE graphics backends, not
%s"),
virDomainGraphicsTypeToString(vm->def->graphics[idx]->type));
goto endjob;
+ case VIR_DOMAIN_GRAPHICS_TYPE_LAST:
+ default:
+ virReportEnumRangeError(virDomainGraphicsType,
+ vm->def->graphics[idx]->type);
+ goto endjob;
}
if (qemuSecuritySetImageFDLabel(driver->securityManager, vm->def, fd) < 0)
@@ -17856,11 +17858,18 @@ qemuDomainOpenGraphicsFD(virDomainPtr dom,
case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
protocol = "spice";
break;
- default:
+ case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
+ case VIR_DOMAIN_GRAPHICS_TYPE_RDP:
+ case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Can only open VNC or SPICE graphics backends, not
%s"),
virDomainGraphicsTypeToString(vm->def->graphics[idx]->type));
goto cleanup;
+ case VIR_DOMAIN_GRAPHICS_TYPE_LAST:
+ default:
+ virReportEnumRangeError(virDomainGraphicsType,
+ vm->def->graphics[idx]->type);
+ goto cleanup;
}
if (qemuSecuritySetSocketLabel(driver->securityManager, vm->def) < 0)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 04f4f689a9..f28006e3cd 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2978,11 +2978,24 @@ qemuDomainChangeNetFilter(virDomainObjPtr vm,
case VIR_DOMAIN_NET_TYPE_BRIDGE:
case VIR_DOMAIN_NET_TYPE_NETWORK:
break;
- default:
+ case VIR_DOMAIN_NET_TYPE_USER:
+ case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+ case VIR_DOMAIN_NET_TYPE_SERVER:
+ case VIR_DOMAIN_NET_TYPE_CLIENT:
+ case VIR_DOMAIN_NET_TYPE_MCAST:
+ case VIR_DOMAIN_NET_TYPE_INTERNAL:
+ case VIR_DOMAIN_NET_TYPE_DIRECT:
+ case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+ case VIR_DOMAIN_NET_TYPE_UDP:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("filters not supported on interfaces of type %s"),
virDomainNetTypeToString(virDomainNetGetActualType(newdev)));
return -1;
+ case VIR_DOMAIN_NET_TYPE_LAST:
+ default:
+ virReportEnumRangeError(virDomainNetType,
+ virDomainNetGetActualType(newdev));
+ return -1;
}
virDomainConfNWFilterTeardown(olddev);
@@ -3277,12 +3290,16 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
/* all handled in common code directly below this switch */
break;
- default:
+ case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+ case VIR_DOMAIN_NET_TYPE_HOSTDEV:
virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
_("unable to change config on '%s' network
type"),
virDomainNetTypeToString(newdev->type));
- break;
-
+ goto cleanup;
+ case VIR_DOMAIN_NET_TYPE_LAST:
+ default:
+ virReportEnumRangeError(virDomainNetType, newdev->type);
+ goto cleanup;
}
} else {
/* interface type has changed. There are a few special cases
@@ -3661,10 +3678,16 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver,
}
break;
- default:
+ case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
+ case VIR_DOMAIN_GRAPHICS_TYPE_RDP:
+ case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unable to change config on '%s' graphics
type"), type);
break;
+ case VIR_DOMAIN_GRAPHICS_TYPE_LAST:
+ default:
+ virReportEnumRangeError(virDomainGraphicsType, dev->type);
+ break;
}
cleanup:
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 99d58325cb..2d92d38936 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1468,11 +1468,20 @@ qemuMigrationJobName(virDomainObjPtr vm)
switch (priv->job.asyncJob) {
case QEMU_ASYNC_JOB_MIGRATION_OUT:
- return _("migration job");
+ return _("migration out job");
case QEMU_ASYNC_JOB_SAVE:
return _("domain save job");
case QEMU_ASYNC_JOB_DUMP:
return _("domain core dump job");
+ case QEMU_ASYNC_JOB_NONE:
+ return _("no job");
+ case QEMU_ASYNC_JOB_MIGRATION_IN:
+ return _("migration in job");
+ case QEMU_ASYNC_JOB_SNAPSHOT:
+ return _("snapshot job");
+ case QEMU_ASYNC_JOB_START:
+ return _("start job");
+ case QEMU_ASYNC_JOB_LAST:
default:
return _("job");
}
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 13b05d236a..dc33eb7bcd 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -679,6 +679,8 @@ qemuProcessHandleShutdown(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
detail = VIR_DOMAIN_EVENT_SHUTDOWN_HOST;
break;
+ case VIR_TRISTATE_BOOL_ABSENT:
+ case VIR_TRISTATE_BOOL_LAST:
default:
detail = VIR_DOMAIN_EVENT_SHUTDOWN_FINISHED;
break;
--
2.14.3
2:28 p.m.
New subject: [libvirt] [PATCH v2 09/14] qemu: handle missing switch enum cases
On 02/20/2018 12:08 PM, Daniel P. Berrangé wrote:
Ensure all enum cases are listed in switch statements, or cast away
enum type in places where we don't wish to cover all cases.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/qemu/qemu_command.c | 26 ++++++++++++++++++--------
src/qemu/qemu_domain.c | 21 +++++++++++++++++++++
src/qemu/qemu_driver.c | 25 +++++++++++++++++--------
src/qemu/qemu_hotplug.c | 33 ++++++++++++++++++++++++++++-----
src/qemu/qemu_migration.c | 11 ++++++++++-
src/qemu/qemu_process.c | 2 ++
6 files changed, 96 insertions(+), 22 deletions(-)
[...]
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 99d58325cb..2d92d38936 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1468,11 +1468,20 @@ qemuMigrationJobName(virDomainObjPtr vm)
switch (priv->job.asyncJob) {
case QEMU_ASYNC_JOB_MIGRATION_OUT:
- return _("migration job");
+ return _("migration out job");
case QEMU_ASYNC_JOB_SAVE:
return _("domain save job");
case QEMU_ASYNC_JOB_DUMP:
return _("domain core dump job");
+ case QEMU_ASYNC_JOB_NONE:
+ return _("no job");
Note from previous review applies...
Can this be "undefined" instead of "no"? Looking at the caller you
could get output such as "operation failed: no job: %s" where %s could
be "is not active", "unexpectedly failed", "canceled by
client", or
"failed due to I/O error"
Could also just lump it in with LAST and default.
+ case QEMU_ASYNC_JOB_MIGRATION_IN:
+ return _("migration in job");
+ case QEMU_ASYNC_JOB_SNAPSHOT:
+ return _("snapshot job");
+ case QEMU_ASYNC_JOB_START:
+ return _("start job");
+ case QEMU_ASYNC_JOB_LAST:
default:
return _("job");
}
[...]
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
3:25 a.m.
New subject: [libvirt] [PATCH v2 09/14] qemu: handle missing switch enum cases
On Tue, Feb 20, 2018 at 03:28:56PM -0500, John Ferlan wrote:
On 02/20/2018 12:08 PM, Daniel P. Berrangé wrote:
> Ensure all enum cases are listed in switch statements, or cast away
> enum type in places where we don't wish to cover all cases.
>
> Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
> ---
> src/qemu/qemu_command.c | 26 ++++++++++++++++++--------
> src/qemu/qemu_domain.c | 21 +++++++++++++++++++++
> src/qemu/qemu_driver.c | 25 +++++++++++++++++--------
> src/qemu/qemu_hotplug.c | 33 ++++++++++++++++++++++++++++-----
> src/qemu/qemu_migration.c | 11 ++++++++++-
> src/qemu/qemu_process.c | 2 ++
> 6 files changed, 96 insertions(+), 22 deletions(-)
>
[...]
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 99d58325cb..2d92d38936 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -1468,11 +1468,20 @@ qemuMigrationJobName(virDomainObjPtr vm)
>
> switch (priv->job.asyncJob) {
> case QEMU_ASYNC_JOB_MIGRATION_OUT:
> - return _("migration job");
> + return _("migration out job");
> case QEMU_ASYNC_JOB_SAVE:
> return _("domain save job");
> case QEMU_ASYNC_JOB_DUMP:
> return _("domain core dump job");
> + case QEMU_ASYNC_JOB_NONE:
> + return _("no job");
Note from previous review applies...
Can this be "undefined" instead of "no"? Looking at the caller you
could get output such as "operation failed: no job: %s" where %s could
be "is not active", "unexpectedly failed", "canceled by
client", or
"failed due to I/O error"
Could also just lump it in with LAST and default.
Ok, yes.
> + case QEMU_ASYNC_JOB_MIGRATION_IN:
> + return _("migration in job");
> + case QEMU_ASYNC_JOB_SNAPSHOT:
> + return _("snapshot job");
> + case QEMU_ASYNC_JOB_START:
> + return _("start job");
> + case QEMU_ASYNC_JOB_LAST:
> default:
> return _("job");
> }
[...]
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
11:08 a.m.
New subject: [libvirt] [PATCH v2 10/14] rpc: handle missing switch enum cases
Ensure all enum cases are listed in switch statements.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/rpc/virnetclient.c | 2 ++
src/rpc/virnetclientprogram.c | 1 +
src/rpc/virnetserverprogram.c | 4 ++++
3 files changed, 7 insertions(+)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 8aeacf8774..0c8d58c32c 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -1287,6 +1287,8 @@ virNetClientCallDispatch(virNetClientPtr client)
case VIR_NET_STREAM_HOLE: /* Sparse stream protocol*/
return virNetClientCallDispatchStream(client);
+ case VIR_NET_CALL:
+ case VIR_NET_CALL_WITH_FDS:
default:
virReportError(VIR_ERR_RPC,
_("got unexpected RPC call prog %d vers %d proc %d type
%d"),
diff --git a/src/rpc/virnetclientprogram.c b/src/rpc/virnetclientprogram.c
index d81a055424..505b40fc4b 100644
--- a/src/rpc/virnetclientprogram.c
+++ b/src/rpc/virnetclientprogram.c
@@ -384,6 +384,7 @@ int virNetClientProgramCall(virNetClientProgramPtr prog,
virNetClientProgramDispatchError(prog, msg);
goto error;
+ case VIR_NET_CONTINUE:
default:
virReportError(VIR_ERR_RPC,
_("Unexpected message status %d"),
msg->header.status);
diff --git a/src/rpc/virnetserverprogram.c b/src/rpc/virnetserverprogram.c
index 557651ffbd..75b0052cdb 100644
--- a/src/rpc/virnetserverprogram.c
+++ b/src/rpc/virnetserverprogram.c
@@ -324,6 +324,10 @@ int virNetServerProgramDispatch(virNetServerProgramPtr prog,
ret = 0;
break;
+ case VIR_NET_REPLY:
+ case VIR_NET_REPLY_WITH_FDS:
+ case VIR_NET_MESSAGE:
+ case VIR_NET_STREAM_HOLE:
default:
virReportError(VIR_ERR_RPC,
_("Unexpected message type %u"),
--
2.14.3
2:30 p.m.
New subject: [libvirt] [PATCH v2 10/14] rpc: handle missing switch enum cases
On 02/20/2018 12:08 PM, Daniel P. Berrangé wrote:
Ensure all enum cases are listed in switch statements.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/rpc/virnetclient.c | 2 ++
src/rpc/virnetclientprogram.c | 1 +
src/rpc/virnetserverprogram.c | 4 ++++
3 files changed, 7 insertions(+)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
11:08 a.m.
New subject: [libvirt] [PATCH v2 11/14] security: handle missing switch enum cases
Ensure all enum cases are listed in switch statements.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/security/security_driver.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/security/security_driver.c b/src/security/security_driver.c
index 4800d5255a..a845dc7995 100644
--- a/src/security/security_driver.c
+++ b/src/security/security_driver.c
@@ -82,6 +82,7 @@ virSecurityDriverPtr virSecurityDriverLookup(const char *name,
}
break;
+ case SECURITY_DRIVER_ERROR:
default:
return NULL;
}
--
2.14.3
2:31 p.m.
New subject: [libvirt] [PATCH v2 11/14] security: handle missing switch enum cases
On 02/20/2018 12:08 PM, Daniel P. Berrangé wrote:
Ensure all enum cases are listed in switch statements.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/security/security_driver.c | 1 +
1 file changed, 1 insertion(+)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
11:08 a.m.
New subject: [libvirt] [PATCH v2 12/14] xen: handle missing switch enum cases
Ensure all enum cases are listed in switch statements.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/vmx/vmx.c | 26 ++++++++++++++++++++++++--
src/xenconfig/xen_common.c | 18 +++++++++++++++---
src/xenconfig/xen_xl.c | 7 ++++++-
3 files changed, 45 insertions(+), 6 deletions(-)
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 1d13ab03db..3b0c16d5a6 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -3283,11 +3283,19 @@ virVMXFormatConfig(virVMXContext *ctx, virDomainXMLOptionPtr
xmlopt, virDomainDe
break;
- default:
+ case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
+ case VIR_DOMAIN_GRAPHICS_TYPE_RDP:
+ case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP:
+ case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Unsupported graphics type '%s'"),
virDomainGraphicsTypeToString(def->graphics[i]->type));
goto cleanup;
+
+ case VIR_DOMAIN_GRAPHICS_TYPE_LAST:
+ default:
+ virReportEnumRangeError(virDomainGraphicsType,
def->graphics[i]->type);
+ goto cleanup;
}
}
@@ -3782,10 +3790,24 @@ virVMXFormatEthernet(virDomainNetDefPtr def, int controller,
controller);
break;
- default:
+ case VIR_DOMAIN_NET_TYPE_ETHERNET:
+ case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+ case VIR_DOMAIN_NET_TYPE_SERVER:
+ case VIR_DOMAIN_NET_TYPE_CLIENT:
+ case VIR_DOMAIN_NET_TYPE_MCAST:
+ case VIR_DOMAIN_NET_TYPE_NETWORK:
+ case VIR_DOMAIN_NET_TYPE_INTERNAL:
+ case VIR_DOMAIN_NET_TYPE_DIRECT:
+ case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+ case VIR_DOMAIN_NET_TYPE_UDP:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unsupported net type
'%s'"),
virDomainNetTypeToString(def->type));
return -1;
+
+ case VIR_DOMAIN_NET_TYPE_LAST:
+ default:
+ virReportEnumRangeError(virDomainNetType, def->type);
+ return -1;
}
/* def:mac -> vmx:addressType, vmx:(generated)Address, vmx:checkMACAddress */
diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
index ca3b4dee6a..bc43185363 100644
--- a/src/xenconfig/xen_common.c
+++ b/src/xenconfig/xen_common.c
@@ -1258,10 +1258,22 @@ xenFormatNet(virConnectPtr conn,
}
break;
+ case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+ case VIR_DOMAIN_NET_TYPE_SERVER:
+ case VIR_DOMAIN_NET_TYPE_CLIENT:
+ case VIR_DOMAIN_NET_TYPE_MCAST:
+ case VIR_DOMAIN_NET_TYPE_INTERNAL:
+ case VIR_DOMAIN_NET_TYPE_DIRECT:
+ case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+ case VIR_DOMAIN_NET_TYPE_UDP:
+ case VIR_DOMAIN_NET_TYPE_USER:
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unsupported net type
'%s'"),
+ virDomainNetTypeToString(net->type));
+ goto cleanup;
+
+ case VIR_DOMAIN_NET_TYPE_LAST:
default:
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unsupported network type %d"),
- net->type);
+ virReportEnumRangeError(virDomainNetType, net->type);
goto cleanup;
}
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 2ef80eb83a..e1ec8e765f 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -1641,8 +1641,13 @@ xenFormatXLSpice(virConfPtr conf, virDomainDefPtr def)
if (xenConfigSetInt(conf, "spicevdagent", 1) < 0)
return -1;
break;
- default:
+ case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_DEFAULT:
break;
+ case VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_LAST:
+ default:
+ virReportEnumRangeError(virDomainGraphicsSpiceMouseMode,
+ graphics->data.spice.mousemode);
+ return -1;
}
}
--
2.14.3
2:34 p.m.
New subject: [libvirt] [PATCH v2 12/14] xen: handle missing switch enum cases
On 02/20/2018 12:08 PM, Daniel P. Berrangé wrote:
Ensure all enum cases are listed in switch statements.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/vmx/vmx.c | 26 ++++++++++++++++++++++++--
src/xenconfig/xen_common.c | 18 +++++++++++++++---
src/xenconfig/xen_xl.c | 7 ++++++-
3 files changed, 45 insertions(+), 6 deletions(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
11:08 a.m.
New subject: [libvirt] [PATCH v2 13/14] tools: handle missing switch enum cases
Cast away enum type in places where we don't wish to cover all cases.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
tools/virt-host-validate-qemu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/virt-host-validate-qemu.c b/tools/virt-host-validate-qemu.c
index 2aa396e3ab..d7573ea8b3 100644
--- a/tools/virt-host-validate-qemu.c
+++ b/tools/virt-host-validate-qemu.c
@@ -33,13 +33,14 @@ int virHostValidateQEMU(void)
int ret = 0;
bool hasHwVirt = false;
bool hasVirtFlag = false;
+ virArch arch = virArchFromHost();
const char *kvmhint = _("Check that CPU and firmware supports virtualization
"
"and kvm module is loaded");
if (!(flags = virHostValidateGetCPUFlags()))
return -1;
- switch (virArchFromHost()) {
+ switch ((int)arch) {
case VIR_ARCH_I686:
case VIR_ARCH_X86_64:
hasVirtFlag = true;
--
2.14.3
2:35 p.m.
New subject: [libvirt] [PATCH v2 13/14] tools: handle missing switch enum cases
On 02/20/2018 12:08 PM, Daniel P. Berrangé wrote:
Cast away enum type in places where we don't wish to cover all
cases.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
tools/virt-host-validate-qemu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
11:08 a.m.
New subject: [libvirt] [PATCH v2 14/14] m4: enforce that all enum cases are listed in switch statements
As a general rule any time we switch() on something that is an enum, we
want to have a case for every enum constant. The -Wswitch warning will
report any switch where we've violated this rule, except if that switch
has a default case.
Unfortunately it is reasonable to want to list all enum constants *and*
also have a default case. To get a warning in that scenario requires
that we turn on -Wswitch-enum.
In a few cases where we explicitly don't want to list all enum cases, we
can discard the enum type checking by casting the value to a plain int.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
m4/virt-compile-warnings.m4 | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4
index 4b35a6f6b5..fc185aef38 100644
--- a/m4/virt-compile-warnings.m4
+++ b/m4/virt-compile-warnings.m4
@@ -47,8 +47,6 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[
dontwarn="$dontwarn -Wlong-long"
# We allow manual list of all enum cases without default:
dontwarn="$dontwarn -Wswitch-default"
- # We allow optional default: instead of listing all enum values
- dontwarn="$dontwarn -Wswitch-enum"
# Not a problem since we don't use -fstrict-overflow
dontwarn="$dontwarn -Wstrict-overflow"
# Not a problem since we don't use -funsafe-loop-optimizations
@@ -184,6 +182,11 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[
# that one off, so we need to manually enable this again
wantwarn="$wantwarn -Wjump-misses-init"
+ # GNULIB explicitly filters it out, preferring -Wswitch
+ # but that doesn't report missing enums if a default:
+ # is present.
+ wantwarn="$wantwarn -Wswitch-enum"
+
# GNULIB turns on -Wformat=2 which implies -Wformat-nonliteral,
# so we need to manually re-exclude it. Also, older gcc 4.2
# added an implied ATTRIBUTE_NONNULL on any parameter marked
--
2.14.3
2:37 p.m.
New subject: [libvirt] [PATCH v2 14/14] m4: enforce that all enum cases are listed in switch statements
On 02/20/2018 12:08 PM, Daniel P. Berrangé wrote:
As a general rule any time we switch() on something that is an enum,
we
want to have a case for every enum constant. The -Wswitch warning will
report any switch where we've violated this rule, except if that switch
has a default case.
Unfortunately it is reasonable to want to list all enum constants *and*
also have a default case. To get a warning in that scenario requires
that we turn on -Wswitch-enum.
In a few cases where we explicitly don't want to list all enum cases, we
can discard the enum type checking by casting the value to a plain int.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
m4/virt-compile-warnings.m4 | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
2469
days inactive
2470
days old
35 comments
3 participants
participants (3)
-
Daniel P. Berrangé -
John Ferlan -
Martin Kletzander