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