[libvirt] [PATCH 0/3] Fix improper union member access on hostdevs and refactoring
This patch series fixes improper union member accesses on hostdevs. Additionally, it adds a util function which checks whether a hostdev is a SCSI device or not. This function is used to simplify and clarify the code. Marc Hartmayer (3): qemu: Fix improper union member access on hostdevs util: Add virHostdevIsSCSIDevice() Refactoring: Use virHostdevIsSCSIDevice() src/Makefile.am | 3 ++- src/conf/domain_conf.c | 8 +++----- src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 3 +-- src/qemu/qemu_conf.c | 7 +++---- src/qemu/qemu_domain.c | 14 +++++--------- src/qemu/qemu_process.c | 3 +++ src/util/virhostdev.c | 23 +++++++++++++++++------ src/util/virhostdev.h | 3 +++ 9 files changed, 38 insertions(+), 27 deletions(-) -- 2.5.5
Add missing checks if a hostdev is a subsystem/SCSI device before access the union member 'subsys'/'scsi'. Also fix indentation and simplify qemuDomainObjCheckHostdevTaint(). Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 10 +++++----- src/qemu/qemu_process.c | 4 ++++ 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0f113a7..e3c1805 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4037,12 +4037,12 @@ void qemuDomainObjCheckHostdevTaint(virQEMUDriverPtr driver, virDomainHostdevDefPtr hostdev, qemuDomainLogContextPtr logCtxt) { - virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; + if (!(hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)) + return; - if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && - scsisrc->rawio == VIR_TRISTATE_BOOL_YES) - qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_HIGH_PRIVILEGES, - logCtxt); + if (hostdev->source.subsys.u.scsi.rawio == VIR_TRISTATE_BOOL_YES) + qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_HIGH_PRIVILEGES, logCtxt); } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 3552a31..cca6754 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4312,6 +4312,10 @@ qemuProcessSetupRawIO(virQEMUDriverPtr driver, /* If rawio not already set, check hostdevs as well */ if (!rawio) { for (i = 0; i < vm->def->nhostdevs; i++) { + if (!(vm->def->hostdevs[i]->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + vm->def->hostdevs[i]->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)) + continue; + virDomainHostdevSubsysSCSIPtr scsisrc = &vm->def->hostdevs[i]->source.subsys.u.scsi; if (scsisrc->rawio == VIR_TRISTATE_BOOL_YES) { -- 2.5.5
Add the function virHostdevIsSCSIDevice() which detects whether a hostdev is a SCSI device or not. Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/libvirt_private.syms | 1 + src/util/virhostdev.c | 14 ++++++++++++++ src/util/virhostdev.h | 3 +++ 3 files changed, 18 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index baff82b..b7d26fd 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1677,6 +1677,7 @@ virHookPresent; # util/virhostdev.h virHostdevFindUSBDevice; +virHostdevIsSCSIDevice; virHostdevManagerGetDefault; virHostdevPCINodeDeviceDetach; virHostdevPCINodeDeviceReAttach; diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 9c2262e..d283306 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -337,6 +337,20 @@ virHostdevIsPCINetDevice(virDomainHostdevDefPtr hostdev) } +/** + * virHostdevIsSCSIDevice: + * @hostdev: host device to check + * + * Returns true if @hostdev is a SCSI device, false otherwise. + */ +bool +virHostdevIsSCSIDevice(virDomainHostdevDefPtr hostdev) +{ + return hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI; +} + + static int virHostdevNetConfigVirtPortProfile(const char *linkdev, int vf, virNetDevVPortProfilePtr virtPort, diff --git a/src/util/virhostdev.h b/src/util/virhostdev.h index f2f51bd..aa056eb 100644 --- a/src/util/virhostdev.h +++ b/src/util/virhostdev.h @@ -149,6 +149,9 @@ virHostdevReAttachDomainDevices(virHostdevManagerPtr mgr, unsigned int flags, const char *oldStateDir) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); +bool +virHostdevIsSCSIDevice(virDomainHostdevDefPtr hostdev) + ATTRIBUTE_NONNULL(1); /* functions used by NodeDevDetach/Reattach/Reset */ int virHostdevPCINodeDeviceDetach(virHostdevManagerPtr mgr, -- 2.5.5
Use the util function virHostdevIsSCSIDevice() to simplify if statements. Signed-off-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/Makefile.am | 3 ++- src/conf/domain_conf.c | 8 +++----- src/qemu/qemu_command.c | 3 +-- src/qemu/qemu_conf.c | 7 +++---- src/qemu/qemu_domain.c | 8 ++------ src/qemu/qemu_process.c | 3 +-- src/util/virhostdev.c | 9 +++------ 7 files changed, 15 insertions(+), 26 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index aaba9e6..b358e0e 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1137,7 +1137,8 @@ libvirt_util_la_LIBADD = $(CAPNG_LIBS) $(YAJL_LIBS) $(LIBNL_LIBS) \ noinst_LTLIBRARIES += libvirt_conf.la libvirt_la_BUILT_LIBADD += libvirt_conf.la libvirt_conf_la_SOURCES = $(CONF_SOURCES) -libvirt_conf_la_CFLAGS = $(AM_CFLAGS) +libvirt_conf_la_CFLAGS = \ + -I$(srcdir)/conf $(AM_CFLAGS) libvirt_conf_la_LDFLAGS = $(AM_LDFLAGS) libvirt_conf_la_LIBADD = $(LIBXML_LIBS) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6e008e2..e7517d9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -55,6 +55,7 @@ #include "virtpm.h" #include "virstring.h" #include "virnetdev.h" +#include "virhostdev.h" #define VIR_FROM_THIS VIR_FROM_DOMAIN @@ -4247,9 +4248,7 @@ virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev, if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { virDomainHostdevDefPtr hdev = dev->data.hostdev; - if (hdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - hdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) { - + if (virHostdevIsSCSIDevice(hdev)) { if (hdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && virDomainHostdevAssignAddress(xmlopt, def, hdev) < 0) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -15648,8 +15647,7 @@ virDomainDefMaybeAddHostdevSCSIcontroller(virDomainDefPtr def) for (i = 0; i < def->nhostdevs; i++) { hostdev = def->hostdevs[i]; - if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && + if (virHostdevIsSCSIDevice(hostdev) && (int)hostdev->info->addr.drive.controller > maxController) { maxController = hostdev->info->addr.drive.controller; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4a5fce3..51b5aaf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5199,8 +5199,7 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd, } /* SCSI */ - if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) { + if (virHostdevIsSCSIDevice(hostdev)) { if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SCSI_GENERIC)) { char *drvstr; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 0ed88f5..fbac3ea 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1187,10 +1187,9 @@ static bool qemuIsSharedHostdev(virDomainHostdevDefPtr hostdev) { return (hostdev->shareable && - (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && - hostdev->source.subsys.u.scsi.protocol != - VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI)); + (virHostdevIsSCSIDevice(hostdev) && + hostdev->source.subsys.u.scsi.protocol != + VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI)); } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index e3c1805..55215ec 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1185,12 +1185,9 @@ qemuDomainSecretHostdevPrepare(virConnectPtr conn, qemuDomainObjPrivatePtr priv, virDomainHostdevDefPtr hostdev) { - virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys; qemuDomainSecretInfoPtr secinfo = NULL; - if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) { - + if (virHostdevIsSCSIDevice(hostdev)) { virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi; @@ -4037,8 +4034,7 @@ void qemuDomainObjCheckHostdevTaint(virQEMUDriverPtr driver, virDomainHostdevDefPtr hostdev, qemuDomainLogContextPtr logCtxt) { - if (!(hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)) + if (!virHostdevIsSCSIDevice(hostdev)) return; if (hostdev->source.subsys.u.scsi.rawio == VIR_TRISTATE_BOOL_YES) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index cca6754..4758c49 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4312,8 +4312,7 @@ qemuProcessSetupRawIO(virQEMUDriverPtr driver, /* If rawio not already set, check hostdevs as well */ if (!rawio) { for (i = 0; i < vm->def->nhostdevs; i++) { - if (!(vm->def->hostdevs[i]->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - vm->def->hostdevs[i]->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)) + if (!virHostdevIsSCSIDevice(vm->def->hostdevs[i])) continue; virDomainHostdevSubsysSCSIPtr scsisrc = diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index d283306..a038e13 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -1114,8 +1114,7 @@ virHostdevUpdateActiveSCSIDevices(virHostdevManagerPtr mgr, hostdev = hostdevs[i]; scsisrc = &hostdev->source.subsys.u.scsi; - if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || - hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) + if (!virHostdevIsSCSIDevice(hostdev)) continue; if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { @@ -1413,8 +1412,7 @@ virHostdevPrepareSCSIDevices(virHostdevManagerPtr mgr, virDomainHostdevDefPtr hostdev = hostdevs[i]; virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; - if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || - hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) + if (!virHostdevIsSCSIDevice(hostdev)) continue; if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { @@ -1605,8 +1603,7 @@ virHostdevReAttachSCSIDevices(virHostdevManagerPtr mgr, virDomainHostdevDefPtr hostdev = hostdevs[i]; virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; - if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || - hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) + if (!virHostdevIsSCSIDevice(hostdev)) continue; if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) -- 2.5.5
On 15.11.2016 19:25, Marc Hartmayer wrote:
This patch series fixes improper union member accesses on hostdevs. Additionally, it adds a util function which checks whether a hostdev is a SCSI device or not. This function is used to simplify and clarify the code.
Marc Hartmayer (3): qemu: Fix improper union member access on hostdevs util: Add virHostdevIsSCSIDevice() Refactoring: Use virHostdevIsSCSIDevice()
src/Makefile.am | 3 ++- src/conf/domain_conf.c | 8 +++----- src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 3 +-- src/qemu/qemu_conf.c | 7 +++---- src/qemu/qemu_domain.c | 14 +++++--------- src/qemu/qemu_process.c | 3 +++ src/util/virhostdev.c | 23 +++++++++++++++++------ src/util/virhostdev.h | 3 +++ 9 files changed, 38 insertions(+), 27 deletions(-)
ACK series. Michal
participants (2)
-
Marc Hartmayer -
Michal Privoznik