* Re: [PATCH 34/40] Staging: hv: storvsc: Add the contents of hyperv_storage.h to storvsc_drv.c
From: Christoph Hellwig @ 2011-07-01 8:16 UTC (permalink / raw)
To: KY Srinivasan
Cc: Christoph Hellwig, gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang, Abhishek Kane (Mindtree Consulting PVT LTD),
Hank Janssen
In-Reply-To: <6E21E5352C11B742B20C142EB499E0480816C297@TK5EX14MBXC126.redmond.corp.microsoft.com>
On Thu, Jun 30, 2011 at 08:13:51PM +0000, KY Srinivasan wrote:
> > > Add the contents of hyperv_storage.h to storvsc_drv.c and cleanup
> > storvsc_drv.c.n
> >
> > I'd at least leave the first half of the header that defines the
> > protocol around.
>
> I only got rid of the block comment at the start of hyperv_storage.h
> and consolidated the include files. Nothing of substance was deleted.
I meant to say keeping a separate header file for the protocol defintion
might be a good idea. That way it is kept separate from the
implementation details
^ permalink raw reply
* Re: [PATCH 38/40] Staging: hv: storvsc: Fixup srb_status for INQUIRY and MODE_SENSE
From: Christoph Hellwig @ 2011-07-01 8:00 UTC (permalink / raw)
To: KY Srinivasan
Cc: Christoph Hellwig, gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang, Abhishek Kane (Mindtree Consulting PVT LTD),
Hank Janssen
In-Reply-To: <6E21E5352C11B742B20C142EB499E0480816C27C@TK5EX14MBXC126.redmond.corp.microsoft.com>
On Thu, Jun 30, 2011 at 07:59:28PM +0000, KY Srinivasan wrote:
> The reason I did this was so that I could key off on real failures indicated by
> srb_status == 0x4 to off-line the device.
Ok. Please document what your doing in detail in the code, though.
^ permalink raw reply
* Re: virtio scsi host draft specification, v3
From: Hannes Reinecke @ 2011-07-01 7:14 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Christoph Hellwig, Stefan Hajnoczi, kvm, Michael S. Tsirkin,
qemu-devel, Linux Kernel Mailing List, Linux Virtualization
In-Reply-To: <4E0D6C08.1040201@redhat.com>
On 07/01/2011 08:41 AM, Paolo Bonzini wrote:
> On 06/29/2011 11:39 AM, Stefan Hajnoczi wrote:
>> > > Of course, when doing so we would be lose the ability to
>> freely remap
>> > > LUNs. But then remapping LUNs doesn't gain you much imho.
>> > > Plus you could always use qemu block backend here if you want
>> > > to hide the details.
>> >
>> > And you could always use the QEMU block backend with
>> > scsi-generic if you want to remap LUNs, instead of true
>> > passthrough via the kernel target.
>>
>> IIUC the in-kernel target always does remapping. It passes through
>> individual LUNs rather than entire targets and you pick LU Numbers to
>> map to the backing storage (which may or may not be a SCSI
>> pass-through device). Nicholas Bellinger can confirm whether this is
>> correct.
>
> But then I don't understand. If you pick LU numbers both with the
> in-kernel target and with QEMU, you do not need to use e.g. WWPNs
> with fiber channel, because we are not passing through the details
> of the transport protocol (one day we might have virtio-fc, but more
> likely not). So the LUNs you use might as well be represented by
> hierarchical LUNs.
>
Actually, the kernel does _not_ do a LUN remapping. It just so
happens that most storage arrays will present the LUN starting with
0, so normally you wouldn't notice.
However, some arrays have an array-wide LUN range, so you start
seeing LUNs at odd places:
[3:0:5:0] disk LSI INF-01-00 0750 /dev/sdw
[3:0:5:7] disk LSI Universal Xport 0750 /dev/sdx
> Using NPIV with KVM would be done by mapping the same virtual N_Port
> ID in the host(s) to the same LU number in the guest. You might
> already do this now with virtio-blk, in fact.
>
The point here is not the mapping. The point is rescanning.
You can map existing NPIV devices already. But you _cannot_ rescan
the host/device whatever _from the guest_ to detect if new devices
are present.
That is the problem I'm trying to describe here.
To be more explicit:
Currently you have to map existing devices directly as individual
block or scsi devices to the guest.
And rescan within the guest can only be sent to that device, so the
only information you will get able to gather is if the device itself
is still present.
You are unable to detect if there are other devices attached to your
guest which you should connect to.
So we have to have an enclosing instance (ie the equivalent of a
SCSI target), which is capable of telling us exactly this.
> Put in another way: the virtio-scsi device is itself a SCSI target,
> so yes, there is a single target port identifier in virtio-scsi. But
> this SCSI target just passes requests down to multiple real targets,
> and so will let you do ALUA and all that.
>
Argl. No way. The virtio-scsi device has to map to a single LUN.
I thought I mentioned this already, but I'd better clarify this again:
The SCSI spec itself only deals with LUNs, so anything you'll read
in there obviously will only handle the interaction between the
initiator (read: host) and the LUN itself. However, the actual
command is send via an intermediat target, hence you'll always see
the reference to the ITL (initiator-target-lun) nexus.
The SCSI spec details discovery of the individual LUNs presented by
a given target, it does _NOT_ detail the discovery of the targets
themselves.
That is being delegated to the underlying transport, in most cases
SAS or FibreChannel.
For the same reason the SCSI spec can afford to disdain any
reference to path failure, device hot-plugging etc; all of these
things are being delegated to the transport.
In our context the virtio-scsi device should map to the LUN, and the
virtio-scsi _host_ backend should map to the target.
The virtio-scsi _guest_ driver will then map to the initiator.
So we should be able to attach more than one device to the backend,
which then will be presented to the initiator.
In the case of NPIV it would make sense to map the virtual SCSI host
to the backend, so that all devices presented to the virtual SCSI
host will be presented to the backend, too.
However, when doing so these devices will normally be referenced by
their original LUN, as these will be presented to the guest via eg
'REPORT LUNS'.
The above thread now tries to figure out if we should remap those
LUN numbers or just expose them as they are.
If we decide on remapping, we have to emulate _all_ commands
referring explicitely to those LUN numbers (persistent reservations,
anyone?). If we don't, we would expose some hardware detail to the
guest, but would save us _a lot_ of processing.
I'm all for the latter.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
^ permalink raw reply
* Re: virtio scsi host draft specification, v3
From: Paolo Bonzini @ 2011-07-01 6:41 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Christoph Hellwig, Stefan Hajnoczi, kvm, Michael S. Tsirkin,
qemu-devel, Linux Kernel Mailing List, Linux Virtualization
In-Reply-To: <BANLkTik2AXXkWvV23OP1eDdLirP2N0PC3A@mail.gmail.com>
On 06/29/2011 11:39 AM, Stefan Hajnoczi wrote:
> > > Of course, when doing so we would be lose the ability to freely remap
> > > LUNs. But then remapping LUNs doesn't gain you much imho.
> > > Plus you could always use qemu block backend here if you want
> > > to hide the details.
> >
> > And you could always use the QEMU block backend with scsi-generic if you
> > want to remap LUNs, instead of true passthrough via the kernel target.
>
> IIUC the in-kernel target always does remapping. It passes through
> individual LUNs rather than entire targets and you pick LU Numbers to
> map to the backing storage (which may or may not be a SCSI
> pass-through device). Nicholas Bellinger can confirm whether this is
> correct.
But then I don't understand. If you pick LU numbers both with the
in-kernel target and with QEMU, you do not need to use e.g. WWPNs with
fiber channel, because we are not passing through the details of the
transport protocol (one day we might have virtio-fc, but more likely
not). So the LUNs you use might as well be represented by hierarchical
LUNs.
Using NPIV with KVM would be done by mapping the same virtual N_Port ID
in the host(s) to the same LU number in the guest. You might already do
this now with virtio-blk, in fact.
Put in another way: the virtio-scsi device is itself a SCSI target, so
yes, there is a single target port identifier in virtio-scsi. But this
SCSI target just passes requests down to multiple real targets, and so
will let you do ALUA and all that.
Of course if I am dead wrong please correct me.
Paolo
^ permalink raw reply
* Re: [PATCH 00/40] Staging: hv: Driver cleanup
From: Stephen Hemminger @ 2011-07-01 4:45 UTC (permalink / raw)
To: KY Srinivasan
Cc: Christoph Hellwig, devel@linuxdriverproject.org, gregkh@suse.de,
linux-kernel@vger.kernel.org, virtualization@lists.osdl.org
In-Reply-To: <6E21E5352C11B742B20C142EB499E0480816C3AF@TK5EX14MBXC126.redmond.corp.microsoft.com>
On Fri, 1 Jul 2011 00:19:38 +0000
KY Srinivasan <kys@microsoft.com> wrote:
>
>
> > -----Original Message-----
> > From: Stephen Hemminger [mailto:shemminger@vyatta.com]
> > Sent: Thursday, June 30, 2011 7:48 PM
> > To: KY Srinivasan
> > Cc: Christoph Hellwig; devel@linuxdriverproject.org; gregkh@suse.de; linux-
> > kernel@vger.kernel.org; virtualization@lists.osdl.org
> > Subject: Re: [PATCH 00/40] Staging: hv: Driver cleanup
> >
> > On Thu, 30 Jun 2011 23:32:34 +0000
> > KY Srinivasan <kys@microsoft.com> wrote:
> >
> > >
> > > > -----Original Message-----
> > > > From: Christoph Hellwig [mailto:hch@infradead.org]
> > > > Sent: Thursday, June 30, 2011 3:34 PM
> > > > To: KY Srinivasan
> > > > Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> > > > devel@linuxdriverproject.org; virtualization@lists.osdl.org
> > > > Subject: Re: [PATCH 00/40] Staging: hv: Driver cleanup
> > > >
> > > > On Wed, Jun 29, 2011 at 07:38:21AM -0700, K. Y. Srinivasan wrote:
> > > > > Further cleanup of the hv drivers:
> > > > >
> > > > > 1) Cleanup the reference counting mess for both stor and net devices.
> > > >
> > > > I really don't understand the need for reference counting on the storage
> > > > side, especially now that you only have a SCSI driver. The SCSI
> > > > midlayer does proper counting on it's objects (Scsi_Host, scsi_device,
> > > > scsi_cmnd), so you'll get that for free given that SCSI drivers just
> > > > piggyback on the midlayer lifetime rules.
> > > >
> > > > For now your patches should probably go in as-is, but mid-term you
> > > > should be able to completely remove that code on the storage side.
> > > >
> > >
> > > Greg,
> > >
> > > I am thinking of going back to my original implementation where I had one scsi
> > host
> > > per IDE device. This will certainly simply the code. Let me know what you think.
> > If you
> > > agree with this approach, please drop this patch-set, I will send you a new set
> > of patches.
> >
> > I think there ref counting on network devices is also unneeded
> > as long as the unregister logic handles RCU correctly. The network layer
> > calls the driver unregister routine after all packets are gone.
> On the networking side, what about incoming packets that may be racing
> with the device destruction. The current ref counting scheme deals with
> that case.
Not sure how HV driver tells hypervisor to stop sending packets. But the
destructor is not called until after all other CPU's are done processing
packets from that device.
^ permalink raw reply
* Re: [PATCH 7/7] [v6] drivers/virt: introduce Freescale hypervisor management driver
From: Tabi Timur-B04825 @ 2011-07-01 1:18 UTC (permalink / raw)
To: Arnd Bergmann
Cc: konrad.wilk@oracle.com, benh@kernel.crashing.org,
linux-kernel@vger.kernel.org, cmetcalf@tilera.com,
dsaxena@linaro.org, linux-console@vger.kernel.org, greg@kroah.com,
Gala Kumar-B11780, virtualization@lists.linux-foundation.org,
linuxppc-dev@lists.ozlabs.org, alan@lxorguk.ukuu.org.uk
In-Reply-To: <201106092304.55479.arnd@arndb.de>
On Thu, Jun 9, 2011 at 4:04 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 09 June 2011 22:52:06 Timur Tabi wrote:
>> Add the drivers/virt directory, which houses drivers that support
>> virtualization environments, and add the Freescale hypervisor management
>> driver.
....
>>
>> Signed-off-by: Timur Tabi <timur@freescale.com>
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
So I've made the changes that people have asked for, and Arnd has
acked this driver. Who's going to pick it up? Who will be the
maintainer for drivers/virt? I'd really like to see this driver in
3.1, and there's not much time left.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* RE: [PATCH 00/40] Staging: hv: Driver cleanup
From: KY Srinivasan @ 2011-07-01 0:19 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Christoph Hellwig, devel@linuxdriverproject.org, gregkh@suse.de,
linux-kernel@vger.kernel.org, virtualization@lists.osdl.org
In-Reply-To: <20110630164808.17c506a4@nehalam.ftrdhcpuser.net>
> -----Original Message-----
> From: Stephen Hemminger [mailto:shemminger@vyatta.com]
> Sent: Thursday, June 30, 2011 7:48 PM
> To: KY Srinivasan
> Cc: Christoph Hellwig; devel@linuxdriverproject.org; gregkh@suse.de; linux-
> kernel@vger.kernel.org; virtualization@lists.osdl.org
> Subject: Re: [PATCH 00/40] Staging: hv: Driver cleanup
>
> On Thu, 30 Jun 2011 23:32:34 +0000
> KY Srinivasan <kys@microsoft.com> wrote:
>
> >
> > > -----Original Message-----
> > > From: Christoph Hellwig [mailto:hch@infradead.org]
> > > Sent: Thursday, June 30, 2011 3:34 PM
> > > To: KY Srinivasan
> > > Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> > > devel@linuxdriverproject.org; virtualization@lists.osdl.org
> > > Subject: Re: [PATCH 00/40] Staging: hv: Driver cleanup
> > >
> > > On Wed, Jun 29, 2011 at 07:38:21AM -0700, K. Y. Srinivasan wrote:
> > > > Further cleanup of the hv drivers:
> > > >
> > > > 1) Cleanup the reference counting mess for both stor and net devices.
> > >
> > > I really don't understand the need for reference counting on the storage
> > > side, especially now that you only have a SCSI driver. The SCSI
> > > midlayer does proper counting on it's objects (Scsi_Host, scsi_device,
> > > scsi_cmnd), so you'll get that for free given that SCSI drivers just
> > > piggyback on the midlayer lifetime rules.
> > >
> > > For now your patches should probably go in as-is, but mid-term you
> > > should be able to completely remove that code on the storage side.
> > >
> >
> > Greg,
> >
> > I am thinking of going back to my original implementation where I had one scsi
> host
> > per IDE device. This will certainly simply the code. Let me know what you think.
> If you
> > agree with this approach, please drop this patch-set, I will send you a new set
> of patches.
>
> I think there ref counting on network devices is also unneeded
> as long as the unregister logic handles RCU correctly. The network layer
> calls the driver unregister routine after all packets are gone.
On the networking side, what about incoming packets that may be racing
with the device destruction. The current ref counting scheme deals with
that case.
Regards,
K. Y
^ permalink raw reply
* Re: [PATCH 00/40] Staging: hv: Driver cleanup
From: Stephen Hemminger @ 2011-06-30 23:48 UTC (permalink / raw)
To: KY Srinivasan
Cc: Christoph Hellwig, devel@linuxdriverproject.org, gregkh@suse.de,
linux-kernel@vger.kernel.org, virtualization@lists.osdl.org
In-Reply-To: <6E21E5352C11B742B20C142EB499E0480816C384@TK5EX14MBXC126.redmond.corp.microsoft.com>
On Thu, 30 Jun 2011 23:32:34 +0000
KY Srinivasan <kys@microsoft.com> wrote:
>
> > -----Original Message-----
> > From: Christoph Hellwig [mailto:hch@infradead.org]
> > Sent: Thursday, June 30, 2011 3:34 PM
> > To: KY Srinivasan
> > Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> > devel@linuxdriverproject.org; virtualization@lists.osdl.org
> > Subject: Re: [PATCH 00/40] Staging: hv: Driver cleanup
> >
> > On Wed, Jun 29, 2011 at 07:38:21AM -0700, K. Y. Srinivasan wrote:
> > > Further cleanup of the hv drivers:
> > >
> > > 1) Cleanup the reference counting mess for both stor and net devices.
> >
> > I really don't understand the need for reference counting on the storage
> > side, especially now that you only have a SCSI driver. The SCSI
> > midlayer does proper counting on it's objects (Scsi_Host, scsi_device,
> > scsi_cmnd), so you'll get that for free given that SCSI drivers just
> > piggyback on the midlayer lifetime rules.
> >
> > For now your patches should probably go in as-is, but mid-term you
> > should be able to completely remove that code on the storage side.
> >
>
> Greg,
>
> I am thinking of going back to my original implementation where I had one scsi host
> per IDE device. This will certainly simply the code. Let me know what you think. If you
> agree with this approach, please drop this patch-set, I will send you a new set of patches.
I think there ref counting on network devices is also unneeded
as long as the unregister logic handles RCU correctly. The network layer
calls the driver unregister routine after all packets are gone.
^ permalink raw reply
* RE: [PATCH 00/40] Staging: hv: Driver cleanup
From: KY Srinivasan @ 2011-06-30 23:32 UTC (permalink / raw)
To: Christoph Hellwig
Cc: devel@linuxdriverproject.org, gregkh@suse.de,
linux-kernel@vger.kernel.org, virtualization@lists.osdl.org
In-Reply-To: <20110630193346.GA22707@infradead.org>
> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Thursday, June 30, 2011 3:34 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org
> Subject: Re: [PATCH 00/40] Staging: hv: Driver cleanup
>
> On Wed, Jun 29, 2011 at 07:38:21AM -0700, K. Y. Srinivasan wrote:
> > Further cleanup of the hv drivers:
> >
> > 1) Cleanup the reference counting mess for both stor and net devices.
>
> I really don't understand the need for reference counting on the storage
> side, especially now that you only have a SCSI driver. The SCSI
> midlayer does proper counting on it's objects (Scsi_Host, scsi_device,
> scsi_cmnd), so you'll get that for free given that SCSI drivers just
> piggyback on the midlayer lifetime rules.
>
> For now your patches should probably go in as-is, but mid-term you
> should be able to completely remove that code on the storage side.
>
Greg,
I am thinking of going back to my original implementation where I had one scsi host
per IDE device. This will certainly simply the code. Let me know what you think. If you
agree with this approach, please drop this patch-set, I will send you a new set of patches.
Regards,
K. Y
^ permalink raw reply
* RE: [PATCH 00/40] Staging: hv: Driver cleanup
From: KY Srinivasan @ 2011-06-30 23:28 UTC (permalink / raw)
To: Christoph Hellwig
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org
In-Reply-To: <20110630193346.GA22707@infradead.org>
> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Thursday, June 30, 2011 3:34 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org
> Subject: Re: [PATCH 00/40] Staging: hv: Driver cleanup
>
> On Wed, Jun 29, 2011 at 07:38:21AM -0700, K. Y. Srinivasan wrote:
> > Further cleanup of the hv drivers:
> >
> > 1) Cleanup the reference counting mess for both stor and net devices.
>
> I really don't understand the need for reference counting on the storage
> side, especially now that you only have a SCSI driver. The SCSI
> midlayer does proper counting on it's objects (Scsi_Host, scsi_device,
> scsi_cmnd), so you'll get that for free given that SCSI drivers just
> piggyback on the midlayer lifetime rules.
The reference counting allows us to properly deal with messages coming back from the host
to the guest with a racing remove of the device. I am told these messages could potentially be
not a response to a message sent from the guest.
>
> For now your patches should probably go in as-is, but mid-term you
> should be able to completely remove that code on the storage side.
>
Thanks. Sure, we will always try to simplify the code.
Regards,
K. Y
^ permalink raw reply
* RE: [PATCH 23/40] Staging: hv: storvsc: Introduce code to manage IDE devices using storvsc HBA
From: KY Srinivasan @ 2011-06-30 21:38 UTC (permalink / raw)
To: Christoph Hellwig
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang, Abhishek Kane (Mindtree Consulting PVT LTD),
Hank Janssen
In-Reply-To: <20110630193843.GB22707@infradead.org>
> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Thursday, June 30, 2011 3:39 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang;
> Abhishek Kane (Mindtree Consulting PVT LTD); Hank Janssen
> Subject: Re: [PATCH 23/40] Staging: hv: storvsc: Introduce code to manage IDE
> devices using storvsc HBA
>
> > +/*
> > + * We want to manage the IDE devices using standard Linux SCSI drivers
> > + * using the storvsc driver.
> > + * Define special channels to support this.
> > + */
> > +
> > +#define HV_MAX_IDE_DEVICES 4
> > +#define HV_IDE_BASE_CHANNEL 10
> > +#define HV_IDE0_DEV1 HV_IDE_BASE_CHANNEL
> > +#define HV_IDE0_DEV2 (HV_IDE_BASE_CHANNEL + 1)
> > +#define HV_IDE1_DEV1 (HV_IDE_BASE_CHANNEL + 2)
> > +#define HV_IDE1_DEV2 (HV_IDE_BASE_CHANNEL + 3)
>
> This at last needs a good explanation of why these devices are called
> IDE if they actually aren't. I know you've explained the reason to me
> before, but it should also be in the code.
These devices are configured as IDE devices for the guest. The current
emulator supports 2 IDE controllers for a total of potentially 4 devices.
I did this to support all these 4 devices under one scsi host and used the
channel information to get at the correct device in the I/O path.
So, if you go to a model with one host per device, this would not be required.
>
> The HV_IDE1_DEVn defines don't seem to useful to me. They are just
> used in one place, and doing an opencoded HV_IDE_BASE_CHANNEL +
> channel_nr would seem a lot easier to understand to me.
>
> > +static struct Scsi_Host *storvsc_host;
> > +
> > +/*
> > + * State to manage IDE devices that register with the storvsc driver.
> > + *
> > + */
> > +static struct hv_device *ide_devices[HV_MAX_IDE_DEVICES];
> > +
> > +static void storvsc_get_ide_info(struct hv_device *dev, int *target, int *path)
> > +{
> > + *target =
> > + dev->dev_instance.data[5] << 8 | dev->dev_instance.data[4];
> > +
> > + *path =
> > + dev->dev_instance.data[3] << 24 | dev->dev_instance.data[2] << 16 |
> > + dev->dev_instance.data[1] << 8 | dev->dev_instance.data[0];
>
> Pretty odd formatting, I'd rather do it as:
>
> *target =
> dev->dev_instance.data[5] << 8 |
> dev->dev_instance.data[4];
>
> but more importanly what does path actually stand for here? Opencoding
> this into the caller and adding proper comments explaining the scheme
> might be more readable.
In the blkvsc driver, the path/target info was used to properly identify the
device - (a) the device was under the first or second IDE controller and (b)
whether it is the first or second device under the controller.
Regards,
K. Y
.
^ permalink raw reply
* RE: [PATCH 24/40] Staging: hv: storvsc: On I/O get the correct IDE device
From: KY Srinivasan @ 2011-06-30 21:15 UTC (permalink / raw)
To: Christoph Hellwig
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang, Abhishek Kane (Mindtree Consulting PVT LTD),
Hank Janssen
In-Reply-To: <20110630194055.GC22707@infradead.org>
> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Thursday, June 30, 2011 3:41 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang;
> Abhishek Kane (Mindtree Consulting PVT LTD); Hank Janssen
> Subject: Re: [PATCH 24/40] Staging: hv: storvsc: On I/O get the correct IDE device
>
> On Wed, Jun 29, 2011 at 07:39:21AM -0700, K. Y. Srinivasan wrote:
> > We use the channel number to distinguish an IDE device managed by the
> > storvsc driver from scsi devices. Add code to get the correct
> > device pointer based on the channel number.
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
> > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> > ---
> > drivers/staging/hv/storvsc_drv.c | 10 ++++++++++
> > 1 files changed, 10 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
> > index cf659d7..fcc3f5d 100644
> > --- a/drivers/staging/hv/storvsc_drv.c
> > +++ b/drivers/staging/hv/storvsc_drv.c
> > @@ -517,6 +517,16 @@ static int storvsc_queuecommand_lck(struct scsi_cmnd
> *scmnd,
> > unsigned int sg_count = 0;
> > struct vmscsi_request *vm_srb;
> >
> > + if (scmnd->device->channel >= HV_IDE_BASE_CHANNEL) {
> > + int channel = scmnd->device->channel;
> > +
> > + /*
> > + * This is an IDE device; get the right dev.
> > + */
> > +
> > + dev = ide_devices[channel - HV_IDE_BASE_CHANNEL];
> > + }
>
> So instead of playing games about getting the right hv_device here,
> why don't you register one scsi host for each IDE device? libata
> does the same for real ATA devices.
>
That is what I did initially. Then looking at the way we were handling scsi devices
where each scsi controller configured for the guest results in an emulated HBA
(scsi host) in the guest and all the block devices under a given controller are
handled through this one host, I decided to mimic a similar structure - one
scsi host for all the block devices configured as an IDE device.
I can go back to my earlier implementation with one host per disk.
Regards,
K. Y
^ permalink raw reply
* RE: [PATCH 34/40] Staging: hv: storvsc: Add the contents of hyperv_storage.h to storvsc_drv.c
From: KY Srinivasan @ 2011-06-30 20:13 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Abhishek Kane (Mindtree Consulting PVT LTD), Haiyang Zhang,
gregkh@suse.de, linux-kernel@vger.kernel.org,
virtualization@lists.osdl.org, devel@linuxdriverproject.org
In-Reply-To: <20110630194530.GE22707@infradead.org>
> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Thursday, June 30, 2011 3:46 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang;
> Abhishek Kane (Mindtree Consulting PVT LTD); Hank Janssen
> Subject: Re: [PATCH 34/40] Staging: hv: storvsc: Add the contents of
> hyperv_storage.h to storvsc_drv.c
>
> On Wed, Jun 29, 2011 at 07:39:31AM -0700, K. Y. Srinivasan wrote:
> > Add the contents of hyperv_storage.h to storvsc_drv.c and cleanup
> storvsc_drv.c.n
>
> I'd at least leave the first half of the header that defines the
> protocol around.
I only got rid of the block comment at the start of hyperv_storage.h
and consolidated the include files. Nothing of substance was deleted.
Regards,
K. Y
^ permalink raw reply
* RE: [PATCH 38/40] Staging: hv: storvsc: Fixup srb_status for INQUIRY and MODE_SENSE
From: KY Srinivasan @ 2011-06-30 19:59 UTC (permalink / raw)
To: Christoph Hellwig
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang, Abhishek Kane (Mindtree Consulting PVT LTD),
Hank Janssen
In-Reply-To: <20110630194754.GG22707@infradead.org>
> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Thursday, June 30, 2011 3:48 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang;
> Abhishek Kane (Mindtree Consulting PVT LTD); Hank Janssen
> Subject: Re: [PATCH 38/40] Staging: hv: storvsc: Fixup srb_status for INQUIRY and
> MODE_SENSE
>
> On Wed, Jun 29, 2011 at 07:39:35AM -0700, K. Y. Srinivasan wrote:
> > The current handler on the Windows Host does not correctly handle
> > INQUIRY and MODE_SENSE commands with some options. Fixup srb_status
> > in these cases since the failure is not fatal.
>
> > + /*
> > + * The current SCSI handling on the host side does
> > + * not correctly handle:
> > + * INQUIRY command with page code parameter set to 0x80
> > + * MODE_SENSE command with cmd[2] == 0x1c
> > + *
> > + * Setup srb status so this won't be fatal.
> > + */
> > +
> > + if ((stor_pkt->vm_srb.cdb[0] == INQUIRY) ||
> > + (stor_pkt->vm_srb.cdb[0] == MODE_SENSE))
> > + vstor_packet->vm_srb.srb_status = 0;
>
> Given that the srb_status is only used for debug printks I don't
> quite see the point. If people explicitly turn on debugging they
> should see that these commands fail, shouldn't they?
>
The reason I did this was so that I could key off on real failures indicated by
srb_status == 0x4 to off-line the device.
Regards,
K. Y
^ permalink raw reply
* Re: [PATCH 38/40] Staging: hv: storvsc: Fixup srb_status for INQUIRY and MODE_SENSE
From: Christoph Hellwig @ 2011-06-30 19:47 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang,
Abhishek Kane, Hank Janssen
In-Reply-To: <1309358377-8537-38-git-send-email-kys@microsoft.com>
On Wed, Jun 29, 2011 at 07:39:35AM -0700, K. Y. Srinivasan wrote:
> The current handler on the Windows Host does not correctly handle
> INQUIRY and MODE_SENSE commands with some options. Fixup srb_status
> in these cases since the failure is not fatal.
> + /*
> + * The current SCSI handling on the host side does
> + * not correctly handle:
> + * INQUIRY command with page code parameter set to 0x80
> + * MODE_SENSE command with cmd[2] == 0x1c
> + *
> + * Setup srb status so this won't be fatal.
> + */
> +
> + if ((stor_pkt->vm_srb.cdb[0] == INQUIRY) ||
> + (stor_pkt->vm_srb.cdb[0] == MODE_SENSE))
> + vstor_packet->vm_srb.srb_status = 0;
Given that the srb_status is only used for debug printks I don't
quite see the point. If people explicitly turn on debugging they
should see that these commands fail, shouldn't they?
^ permalink raw reply
* Re: [PATCH 35/40] Staging: hv: storvsc: Make storvsc_dev_add() a static function
From: Christoph Hellwig @ 2011-06-30 19:45 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang,
Abhishek Kane, Hank Janssen
In-Reply-To: <1309358377-8537-35-git-send-email-kys@microsoft.com>
On Wed, Jun 29, 2011 at 07:39:32AM -0700, K. Y. Srinivasan wrote:
> Make storvsc_dev_add() a static function.
Making all pending functions static should be fine in a single patch.
^ permalink raw reply
* Re: [PATCH 34/40] Staging: hv: storvsc: Add the contents of hyperv_storage.h to storvsc_drv.c
From: Christoph Hellwig @ 2011-06-30 19:45 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang,
Abhishek Kane, Hank Janssen
In-Reply-To: <1309358377-8537-34-git-send-email-kys@microsoft.com>
On Wed, Jun 29, 2011 at 07:39:31AM -0700, K. Y. Srinivasan wrote:
> Add the contents of hyperv_storage.h to storvsc_drv.c and cleanup storvsc_drv.c.n
I'd at least leave the first half of the header that defines the
protocol around.
^ permalink raw reply
* Re: [PATCH 25/40] Staging: hv: storvsc: Add state to manage the lifecycle of emulated HBA
From: Christoph Hellwig @ 2011-06-30 19:44 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang,
Abhishek Kane, Hank Janssen
In-Reply-To: <1309358377-8537-25-git-send-email-kys@microsoft.com>
I think all this would be a lot simpler if you simply used one scsi
host per ide device as mentioned before.
Also it would be nice if you could sent patches 23 to 28 as one patch
in the next round, as that allows reviewing the whole IDE related code
in one go, which is useful given that it's all interwinded.
^ permalink raw reply
* Re: [PATCH 24/40] Staging: hv: storvsc: On I/O get the correct IDE device
From: Christoph Hellwig @ 2011-06-30 19:40 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: Abhishek Kane, Haiyang Zhang, gregkh, linux-kernel,
virtualization, devel
In-Reply-To: <1309358377-8537-24-git-send-email-kys@microsoft.com>
On Wed, Jun 29, 2011 at 07:39:21AM -0700, K. Y. Srinivasan wrote:
> We use the channel number to distinguish an IDE device managed by the
> storvsc driver from scsi devices. Add code to get the correct
> device pointer based on the channel number.
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
> Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> ---
> drivers/staging/hv/storvsc_drv.c | 10 ++++++++++
> 1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/staging/hv/storvsc_drv.c b/drivers/staging/hv/storvsc_drv.c
> index cf659d7..fcc3f5d 100644
> --- a/drivers/staging/hv/storvsc_drv.c
> +++ b/drivers/staging/hv/storvsc_drv.c
> @@ -517,6 +517,16 @@ static int storvsc_queuecommand_lck(struct scsi_cmnd *scmnd,
> unsigned int sg_count = 0;
> struct vmscsi_request *vm_srb;
>
> + if (scmnd->device->channel >= HV_IDE_BASE_CHANNEL) {
> + int channel = scmnd->device->channel;
> +
> + /*
> + * This is an IDE device; get the right dev.
> + */
> +
> + dev = ide_devices[channel - HV_IDE_BASE_CHANNEL];
> + }
So instead of playing games about getting the right hv_device here,
why don't you register one scsi host for each IDE device? libata
does the same for real ATA devices.
^ permalink raw reply
* Re: [PATCH 23/40] Staging: hv: storvsc: Introduce code to manage IDE devices using storvsc HBA
From: Christoph Hellwig @ 2011-06-30 19:38 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang,
Abhishek Kane, Hank Janssen
In-Reply-To: <1309358377-8537-23-git-send-email-kys@microsoft.com>
> +/*
> + * We want to manage the IDE devices using standard Linux SCSI drivers
> + * using the storvsc driver.
> + * Define special channels to support this.
> + */
> +
> +#define HV_MAX_IDE_DEVICES 4
> +#define HV_IDE_BASE_CHANNEL 10
> +#define HV_IDE0_DEV1 HV_IDE_BASE_CHANNEL
> +#define HV_IDE0_DEV2 (HV_IDE_BASE_CHANNEL + 1)
> +#define HV_IDE1_DEV1 (HV_IDE_BASE_CHANNEL + 2)
> +#define HV_IDE1_DEV2 (HV_IDE_BASE_CHANNEL + 3)
This at last needs a good explanation of why these devices are called
IDE if they actually aren't. I know you've explained the reason to me
before, but it should also be in the code.
The HV_IDE1_DEVn defines don't seem to useful to me. They are just
used in one place, and doing an opencoded HV_IDE_BASE_CHANNEL +
channel_nr would seem a lot easier to understand to me.
> +static struct Scsi_Host *storvsc_host;
> +
> +/*
> + * State to manage IDE devices that register with the storvsc driver.
> + *
> + */
> +static struct hv_device *ide_devices[HV_MAX_IDE_DEVICES];
> +
> +static void storvsc_get_ide_info(struct hv_device *dev, int *target, int *path)
> +{
> + *target =
> + dev->dev_instance.data[5] << 8 | dev->dev_instance.data[4];
> +
> + *path =
> + dev->dev_instance.data[3] << 24 | dev->dev_instance.data[2] << 16 |
> + dev->dev_instance.data[1] << 8 | dev->dev_instance.data[0];
Pretty odd formatting, I'd rather do it as:
*target =
dev->dev_instance.data[5] << 8 |
dev->dev_instance.data[4];
but more importanly what does path actually stand for here? Opencoding
this into the caller and adding proper comments explaining the scheme
might be more readable.
> @@ -469,7 +517,6 @@ static int storvsc_queuecommand_lck(struct scsi_cmnd *scmnd,
> unsigned int sg_count = 0;
> struct vmscsi_request *vm_srb;
>
> -
> /* If retrying, no need to prep the cmd */
> if (scmnd->host_scribble) {
>
> @@ -707,7 +754,6 @@ static int storvsc_probe(struct hv_device *device)
> scsi_host_put(host);
> return -ENODEV;
> }
> -
> scsi_scan_host(host);
> return ret;
Completely unrelated whitespace changes.
^ permalink raw reply
* Re: [PATCH 00/40] Staging: hv: Driver cleanup
From: Christoph Hellwig @ 2011-06-30 19:33 UTC (permalink / raw)
To: K. Y. Srinivasan; +Cc: gregkh, linux-kernel, devel, virtualization
In-Reply-To: <1309358301-8488-1-git-send-email-kys@microsoft.com>
On Wed, Jun 29, 2011 at 07:38:21AM -0700, K. Y. Srinivasan wrote:
> Further cleanup of the hv drivers:
>
> 1) Cleanup the reference counting mess for both stor and net devices.
I really don't understand the need for reference counting on the storage
side, especially now that you only have a SCSI driver. The SCSI
midlayer does proper counting on it's objects (Scsi_Host, scsi_device,
scsi_cmnd), so you'll get that for free given that SCSI drivers just
piggyback on the midlayer lifetime rules.
For now your patches should probably go in as-is, but mid-term you
should be able to completely remove that code on the storage side.
^ permalink raw reply
* Re: [PATCH 4/4] xen/netback: Add module alias for autoloading
From: David Miller @ 2011-06-30 18:19 UTC (permalink / raw)
To: konrad.wilk; +Cc: axboe, xen-devel, waldi, netdev, linux-kernel, virtualization
In-Reply-To: <20110630163954.GE5771@dumpdata.com>
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Thu, 30 Jun 2011 12:39:54 -0400
> On Wed, Jun 29, 2011 at 02:41:32PM +0200, Bastian Blank wrote:
>> Add xen-backend:vif module alias to the xen-netback module. This allows
>> automatic loading of the module.
>
> Dave,
>
> Could you queue this up for 3.1 please? I've the other two patches in my
> tree for 3.1 and the block patch ready for Jens.
Done.
^ permalink raw reply
* Re: [PATCH 4/4] xen/netback: Add module alias for autoloading
From: Konrad Rzeszutek Wilk @ 2011-06-30 16:39 UTC (permalink / raw)
To: Bastian Blank, xen-devel, virtualization, Jens Axboe,
linux-kernel
In-Reply-To: <20110629124132.GD31038@wavehammer.waldi.eu.org>
On Wed, Jun 29, 2011 at 02:41:32PM +0200, Bastian Blank wrote:
> Add xen-backend:vif module alias to the xen-netback module. This allows
> automatic loading of the module.
Dave,
Could you queue this up for 3.1 please? I've the other two patches in my
tree for 3.1 and the block patch ready for Jens.
>
> Signed-off-by: Bastian Blank <waldi@debian.org>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> drivers/net/xen-netback/netback.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index 0e4851b..fd00f25 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -1743,3 +1743,4 @@ failed_init:
> module_init(netback_init);
>
> MODULE_LICENSE("Dual BSD/GPL");
> +MODULE_ALIAS("xen-backend:vif");
> --
> 1.7.5.4
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* RE: [PATCH 1/1] Staging: hv: Integrate the time source driver with Hyper-V detection code
From: KY Srinivasan @ 2011-06-30 0:42 UTC (permalink / raw)
To: Greg KH
Cc: linux-kernel@vger.kernel.org, devel@linuxdriverproject.org,
virtualization@lists.osdl.org, Haiyang Zhang
In-Reply-To: <20110629190642.GA19698@suse.de>
> -----Original Message-----
> From: Greg KH [mailto:gregkh@suse.de]
> Sent: Wednesday, June 29, 2011 3:07 PM
> To: KY Srinivasan
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> virtualization@lists.osdl.org; Haiyang Zhang
> Subject: Re: [PATCH 1/1] Staging: hv: Integrate the time source driver with Hyper-
> V detection code
>
> On Wed, Jun 29, 2011 at 12:06:04PM -0700, K. Y. Srinivasan wrote:
> > The Hyper-V timesource driver is best integrated with Hyper-V detection code
> > since: (a) Linux guests running on Hyper-V need this timesource and (b)
> > by integrating with Hyper-V detection, we could minimize the code in the
> > timesource driver. Greg, you had helped me initially in getting the Hyper-V
> > detection code into the kernel (mshyperv.c). I am hoping you can take
> > this patch.
>
> Nope, it needs to go through the x86 maintainers, please resend it to
> them. The scripts/get_maintainer.pl file should have told you this,
> right?
Thanks Greg. I have done what you have recommended.
Regards,
K. Y
^ permalink raw reply
* Re: [PATCH 1/1] Staging: hv: Integrate the time source driver with Hyper-V detection code
From: Greg KH @ 2011-06-29 19:06 UTC (permalink / raw)
To: K. Y. Srinivasan; +Cc: devel, Haiyang Zhang, linux-kernel, virtualization
In-Reply-To: <1309374364-23528-1-git-send-email-kys@microsoft.com>
On Wed, Jun 29, 2011 at 12:06:04PM -0700, K. Y. Srinivasan wrote:
> The Hyper-V timesource driver is best integrated with Hyper-V detection code
> since: (a) Linux guests running on Hyper-V need this timesource and (b)
> by integrating with Hyper-V detection, we could minimize the code in the
> timesource driver. Greg, you had helped me initially in getting the Hyper-V
> detection code into the kernel (mshyperv.c). I am hoping you can take
> this patch.
Nope, it needs to go through the x86 maintainers, please resend it to
them. The scripts/get_maintainer.pl file should have told you this,
right?
thanks,
greg k-h
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox