[libvirt] [PATCH 0/2] Allow usage of unpriv_sgio for SCSI generic hostdev
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
Add necessary checks in order to allow setting sgio values for a scsi host device. Signed-off-by: John Ferlan <jferlan@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
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@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
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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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 :|
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
participants (3)
-
Daniel P. Berrangé -
John Ferlan -
Ján Tomko