Re: [PATCH 2/2] Add serial number support for virtio_blk, V4a

2009-06-09 Thread Rusty Russell
On Thu, 4 Jun 2009 06:37:19 am you wrote: > This patch extracts the opaque data from pci i/o > region 0 via the added VIRTIO_BLK_F_IDENTIFY > field. Thanks John, I fixed textual conflicts and forwarded it to Jens: he has the other pending virtio_blk patches as well. I removed one comment: > +/*

Re: [PATCH 2/2] Add serial number support for virtio_blk, V4a

2009-06-03 Thread john cooper
This patch extracts the opaque data from pci i/o region 0 via the added VIRTIO_BLK_F_IDENTIFY field. By convention this data takes the form of that returned by an ATA IDENTIFY DEVICE command, however the driver (except for structure size) makes no interpretation of the data. The structure data is

Re: [PATCH 2/2] Add serial number support for virtio_blk, V4

2009-06-03 Thread john cooper
Jens Axboe wrote: > On Mon, Jun 01 2009, Rusty Russell wrote: >> On Fri, 29 May 2009 01:45:27 pm john cooper wrote: >>> virtio_blk-serial-4.patch >> Hate to ask dumb questions, but is there a scsi equivalent of this? It'd be >> nice if we could avoid being ATA-specific in the long run... > > SCS

Re: [PATCH 2/2] Add serial number support for virtio_blk, V4

2009-06-03 Thread Jens Axboe
On Mon, Jun 01 2009, Rusty Russell wrote: > On Fri, 29 May 2009 01:45:27 pm john cooper wrote: > > virtio_blk-serial-4.patch > > Hate to ask dumb questions, but is there a scsi equivalent of this? It'd be > nice if we could avoid being ATA-specific in the long run... SCSI has mode pages, where

Re: [PATCH 2/2] Add serial number support for virtio_blk, V4

2009-06-01 Thread Rusty Russell
On Fri, 29 May 2009 01:45:27 pm john cooper wrote: > virtio_blk-serial-4.patch Hate to ask dumb questions, but is there a scsi equivalent of this? It'd be nice if we could avoid being ATA-specific in the long run... Also, why u16? Thanks, Rusty. > +/* return ATA identify data > + */ > +static

Re: [Qemu-devel] [PATCH 2/2] Add serial number support for virtio_blk, V4

2009-05-31 Thread Christoph Hellwig
On Fri, May 29, 2009 at 12:15:27AM -0400, john cooper wrote: > virtio_blk-serial-4.patch > > The patch looks good to me, but to get it applied to the kernel tree the patch needs a good description and we also need your Signed-off-by line (See Documentation/SubmittingPatches in the kernel source

[PATCH 2/2] Add serial number support for virtio_blk, V4

2009-05-28 Thread john cooper
virtio_blk-serial-4.patch drivers/block/virtio_blk.c | 41 ++--- include/linux/virtio_blk.h |7 +++ 2 files changed, 45 insertions(+), 3 deletions(-) = --- a/drivers/block/virtio_blk.c

Re: [Qemu-devel] [PATCH 2/2] Add serial number support for virtio_blk, V3

2009-05-28 Thread john cooper
>> +if (!(id = kzalloc(ATA_ID_WORDS, GFP_KERNEL))) >> +rv = -ENOMEM; > > Doesn't ATA_ID_WORDS seem like a strange name for a number of bytes? Yes I caught that bug in the rework as well. > What's this *for* BTW? Sorry -- I assumed you were on either list. Please see patch to fo

Re: [Qemu-devel] [PATCH 2/2] Add serial number support for virtio_blk, V3

2009-05-28 Thread john cooper
Christoph Hellwig wrote: > On Wed, May 27, 2009 at 09:49:19AM +0200, Christoph Hellwig wrote: >> /* >> * IDE-compatible identify ioctl. >> * >> * Currenlyt only returns the serial number and leaves all other fields >> * zero. >> */ > > Btw, thinking about it the rest of the information in the

Re: [Qemu-devel] [PATCH 2/2] Add serial number support for virtio_blk, V3

2009-05-27 Thread Rusty Russell
On Wed, 27 May 2009 05:19:19 pm Christoph Hellwig wrote: > You should probably include rusty as he's collecting the patches > for the virtio guest drivers. Yes. It *does* help to cc the maintainer if you want your patches applied :) And I particularly love untested code like this! > + if (!

Re: [Qemu-devel] [PATCH 2/2] Add serial number support for virtio_blk, V3

2009-05-27 Thread john cooper
Christoph Hellwig wrote: This looks functionally correct, but pretty far from normal kernel coding style. I tend to avoid 'goto's. Christoph Hellwig wrote: /* * IDE-compatible identify ioctl. * * Currenlyt only returns the serial number and leaves all other fields * zero. */ Btw, thin

Re: [Qemu-devel] [PATCH 2/2] Add serial number support for virtio_blk, V3

2009-05-27 Thread Christoph Hellwig
On Wed, May 27, 2009 at 09:49:19AM +0200, Christoph Hellwig wrote: > /* > * IDE-compatible identify ioctl. > * > * Currenlyt only returns the serial number and leaves all other fields > * zero. > */ Btw, thinking about it the rest of the information in the ioctl should probably be filled up w

Re: [Qemu-devel] [PATCH 2/2] Add serial number support for virtio_blk, V3

2009-05-27 Thread Christoph Hellwig
You should probably include rusty as he's collecting the patches for the virtio guest drivers. Also can you send the patch inline next time? That makes quoting it for review a lot easier. drivers/block/virtio_blk.c | 32 +--- include/linux/virtio_blk.h |6 +

[PATCH 2/2] Add serial number support for virtio_blk, V3

2009-05-26 Thread john cooper
-- john.coo...@redhat.com drivers/block/virtio_blk.c | 32 +--- include/linux/virtio_blk.h |6 ++ 2 files changed, 35 insertions(+), 3 deletions(-) = --- a/drivers/block/virtio_blk.c +++ b/driv

[PATCH 2/2] Add serial number support for virtio_blk, V2

2009-05-13 Thread john cooper
-- john.coo...@redhat.com drivers/block/virtio_blk.c | 35 --- include/linux/virtio_blk.h | 10 ++ 2 files changed, 42 insertions(+), 3 deletions(-) = --- a/drivers/block/virtio_blk.c +++

[PATCH 2/2] Add serial number support for virtio_blk

2009-04-29 Thread john cooper
-- john.coo...@third-harmonic.com drivers/block/virtio_blk.c | 36 +--- include/linux/virtio_blk.h | 10 ++ 2 files changed, 43 insertions(+), 3 deletions(-) = --- a/include/linux/virtio_b

[PATCH 2/2] Add serial number support for virtio_blk

2009-04-29 Thread john cooper
-- john.coo...@third-harmonic.com drivers/block/virtio_blk.c | 36 +--- include/linux/virtio_blk.h | 10 ++ 2 files changed, 43 insertions(+), 3 deletions(-) = --- a/include/linux/virtio_b