RE: [PATCH 06/59] Staging: hv: blkvsc: Use the newly introducedvmbus ID in the blockvsc driver

[KY Srinivasan]

> -----Original Message-----
> From: Greg KH [mailto:greg@xxxxxxxxx]
> Sent: Thursday, August 25, 2011 1:36 PM
> To: KY Srinivasan
> Cc: gregkh@xxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> devel@xxxxxxxxxxxxxxxxxxxxxx; virtualization@xxxxxxxxxxxxxx; Haiyang Zhang
> Subject: Re: [PATCH 06/59] Staging: hv: blkvsc: Use the newly introduced vmbus
> ID in the blockvsc driver
> 
> On Thu, Aug 25, 2011 at 09:48:32AM -0700, K. Y. Srinivasan wrote:
> > Use the newly introduced vmbus ID in the blockvsc driver. Also, do
> > the associated cleanup.
> >
> > Signed-off-by: K. Y. Srinivasan <kys@xxxxxxxxxxxxx>
> > Signed-off-by: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>
> > ---
> >  drivers/staging/hv/blkvsc_drv.c |   23 ++++++++++++++---------
> >  1 files changed, 14 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/staging/hv/blkvsc_drv.c b/drivers/staging/hv/blkvsc_drv.c
> > index 018b293..9b99387 100644
> > --- a/drivers/staging/hv/blkvsc_drv.c
> > +++ b/drivers/staging/hv/blkvsc_drv.c
> > @@ -111,14 +111,6 @@ struct block_device_context {
> >
> >  static const char *drv_name = "blkvsc";
> >
> > -/* {32412632-86cb-44a2-9b5c-50d1417354f5} */
> > -static const uuid_le dev_type = {
> > -	.b = {
> > -		0x32, 0x26, 0x41, 0x32, 0xcb, 0x86, 0xa2, 0x44,
> > -		0x9b, 0x5c, 0x50, 0xd1, 0x41, 0x73, 0x54, 0xf5
> > -	}
> > -};
> > -
> >  /*
> >   * There is a circular dependency involving blkvsc_request_completion()
> >   * and blkvsc_do_request().
> > @@ -802,10 +794,24 @@ static void blkvsc_request(struct request_queue
> *queue)
> >  	}
> >  }
> >
> > +static const struct hv_vmbus_device_id id_table[] = {
> > +	{
> > +		/* IDE guid */
> > +		.guid = {
> > +			0x32, 0x26, 0x41, 0x32, 0xcb, 0x86, 0xa2, 0x44,
> > +			0x9b, 0x5c, 0x50, 0xd1, 0x41, 0x73, 0x54, 0xf5
> > +		}
> > +	},
> > +	{
> > +		.guid = { }
> > +	},
> > +};
> 
> Ah, so close, and yet, not quite :(
> 
> This should look like:
> 
> static const struct hv_vmbus_device_id id_table[] = {
> 	{
> 		/* IDE guid */
> 		.guid = {
> 			0x32, 0x26, 0x41, 0x32, 0xcb, 0x86, 0xa2, 0x44,
> 			0x9b, 0x5c, 0x50, 0xd1, 0x41, 0x73, 0x54, 0xf5
> 		}
> 	},
> 	{ },
> };
> 
> No need to set the field name to nothing.
> 
> Hm, this could all be made even smaller with something like the
> USB_DEVICE() macro.  How about this instead:
> 
> /* IDE guid */
> static const struct hv_vmbus_device_id id_table[] = {
> 	{ VMBUS_DEVICE(0x32, 0x26, 0x41, 0x32, 0xcb, 0x86, 0xa2, 0x44,
> 		       0x9b, 0x5c, 0x50, 0xd1, 0x41, 0x73, 0x54, 0xf5) },
> 	{ },
> };
> 
> That would be easier to read and understand, right?  Half the number of
> lines as well, which is always nice.
> 
> If so, I'll go make that change here so you don't have to respin these.

Looks great Greg. So you are going to add the VMBUS_DEVICE() macro?
Thanks for doing this.

Regards,

K. Y

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/