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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list