10:50 a.m.
In what is perhaps a couple lifetimes ago at this point, patches
were posted and "mostly" all eventually accepted upstream to support
unpriv_sgio on SCSI generic hostdev devices. Since the needed parts
of the functionality from the kernel perspective were not yet present
upstream, a couple of the patches were not accepted. See:
https://www.redhat.com/archives/libvir-list/2015-July/msg00204.html
and in particular patches 9 and 10. Since that time it seems things
have changed and this essentially reposts those two patches with
some minor adjustments in logic in order to "restore" the support.
There are lots of interesting tidbits in unreadable by the general
public bz's, so I won't include links here.
John Ferlan (2):
qemu: Add ability to set sgio values for hostdev
qemu: Add check for unpriv sgio for SCSI generic host device
src/qemu/qemu_conf.c | 34 ++++++++++++++++++++++++----------
1 file changed, 24 insertions(+), 10 deletions(-)
--
2.17.1
10:50 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: Add ability to set sgio values for hostdev
Add necessary checks in order to allow setting sgio values for a scsi
host device.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_conf.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index a4f545ef92..3d28c03fa1 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1633,13 +1633,15 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev)
virDomainDiskDefPtr disk = NULL;
virDomainHostdevDefPtr hostdev = NULL;
char *sysfs_path = NULL;
+ char *hostdev_path = NULL;
const char *path = NULL;
int val = -1;
int ret = -1;
/* "sgio" is only valid for block disk; cdrom
* and floopy disk can have empty source.
- */
+ * NB: The default is to filter SG_IO commands
+ * (i.e. set unpriv_sgio to 0). */
if (dev->type == VIR_DOMAIN_DEVICE_DISK) {
disk = dev->data.disk;
@@ -1648,20 +1650,19 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev)
return 0;
path = virDomainDiskGetSource(disk);
+ val = (disk->sgio == VIR_DOMAIN_DEVICE_SGIO_UNFILTERED);
} else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) {
hostdev = dev->data.hostdev;
if (!qemuIsSharedHostdev(hostdev))
return 0;
- if (hostdev->source.subsys.u.scsi.sgio) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("'sgio' is not supported for SCSI "
- "generic device yet "));
+ if (!(hostdev_path = qemuGetHostdevPath(hostdev)))
goto cleanup;
- }
- return 0;
+ path = hostdev_path;
+ val = (hostdev->source.subsys.u.scsi.sgio ==
+ VIR_DOMAIN_DEVICE_SGIO_UNFILTERED);
} else {
return 0;
}
@@ -1669,9 +1670,6 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev)
if (!(sysfs_path = virGetUnprivSGIOSysfsPath(path, NULL)))
goto cleanup;
- /* By default, filter the SG_IO commands, i.e. set unpriv_sgio to 0. */
- val = (disk->sgio == VIR_DOMAIN_DEVICE_SGIO_UNFILTERED);
-
/* Do not do anything if unpriv_sgio is not supported by the kernel and the
* whitelist is enabled. But if requesting unfiltered access, always call
* virSetDeviceUnprivSGIO, to report an error for unsupported unpriv_sgio.
@@ -1683,6 +1681,7 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev)
ret = 0;
cleanup:
+ VIR_FREE(hostdev_path);
VIR_FREE(sysfs_path);
return ret;
}
--
2.17.1
10:50 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: Add check for unpriv sgio for SCSI generic host device
Check if the hostdev has set the sgio filtered/unfiltered and handle
appropriately.
This restores functionality removed by commit id 'ce346623' to remove
sgio support for the SCSI generic host device in the formerly named
method qemuCheckSharedDevice.
For most kernels the result of this operation is a no-op; however, for
those that do support it the check is necessary.
Note that this patch fixes a bug found in the reverted change where
if the qemuGetDeviceUnprivSGIO returned either 0 or 1 in 'val', the
'disk' would be dereferenced; however, since a hostdev didn't have
one - that dereference would have caused a segfault. That was fixed
by commit id 'cd01d2ad5' using virDomainDiskGetSource. Instead these
changes use the hostdev's address value.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_conf.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 3d28c03fa1..3f24c8b8fc 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1473,6 +1473,8 @@ qemuAddSharedHostdev(virQEMUDriverPtr driver,
{
char *dev_path = NULL;
char *key = NULL;
+ virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi;
+ virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host;
int ret = -1;
if (!qemuIsSharedHostdev(hostdev))
@@ -1481,6 +1483,19 @@ qemuAddSharedHostdev(virQEMUDriverPtr driver,
if (!(dev_path = qemuGetHostdevPath(hostdev)))
goto cleanup;
+ if ((ret = qemuCheckUnprivSGIO(driver->sharedDevices, dev_path,
+ scsisrc->sgio)) < 0) {
+ if (ret == -2) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ _("sgio of shared scsi host device
'%s-%u-%u-%llu' "
+ "conflicts with other active domains"),
+ scsihostsrc->adapter, scsihostsrc->bus,
+ scsihostsrc->target, scsihostsrc->unit);
+ ret = -1;
+ }
+ goto cleanup;
+ }
+
if (!(key = qemuGetSharedDeviceKey(dev_path)))
goto cleanup;
--
2.17.1
6:32 a.m.
On Thu, Aug 23, 2018 at 11:50:00AM -0400, John Ferlan wrote:
In what is perhaps a couple lifetimes ago at this point, patches
were posted and "mostly" all eventually accepted upstream to support
unpriv_sgio on SCSI generic hostdev devices. Since the needed parts
of the functionality from the kernel perspective were not yet present
upstream, a couple of the patches were not accepted. See:
https://www.redhat.com/archives/libvir-list/2015-July/msg00204.html
and in particular patches 9 and 10. Since that time it seems things
have changed
Nice!
Do you have the kernel commit IDs?
Jano
and this essentially reposts those two patches with
some minor adjustments in logic in order to "restore" the support.
There are lots of interesting tidbits in unreadable by the general
public bz's, so I won't include links here.
John Ferlan (2):
qemu: Add ability to set sgio values for hostdev
qemu: Add check for unpriv sgio for SCSI generic host device
src/qemu/qemu_conf.c | 34 ++++++++++++++++++++++++----------
1 file changed, 24 insertions(+), 10 deletions(-)
--
2.17.1
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
8:02 a.m.
On Fri, Aug 24, 2018 at 01:32:41PM +0200, Ján Tomko wrote:
On Thu, Aug 23, 2018 at 11:50:00AM -0400, John Ferlan wrote:
> In what is perhaps a couple lifetimes ago at this point, patches
> were posted and "mostly" all eventually accepted upstream to support
> unpriv_sgio on SCSI generic hostdev devices. Since the needed parts
> of the functionality from the kernel perspective were not yet present
> upstream, a couple of the patches were not accepted. See:
>
> https://www.redhat.com/archives/libvir-list/2015-July/msg00204.html
>
> and in particular patches 9 and 10. Since that time it seems things
> have changed
Nice!
Do you have the kernel commit IDs?
I've not found any sign of the "unpriv_sgio" sysfs file existing
in today's upstream LKML tree, so I'm not sure this is correct.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
8:12 a.m.
On 08/24/2018 09:02 AM, Daniel P. Berrangé wrote:
On Fri, Aug 24, 2018 at 01:32:41PM +0200, Ján Tomko wrote:
> On Thu, Aug 23, 2018 at 11:50:00AM -0400, John Ferlan wrote:
>> In what is perhaps a couple lifetimes ago at this point, patches
>> were posted and "mostly" all eventually accepted upstream to support
>> unpriv_sgio on SCSI generic hostdev devices. Since the needed parts
>> of the functionality from the kernel perspective were not yet present
>> upstream, a couple of the patches were not accepted. See:
>>
>> https://www.redhat.com/archives/libvir-list/2015-July/msg00204.html
>>
>> and in particular patches 9 and 10. Since that time it seems things
>> have changed
>
> Nice!
>
> Do you have the kernel commit IDs?
I've not found any sign of the "unpriv_sgio" sysfs file existing
in today's upstream LKML tree, so I'm not sure this is correct.
OK then, so much for assumptions... Well then we'll ignore this for
now.. It's rather a painful to follow all the links in the private bz's
on this.
John
2282
days inactive
2283
days old
5 comments
3 participants
participants (3)
-
Daniel P. Berrangé -
John Ferlan -
Ján Tomko