Re: virtio scsi host draft specification, v3
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 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH 00/40] Staging: hv: Driver cleanup
On Fri, 1 Jul 2011 00:19:38 + KY Srinivasan wrote: > > > > -Original Message- > > From: Stephen Hemminger [mailto:shemmin...@vyatta.com] > > Sent: Thursday, June 30, 2011 7:48 PM > > To: KY Srinivasan > > Cc: Christoph Hellwig; de...@linuxdriverproject.org; gre...@suse.de; linux- > > ker...@vger.kernel.org; virtualizat...@lists.osdl.org > > Subject: Re: [PATCH 00/40] Staging: hv: Driver cleanup > > > > On Thu, 30 Jun 2011 23:32:34 + > > KY Srinivasan wrote: > > > > > > > > > -Original Message- > > > > From: Christoph Hellwig [mailto:h...@infradead.org] > > > > Sent: Thursday, June 30, 2011 3:34 PM > > > > To: KY Srinivasan > > > > Cc: gre...@suse.de; linux-ker...@vger.kernel.org; > > > > de...@linuxdriverproject.org; virtualizat...@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. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH 7/7] [v6] drivers/virt: introduce Freescale hypervisor management driver
On Thu, Jun 9, 2011 at 4:04 PM, Arnd Bergmann 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 > > Acked-by: Arnd Bergmann 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 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
RE: [PATCH 00/40] Staging: hv: Driver cleanup
> -Original Message- > From: Stephen Hemminger [mailto:shemmin...@vyatta.com] > Sent: Thursday, June 30, 2011 7:48 PM > To: KY Srinivasan > Cc: Christoph Hellwig; de...@linuxdriverproject.org; gre...@suse.de; linux- > ker...@vger.kernel.org; virtualizat...@lists.osdl.org > Subject: Re: [PATCH 00/40] Staging: hv: Driver cleanup > > On Thu, 30 Jun 2011 23:32:34 + > KY Srinivasan wrote: > > > > > > -Original Message- > > > From: Christoph Hellwig [mailto:h...@infradead.org] > > > Sent: Thursday, June 30, 2011 3:34 PM > > > To: KY Srinivasan > > > Cc: gre...@suse.de; linux-ker...@vger.kernel.org; > > > de...@linuxdriverproject.org; virtualizat...@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 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH 00/40] Staging: hv: Driver cleanup
On Thu, 30 Jun 2011 23:32:34 + KY Srinivasan wrote: > > > -Original Message- > > From: Christoph Hellwig [mailto:h...@infradead.org] > > Sent: Thursday, June 30, 2011 3:34 PM > > To: KY Srinivasan > > Cc: gre...@suse.de; linux-ker...@vger.kernel.org; > > de...@linuxdriverproject.org; virtualizat...@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. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
RE: [PATCH 00/40] Staging: hv: Driver cleanup
> -Original Message- > From: Christoph Hellwig [mailto:h...@infradead.org] > Sent: Thursday, June 30, 2011 3:34 PM > To: KY Srinivasan > Cc: gre...@suse.de; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; virtualizat...@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 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
RE: [PATCH 00/40] Staging: hv: Driver cleanup
> -Original Message- > From: Christoph Hellwig [mailto:h...@infradead.org] > Sent: Thursday, June 30, 2011 3:34 PM > To: KY Srinivasan > Cc: gre...@suse.de; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; virtualizat...@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 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
RE: [PATCH 23/40] Staging: hv: storvsc: Introduce code to manage IDE devices using storvsc HBA
> -Original Message-
> From: Christoph Hellwig [mailto:h...@infradead.org]
> Sent: Thursday, June 30, 2011 3:39 PM
> To: KY Srinivasan
> Cc: gre...@suse.de; linux-ker...@vger.kernel.org;
> de...@linuxdriverproject.org; virtualizat...@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_CHANNEL10
> > +#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
.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
RE: [PATCH 24/40] Staging: hv: storvsc: On I/O get the correct IDE device
> -Original Message-
> From: Christoph Hellwig [mailto:h...@infradead.org]
> Sent: Thursday, June 30, 2011 3:41 PM
> To: KY Srinivasan
> Cc: gre...@suse.de; linux-ker...@vger.kernel.org;
> de...@linuxdriverproject.org; virtualizat...@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
> > Signed-off-by: Haiyang Zhang
> > Signed-off-by: Abhishek Kane
> > Signed-off-by: Hank Janssen
> > ---
> > 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
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
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? ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
RE: [PATCH 34/40] Staging: hv: storvsc: Add the contents of hyperv_storage.h to storvsc_drv.c
> -Original Message- > From: Christoph Hellwig [mailto:h...@infradead.org] > Sent: Thursday, June 30, 2011 3:46 PM > To: KY Srinivasan > Cc: gre...@suse.de; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; virtualizat...@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 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
RE: [PATCH 38/40] Staging: hv: storvsc: Fixup srb_status for INQUIRY and MODE_SENSE
> -Original Message- > From: Christoph Hellwig [mailto:h...@infradead.org] > Sent: Thursday, June 30, 2011 3:48 PM > To: KY Srinivasan > Cc: gre...@suse.de; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; virtualizat...@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 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
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. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH 35/40] Staging: hv: storvsc: Make storvsc_dev_add() a static function
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. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH 25/40] Staging: hv: storvsc: Add state to manage the lifecycle of emulated HBA
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. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
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
> Signed-off-by: Haiyang Zhang
> Signed-off-by: Abhishek Kane
> Signed-off-by: Hank Janssen
> ---
> 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.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
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.
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.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
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. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH 4/4] xen/netback: Add module alias for autoloading
From: Konrad Rzeszutek Wilk 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. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/virtualization
Re: [PATCH 4/4] xen/netback: Add module alias for autoloading
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
> Acked-by: Ian Campbell
> Acked-by: Konrad Rzeszutek Wilk
> ---
> 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 majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
