On 05/03/2013 02:07 PM, Osier Yang wrote:
Just like previous patches, this changes qemuCheckSharedDisk
into qemuCheckSharedDevice, which takes a virDomainDeviceDefPtr
argument instead.
---
src/qemu/qemu_conf.c | 86 +++++++++++++++++++++++++++++++++++++---------------
1 file changed, 61 insertions(+), 25 deletions(-)
Ahhh finally - never thought I'd get to the last one :-) Taken longer
than I wanted!
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index cf1c7ee..f8264f6 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -943,29 +943,55 @@ qemuGetSharedDeviceKey(const char *device_path)
return key;
}
-/* Check if a shared disk's setting conflicts with the conf
+/* Check if a shared device's setting conflicts with the conf
* used by other domain(s). Currently only checks the sgio
* setting. Note that this should only be called for disk with
- * block source.
+ * block source if the device type is disk.
*
* Returns 0 if no conflicts, otherwise returns -1.
*/
static int
-qemuCheckSharedDisk(virHashTablePtr sharedDevices,
- virDomainDiskDefPtr disk)
+qemuCheckSharedDevice(virHashTablePtr sharedDevices,
+ virDomainDeviceDefPtr dev)
{
+ virDomainDiskDefPtr disk = NULL;
+ virDomainHostdevDefPtr hostdev = NULL;
char *sysfs_path = NULL;
char *key = NULL;
+ char *hostdev_name = NULL;
+ char *hostdev_path = NULL;
+ char *device_path = NULL;
int val;
int ret = 0;
- /* The only conflicts between shared disk we care about now
- * is sgio setting, which is only valid for device='lun'.
- */
- if (disk->device != VIR_DOMAIN_DISK_DEVICE_LUN)
- return 0;
Coverity note #1:
(2) Event cond_false: Condition "dev->type == VIR_DOMAIN_DEVICE_DISK",
taking false branch
+ if (dev->type == VIR_DOMAIN_DEVICE_DISK) {
+ disk = dev->data.disk;
+
+ /* The only conflicts between shared disk we care about now
+ * is sgio setting, which is only valid for device='lun'.
+ */
+ if (disk->device != VIR_DOMAIN_DISK_DEVICE_LUN)
+ return 0;
+
+ device_path = disk->src;
Coverity note #2:
(3) Event cond_false: Condition "dev->type ==
VIR_DOMAIN_DEVICE_HOSTDEV", taking false branch
+ } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) {
+ hostdev = dev->data.hostdev;
+
+ if (!(hostdev_name = virSCSIDeviceGetDevName(hostdev->source.subsys.u.scsi.adapter,
+ hostdev->source.subsys.u.scsi.bus,
+ hostdev->source.subsys.u.scsi.target,
+ hostdev->source.subsys.u.scsi.unit)))
+ goto cleanup;
+
+ if (virAsprintf(&hostdev_path, "/dev/%s", hostdev_name) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ device_path = hostdev_path;
+ }
Coverity Note #3
In the "else" condition (not here) - that means "device_path = NULL"
which is going to be a problem shortly....
Should we return 0 as an "else" condition?
- if (!(sysfs_path = virGetUnprivSGIOSysfsPath(disk->src, NULL))) {
+ if (!(sysfs_path = virGetUnprivSGIOSysfsPath(device_path, NULL))) {
ret = -1;
goto cleanup;
}
@@ -976,7 +1002,7 @@ qemuCheckSharedDisk(virHashTablePtr sharedDevices,
if (!virFileExists(sysfs_path))
goto cleanup;
Coverity complains:
(8) Event var_deref_model: Passing null pointer "device_path" to
function "qemuGetSharedDeviceKey(char const *)", which dereferences it.
(The dereference is assumed on the basis of the 'nonnull' parameter
attribute.)
Also see events: [assign_zero]
Fix that and it's an ACK