3:38 p.m.
On 11/08/2016 01:26 PM, Eric Farman wrote:
As was suggested in an earlier review comment[1], we can
catch some additional code points by cleaning up how we use the
hostdev subsystem type in some switch statements.
[1] End of https://www.redhat.com/archives/libvir-list/2016-September/msg00399.html
Signed-off-by: Eric Farman <farman(a)linux.vnet.ibm.com>
---
src/conf/domain_conf.c | 11 +++++++++--
src/qemu/qemu_cgroup.c | 11 +++++++----
src/security/security_apparmor.c | 4 ++--
src/security/security_selinux.c | 8 ++++----
4 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a233c0c..043f0e2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12992,7 +12992,7 @@ virDomainHostdevDefParseXML(virDomainXMLOptionPtr xmlopt,
}
if (def->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
- switch (def->source.subsys.type) {
+ switch ((virDomainHostdevSubsysType) def->source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
if (def->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
def->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
@@ -13014,6 +13014,11 @@ virDomainHostdevDefParseXML(virDomainXMLOptionPtr xmlopt,
if (virXPathBoolean("boolean(./shareable)", ctxt))
def->shareable = true;
break;
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
+ break;
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
+ goto error;
Since virDomainHostdevDefParseXMLSubsys will validate that 'type' is
valid w/ a call to virDomainHostdevSubsysTypeFromString, that means for
this code there's no way the subsys.type could be invalid. So no need to
jump to error. FWIW: Even if you did jump to error, there should be an
error message.
In fact both USB and LAST "fall through" for now.
Later though when SCSI_HOST is added, that's when you can do some more
address validation since it'll be a new type. As opposed to the lack of
validation for USB since USB existed before we added having <address>
validation (which is in the post parse callback IIRC).
I will adjust this before pushing (shortly)...
ACK -
John
+ break;
}
}
@@ -13880,7 +13885,7 @@ virDomainHostdevMatchSubsys(virDomainHostdevDefPtr a,
if (a->source.subsys.type != b->source.subsys.type)
return 0;
- switch (a->source.subsys.type) {
+ switch ((virDomainHostdevSubsysType) a->source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
return virDomainHostdevMatchSubsysPCI(a, b);
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
@@ -13894,6 +13899,8 @@ virDomainHostdevMatchSubsys(virDomainHostdevDefPtr a,
return virDomainHostdevMatchSubsysSCSIiSCSI(a, b);
else
return virDomainHostdevMatchSubsysSCSIHost(a, b);
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
+ return 0;
}
return 0;
}
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 4489c64..1443f7e 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -301,7 +301,7 @@ qemuSetupHostdevCgroup(virDomainObjPtr vm,
if (dev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
- switch (dev->source.subsys.type) {
+ switch ((virDomainHostdevSubsysType) dev->source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) {
int rv;
@@ -376,7 +376,7 @@ qemuSetupHostdevCgroup(virDomainObjPtr vm,
break;
}
- default:
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
break;
}
}
@@ -410,7 +410,7 @@ qemuTeardownHostdevCgroup(virDomainObjPtr vm,
if (dev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
- switch (dev->source.subsys.type) {
+ switch ((virDomainHostdevSubsysType) dev->source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) {
int rv;
@@ -437,7 +437,10 @@ qemuTeardownHostdevCgroup(virDomainObjPtr vm,
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
/* nothing to tear down for USB */
break;
- default:
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
+ /* nothing to tear down for SCSI */
+ break;
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
break;
}
}
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index af2b639..beddf6d 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -856,7 +856,7 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr,
ptr->mgr = mgr;
ptr->def = def;
- switch (dev->source.subsys.type) {
+ switch ((virDomainHostdevSubsysType) dev->source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
virUSBDevicePtr usb =
virUSBDeviceNew(usbsrc->bus, usbsrc->device, vroot);
@@ -909,7 +909,7 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr,
break;
}
- default:
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
ret = 0;
break;
}
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 5dad22c..89c93dc 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1436,7 +1436,7 @@ virSecuritySELinuxSetHostdevSubsysLabel(virSecurityManagerPtr mgr,
scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI)
return 0;
- switch (dev->source.subsys.type) {
+ switch ((virDomainHostdevSubsysType) dev->source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
virUSBDevicePtr usb;
@@ -1498,7 +1498,7 @@ virSecuritySELinuxSetHostdevSubsysLabel(virSecurityManagerPtr mgr,
break;
}
- default:
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
ret = 0;
break;
}
@@ -1640,7 +1640,7 @@ virSecuritySELinuxRestoreHostdevSubsysLabel(virSecurityManagerPtr
mgr,
scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI)
return 0;
- switch (dev->source.subsys.type) {
+ switch ((virDomainHostdevSubsysType) dev->source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
virUSBDevicePtr usb;
@@ -1700,7 +1700,7 @@ virSecuritySELinuxRestoreHostdevSubsysLabel(virSecurityManagerPtr
mgr,
break;
}
- default:
+ case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
ret = 0;
break;
}