[libvirt] [PATCH 25/25] qemu: Check conflicts for shared scsi host device
Osier Yang
jyang at redhat.com
Fri May 17 11:31:14 UTC 2013
On 08/05/13 08:05, John Ferlan wrote:
> 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
>
To not introduce coverity errors anymore, I setup the coverity
env on my local box, and the errors in this patch are disapeared
after add the else branch:
} else {
return 0;
}
So pushed. Thanks.
Osier
More information about the libvir-list
mailing list