Re: [libvirt] [PATCH 1/9] qemuDomainAttachNetDevice: Explicitly list allowed types for hotplug
On 08/16/2016 11:41 AM, Michal Privoznik wrote:
Instead of blindly claim support for hot-plugging of every
claiming
interface type out there we should copy approach we have for
device types: white listing supported types and explicitly error
out on unsupported ones.
For instance, trying to hotplug vhostuser interface results in
nothing usable from guest currently. vhostuser typed interfaces
require additional work on our side.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
Part of me thinks - sure ACK this, but the other part says why not
convert the "if - elseif - elseif" a few lines below to be essentially
this switch?
Then I wonder if Coverity would pick up on the HOSTDEV specific if and
"flag" the switch case HOSTDEV: and complain?
What would be the drawback to the switch option?
John
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 00e4a75..d1acdd9 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -947,6 +947,30 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
actualType = virDomainNetGetActualType(net);
+
+ switch ((virDomainNetType) actualType) {
+ case VIR_DOMAIN_NET_TYPE_NETWORK:
+ case VIR_DOMAIN_NET_TYPE_ETHERNET:
+ case VIR_DOMAIN_NET_TYPE_DIRECT:
+ case VIR_DOMAIN_NET_TYPE_BRIDGE:
+ case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+ /* These types are supported. */
+ 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_UDP:
+ case VIR_DOMAIN_NET_TYPE_LAST:
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+ _("hotplug of interface type of %s is not implemented
yet"),
+ virDomainNetTypeToString(actualType));
+ goto cleanup;
+ }
+
if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
/* This is really a "smart hostdev", so it should be attached
* as a hostdev (the hostdev code will reach over into the