Re: [PATCH 2/2] qemu: Remove virHostdevIsSCSIDevice from qemuIsSharedHostdev

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 1/11/19 3:26 AM, Ján Tomko wrote:
> On Thu, Jan 10, 2019 at 06:40:33PM -0500, John Ferlan wrote:
>> It's essentially dead code. The only way hostdev->shareable
>> can be true is during virDomainHostdevDefParseXML when the
>> result of virHostdevIsSCSIDevice is true.
> 
> While this might be true in our current codebase,
> I don't think this function should rely on the callers
> to only fill in hostdev->shareable for VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI
> to guard its usage of the hostdev->source union.
> 
> Jano
> 

OK - I guess I was trying to be "compete" and partially considering that
there's no reason this method wouldn't work for other hostdev types
(PCI, USB, SCSI_HOST, MDEV) beyond of course the fact that for those
types virDomainHostdevDefParseXML doesn't parse the readonly and
shareable subelements.

It's also not named qemuIsShareSCSIHostdev, but it does filter on SCSI.
Still the first patch ends up being the fix and I separated them because
I wasn't sure and figured I'd take another opinion on it!

Should I "assume" patch1 is SFF...

Tks -

John

>>
>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
>> ---
>> src/qemu/qemu_conf.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
>> index 20952e9607..945725566c 100644
>> --- a/src/qemu/qemu_conf.c
>> +++ b/src/qemu/qemu_conf.c
>> @@ -1438,8 +1438,7 @@ static bool
>> qemuIsSharedHostdev(virDomainHostdevDefPtr hostdev)
>> {
>>     return (hostdev->shareable &&
>> -            (virHostdevIsSCSIDevice(hostdev) &&
>> -             hostdev->source.subsys.u.scsi.protocol !=
>> +            (hostdev->source.subsys.u.scsi.protocol !=
>>              VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI));
>> }
>>
>> -- 
>> 2.20.1
>>
>> -- 
>> libvir-list mailing list
>> libvir-list@xxxxxxxxxx
>> https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux