[libvirt] [PATCH] qemu: Forbid "sgio" support for SCSI generic host device

The kernel didn't support the unprivileged SGIO for SCSI generic device finally, and since it's unknow whether the way to support unprivileged SGIO for SCSI generic device will be similar as for SCSI block device or not, even it's simliar (I.e. via sysfs, for SCSI block device, it's /sys/dev/block/8\:0/queue/unpriv_sgio, for example), the file name might be different, So it's better not guess what it should be like currently. This patch removes the related code (mainly about the "shareable" checking on the "sgio" setting, it's not supported at all, why we leave checking code there? :-), and error out if "sgio" is specified in the domain config. --- src/qemu/qemu_conf.c | 87 ++++++++++++---------------------------------------- 1 file changed, 20 insertions(+), 67 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 2c397b0..ad6348d 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -739,12 +739,8 @@ 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; @@ -756,27 +752,11 @@ qemuCheckSharedDevice(virHashTablePtr sharedDevices, */ if (disk->device != VIR_DOMAIN_DISK_DEVICE_LUN) return 0; - - device_path = disk->src; - } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { - hostdev = dev->data.hostdev; - - if (!(hostdev_name = virSCSIDeviceGetDevName(NULL, - 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) - goto cleanup; - - device_path = hostdev_path; } else { return 0; } - if (!(sysfs_path = virGetUnprivSGIOSysfsPath(device_path, NULL))) { + if (!(sysfs_path = virGetUnprivSGIOSysfsPath(disk->src, NULL))) { ret = -1; goto cleanup; } @@ -787,7 +767,7 @@ qemuCheckSharedDevice(virHashTablePtr sharedDevices, if (!virFileExists(sysfs_path)) goto cleanup; - if (!(key = qemuGetSharedDeviceKey(device_path))) { + if (!(key = qemuGetSharedDeviceKey(disk->src))) { ret = -1; goto cleanup; } @@ -798,7 +778,7 @@ qemuCheckSharedDevice(virHashTablePtr sharedDevices, if (!(virHashLookup(sharedDevices, key))) goto cleanup; - if (virGetDeviceUnprivSGIO(device_path, NULL, &val) < 0) { + if (virGetDeviceUnprivSGIO(disk->src, NULL, &val) < 0) { ret = -1; goto cleanup; } @@ -810,36 +790,25 @@ qemuCheckSharedDevice(virHashTablePtr sharedDevices, disk->sgio == VIR_DOMAIN_DEVICE_SGIO_UNFILTERED)) goto cleanup; - if (dev->type == VIR_DOMAIN_DEVICE_DISK) { - if (disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME) { - virReportError(VIR_ERR_OPERATION_INVALID, - _("sgio of shared disk 'pool=%s' 'volume=%s' conflicts " - "with other active domains"), - disk->srcpool->pool, - disk->srcpool->volume); - } else { - virReportError(VIR_ERR_OPERATION_INVALID, - _("sgio of shared disk '%s' conflicts with other " - "active domains"), disk->src); - } + if (disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("sgio of shared disk 'pool=%s' 'volume=%s' conflicts " + "with other active domains"), + disk->srcpool->pool, + disk->srcpool->volume); } else { virReportError(VIR_ERR_OPERATION_INVALID, - _("sgio of shared scsi host device '%s-%d-%d-%d' conflicts " - "with other active domains"), - hostdev->source.subsys.u.scsi.adapter, - hostdev->source.subsys.u.scsi.bus, - hostdev->source.subsys.u.scsi.target, - hostdev->source.subsys.u.scsi.unit); + _("sgio of shared disk '%s' conflicts with other " + "active domains"), disk->src); } ret = -1; cleanup: - VIR_FREE(hostdev_name); - VIR_FREE(hostdev_path); VIR_FREE(sysfs_path); VIR_FREE(key); return ret; } + bool qemuSharedDeviceEntryDomainExists(qemuSharedDeviceEntryPtr entry, const char *name, @@ -1116,8 +1085,6 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev) virDomainHostdevDefPtr hostdev = NULL; char *sysfs_path = NULL; char *path = NULL; - char *hostdev_name = NULL; - char *hostdev_path = NULL; int val = -1; int ret = 0; @@ -1135,22 +1102,15 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev) } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { hostdev = dev->data.hostdev; - if (!hostdev->shareable || - !(hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)) - return 0; - - if (!(hostdev_name = virSCSIDeviceGetDevName(NULL, - 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) + if (hostdev->source.subsys.u.scsi.sgio) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'sgio' is not supported for SCSI " + "generic device yet ")); + ret = -1; goto cleanup; + } - path = hostdev_path; + return 0; } else { return 0; } @@ -1162,12 +1122,7 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev) } /* By default, filter the SG_IO commands, i.e. set unpriv_sgio to 0. */ - - if (dev->type == VIR_DOMAIN_DEVICE_DISK) - val = (disk->sgio == VIR_DOMAIN_DEVICE_SGIO_UNFILTERED); - else - val = (hostdev->source.subsys.u.scsi.sgio == - VIR_DOMAIN_DEVICE_SGIO_UNFILTERED); + 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 @@ -1179,8 +1134,6 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev) cleanup: VIR_FREE(sysfs_path); - VIR_FREE(hostdev_name); - VIR_FREE(hostdev_path); return ret; } -- 1.8.1.4

On 07/03/14 22:23, Osier Yang wrote:
The kernel didn't support the unprivileged SGIO for SCSI generic device finally, and since it's unknow whether the way to support unprivileged SGIO for SCSI generic device will be similar as for SCSI block device or not, even it's simliar (I.e. via sysfs, for SCSI block device, it's /sys/dev/block/8\:0/queue/unpriv_sgio, for example), the file name might be different, So it's better not guess what it should be like currently.
This patch removes the related code (mainly about the "shareable" checking on the "sgio" setting, it's not supported at all, why we leave checking code there? :-), and error out if "sgio" is specified in the domain config. --- src/qemu/qemu_conf.c | 87 ++++++++++++---------------------------------------- 1 file changed, 20 insertions(+), 67 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 2c397b0..ad6348d 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -739,12 +739,8 @@ 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;
@@ -756,27 +752,11 @@ qemuCheckSharedDevice(virHashTablePtr sharedDevices, */ if (disk->device != VIR_DOMAIN_DISK_DEVICE_LUN) return 0; - - device_path = disk->src; - } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { - hostdev = dev->data.hostdev; - - if (!(hostdev_name = virSCSIDeviceGetDevName(NULL, - 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) - goto cleanup; - - device_path = hostdev_path; } else { return 0; }
- if (!(sysfs_path = virGetUnprivSGIOSysfsPath(device_path, NULL))) { + if (!(sysfs_path = virGetUnprivSGIOSysfsPath(disk->src, NULL))) { ret = -1; goto cleanup; } @@ -787,7 +767,7 @@ qemuCheckSharedDevice(virHashTablePtr sharedDevices, if (!virFileExists(sysfs_path)) goto cleanup;
- if (!(key = qemuGetSharedDeviceKey(device_path))) { + if (!(key = qemuGetSharedDeviceKey(disk->src))) { ret = -1; goto cleanup; } @@ -798,7 +778,7 @@ qemuCheckSharedDevice(virHashTablePtr sharedDevices, if (!(virHashLookup(sharedDevices, key))) goto cleanup;
- if (virGetDeviceUnprivSGIO(device_path, NULL, &val) < 0) { + if (virGetDeviceUnprivSGIO(disk->src, NULL, &val) < 0) { ret = -1; goto cleanup; } @@ -810,36 +790,25 @@ qemuCheckSharedDevice(virHashTablePtr sharedDevices, disk->sgio == VIR_DOMAIN_DEVICE_SGIO_UNFILTERED)) goto cleanup;
- if (dev->type == VIR_DOMAIN_DEVICE_DISK) { - if (disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME) { - virReportError(VIR_ERR_OPERATION_INVALID, - _("sgio of shared disk 'pool=%s' 'volume=%s' conflicts " - "with other active domains"), - disk->srcpool->pool, - disk->srcpool->volume); - } else { - virReportError(VIR_ERR_OPERATION_INVALID, - _("sgio of shared disk '%s' conflicts with other " - "active domains"), disk->src); - } + if (disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("sgio of shared disk 'pool=%s' 'volume=%s' conflicts " + "with other active domains"), + disk->srcpool->pool, + disk->srcpool->volume); } else { virReportError(VIR_ERR_OPERATION_INVALID, - _("sgio of shared scsi host device '%s-%d-%d-%d' conflicts " - "with other active domains"), - hostdev->source.subsys.u.scsi.adapter, - hostdev->source.subsys.u.scsi.bus, - hostdev->source.subsys.u.scsi.target, - hostdev->source.subsys.u.scsi.unit); + _("sgio of shared disk '%s' conflicts with other " + "active domains"), disk->src); }
ret = -1; cleanup: - VIR_FREE(hostdev_name); - VIR_FREE(hostdev_path); VIR_FREE(sysfs_path); VIR_FREE(key); return ret; } + bool qemuSharedDeviceEntryDomainExists(qemuSharedDeviceEntryPtr entry, const char *name, @@ -1116,8 +1085,6 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev) virDomainHostdevDefPtr hostdev = NULL; char *sysfs_path = NULL; char *path = NULL; - char *hostdev_name = NULL; - char *hostdev_path = NULL; int val = -1; int ret = 0;
@@ -1135,22 +1102,15 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev) } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { hostdev = dev->data.hostdev;
- if (!hostdev->shareable || - !(hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)) - return 0; - - if (!(hostdev_name = virSCSIDeviceGetDevName(NULL, - 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) + if (hostdev->source.subsys.u.scsi.sgio) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'sgio' is not supported for SCSI " + "generic device yet ")); + ret = -1;
With the attached patch squashed in:

On Fri, Mar 07, 2014 at 22:23:26 +0800, Osier Yang wrote:
The kernel didn't support the unprivileged SGIO for SCSI generic device finally, and since it's unknow whether the way to support unprivileged SGIO for SCSI generic device will be similar as for SCSI block device or not, even it's simliar (I.e. via sysfs, for SCSI block device, it's /sys/dev/block/8\:0/queue/unpriv_sgio, for example), the file name might be different, So it's better not guess what it should be like currently.
This patch removes the related code (mainly about the "shareable" checking on the "sgio" setting, it's not supported at all, why we leave checking code there? :-), and error out if "sgio" is specified in the domain config. --- src/qemu/qemu_conf.c | 87 ++++++++++++---------------------------------------- 1 file changed, 20 insertions(+), 67 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 2c397b0..ad6348d 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c ... @@ -1135,22 +1102,15 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev) } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { hostdev = dev->data.hostdev;
- if (!hostdev->shareable || - !(hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)) - return 0;
In the follow-up patch, you recovered just the hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI part. Shouldn't the other checks in this condition remain?
- - if (!(hostdev_name = virSCSIDeviceGetDevName(NULL, - 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) + if (hostdev->source.subsys.u.scsi.sgio) {
In other words, should this be something like if (hostdev->shareable && hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && hostdev->source.subsys.u.scsi.sgio) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'sgio' is not supported for SCSI " + "generic device yet ")); + ret = -1; goto cleanup; + }
- path = hostdev_path; + return 0; } else { return 0; }
Jirka

On 12/03/14 21:54, Jiri Denemark wrote:
On Fri, Mar 07, 2014 at 22:23:26 +0800, Osier Yang wrote:
The kernel didn't support the unprivileged SGIO for SCSI generic device finally, and since it's unknow whether the way to support unprivileged SGIO for SCSI generic device will be similar as for SCSI block device or not, even it's simliar (I.e. via sysfs, for SCSI block device, it's /sys/dev/block/8\:0/queue/unpriv_sgio, for example), the file name might be different, So it's better not guess what it should be like currently.
This patch removes the related code (mainly about the "shareable" checking on the "sgio" setting, it's not supported at all, why we leave checking code there? :-), and error out if "sgio" is specified in the domain config. --- src/qemu/qemu_conf.c | 87 ++++++++++++---------------------------------------- 1 file changed, 20 insertions(+), 67 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 2c397b0..ad6348d 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c ... @@ -1135,22 +1102,15 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev) } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { hostdev = dev->data.hostdev;
- if (!hostdev->shareable || - !(hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)) - return 0; In the follow-up patch, you recovered just the
hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI
part. Shouldn't the other checks in this condition remain?
- - if (!(hostdev_name = virSCSIDeviceGetDevName(NULL, - 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) + if (hostdev->source.subsys.u.scsi.sgio) { In other words, should this be something like
if (hostdev->shareable && hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && hostdev->source.subsys.u.scsi.sgio) {
Should be: If (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && hostdev->source.subsys.u.scsi.sgio) { Regardless of whether the SCSI generic device is shareable or not. We won't support the unpiv_sgio setting. I lost the SUBSYS checking in the follow up patch indeed. If no other issue, I can squash it in when pushing. Thanks for the reviewing. Osier

On Wed, Mar 12, 2014 at 23:13:24 +0800, Osier Yang wrote:
On 12/03/14 21:54, Jiri Denemark wrote:
On Fri, Mar 07, 2014 at 22:23:26 +0800, Osier Yang wrote:
The kernel didn't support the unprivileged SGIO for SCSI generic device finally, and since it's unknow whether the way to support unprivileged SGIO for SCSI generic device will be similar as for SCSI block device or not, even it's simliar (I.e. via sysfs, for SCSI block device, it's /sys/dev/block/8\:0/queue/unpriv_sgio, for example), the file name might be different, So it's better not guess what it should be like currently.
This patch removes the related code (mainly about the "shareable" checking on the "sgio" setting, it's not supported at all, why we leave checking code there? :-), and error out if "sgio" is specified in the domain config. --- src/qemu/qemu_conf.c | 87 ++++++++++++---------------------------------------- 1 file changed, 20 insertions(+), 67 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 2c397b0..ad6348d 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c ... @@ -1135,22 +1102,15 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev) } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { hostdev = dev->data.hostdev;
- if (!hostdev->shareable || - !(hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)) - return 0; In the follow-up patch, you recovered just the
hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI
part. Shouldn't the other checks in this condition remain?
- - if (!(hostdev_name = virSCSIDeviceGetDevName(NULL, - 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) + if (hostdev->source.subsys.u.scsi.sgio) { In other words, should this be something like
if (hostdev->shareable && hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && hostdev->source.subsys.u.scsi.sgio) {
Should be:
If (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && hostdev->source.subsys.u.scsi.sgio) {
Regardless of whether the SCSI generic device is shareable or not. We won't support the unpiv_sgio setting. I lost the SUBSYS checking in the follow up patch indeed. If no other issue, I can squash it in when pushing. Thanks for the reviewing.
OK, makes sense. ACK with this change then. Jirka

On 13/03/14 15:50, Jiri Denemark wrote:
On Wed, Mar 12, 2014 at 23:13:24 +0800, Osier Yang wrote:
On 12/03/14 21:54, Jiri Denemark wrote:
On Fri, Mar 07, 2014 at 22:23:26 +0800, Osier Yang wrote:
The kernel didn't support the unprivileged SGIO for SCSI generic device finally, and since it's unknow whether the way to support unprivileged SGIO for SCSI generic device will be similar as for SCSI block device or not, even it's simliar (I.e. via sysfs, for SCSI block device, it's /sys/dev/block/8\:0/queue/unpriv_sgio, for example), the file name might be different, So it's better not guess what it should be like currently.
This patch removes the related code (mainly about the "shareable" checking on the "sgio" setting, it's not supported at all, why we leave checking code there? :-), and error out if "sgio" is specified in the domain config. --- src/qemu/qemu_conf.c | 87 ++++++++++++---------------------------------------- 1 file changed, 20 insertions(+), 67 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 2c397b0..ad6348d 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c ... @@ -1135,22 +1102,15 @@ qemuSetUnprivSGIO(virDomainDeviceDefPtr dev) } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { hostdev = dev->data.hostdev;
- if (!hostdev->shareable || - !(hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)) - return 0; In the follow-up patch, you recovered just the
hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI
part. Shouldn't the other checks in this condition remain?
- - if (!(hostdev_name = virSCSIDeviceGetDevName(NULL, - 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) + if (hostdev->source.subsys.u.scsi.sgio) { In other words, should this be something like
if (hostdev->shareable && hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && hostdev->source.subsys.u.scsi.sgio) {
Should be:
If (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && hostdev->source.subsys.u.scsi.sgio) {
Regardless of whether the SCSI generic device is shareable or not. We won't support the unpiv_sgio setting. I lost the SUBSYS checking in the follow up patch indeed. If no other issue, I can squash it in when pushing. Thanks for the reviewing. OK, makes sense. ACK with this change then.
Thanks, pushed. Osier
participants (2)
-
Jiri Denemark
-
Osier Yang