7:40 a.m.
See patches for details
John Ferlan (2):
qemu: Filter non SCSI hostdevs in qemuHostdevPrepareSCSIDevices
qemu: Remove virHostdevIsSCSIDevice from qemuIsSharedHostdev
src/qemu/qemu_conf.c | 3 +--
src/qemu/qemu_hostdev.c | 3 +++
2 files changed, 4 insertions(+), 2 deletions(-)
--
2.20.1
7:40 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: Filter non SCSI hostdevs in qemuHostdevPrepareSCSIDevices
When commit 1d94b3e7 added code to walk the [n]hostdevs list looking
to add shared hostdevs, it should've filtered any hostdevs that were
not SCSI hostdev's.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hostdev.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index a487e1d3aa..4eb3f1d7f1 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -275,6 +275,9 @@ qemuHostdevPrepareSCSIDevices(virQEMUDriverPtr driver,
for (i = 0; i < nhostdevs; i++) {
virDomainDeviceDef dev;
+ if (!virHostdevIsSCSIDevice(hostdevs[i]))
+ continue;
+
dev.type = VIR_DOMAIN_DEVICE_HOSTDEV;
dev.data.hostdev = hostdevs[i];
--
2.20.1
4:22 p.m.
New subject: [libvirt] [PATCH 1/2] qemu: Filter non SCSI hostdevs in qemuHostdevPrepareSCSIDevices
On Thu, Jan 10, 2019 at 06:40:32PM -0500, John Ferlan wrote:
When commit 1d94b3e7 added code to walk the [n]hostdevs list looking
to add shared hostdevs, it should've filtered any hostdevs that were
not SCSI hostdev's.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hostdev.c | 3 +++
1 file changed, 3 insertions(+)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
7:40 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: Remove virHostdevIsSCSIDevice from qemuIsSharedHostdev
It's essentially dead code. The only way hostdev->shareable
can be true is during virDomainHostdevDefParseXML when the
result of virHostdevIsSCSIDevice is true.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
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
4:26 p.m.
New subject: [libvirt] [PATCH 2/2] qemu: Remove virHostdevIsSCSIDevice from qemuIsSharedHostdev
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
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
8:44 p.m.
New subject: [libvirt] [PATCH 2/2] qemu: Remove virHostdevIsSCSIDevice from qemuIsSharedHostdev
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(a)redhat.com>
> ---
> 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(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
9:09 p.m.
New subject: [libvirt] [PATCH 2/2] qemu: Remove virHostdevIsSCSIDevice from qemuIsSharedHostdev
On Fri, Jan 11, 2019 at 07:44:38AM -0500, John Ferlan wrote:
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...
Yes,
Jano
2107
days inactive
2108
days old
6 comments
2 participants
participants (2)
-
John Ferlan -
Ján Tomko