Re: [Qemu-devel] [PATCH 3/3] ide: Adds wwn=hex qdev option allowing the user to specify a disk's World Wide Name

[Floris Bos / Maxnet]

On 03/13/2012 01:06 PM, Kevin Wolf wrote:
> Am 13.03.2012 11:29, schrieb Stefan Hajnoczi:
> > On Mon, Mar 12, 2012 at 8:05 PM, Floris Bos<address@hidden>  wrote:
> > > diff --git a/hw/ide/core.c b/hw/ide/core.c
> > > index 3e50c52..b48e5c2 100644
> > > --- a/hw/ide/core.c
> > > +++ b/hw/ide/core.c
> > > @@ -166,6 +166,13 @@ static void ide_identify(IDEState *s)
> > >      if (dev&&  dev->conf.discard_granularity) {
> > >          put_le16(p + 169, 1); /* TRIM support */
> > >      }
> > > +
> > > +    if (s->wwn) {
> > > +        put_le16(p + 108, s->wwn>>  48);
> > > +        put_le16(p + 109, s->wwn>>  32);
> > > +        put_le16(p + 110, s->wwn>>  16);
> > > +        put_le16(p + 111, s->wwn);
> > > +    }
> > Little-endian 16-bit seems weird at first but the spec requires it, so
> > it's fine.  A comment would be nice.
> >
> > ATA8-ACS says:
> >
> > "Bit 8 of word 84 shall be set to one indicating the mandatory World
> > Wide Name in words 108-111 is supported."
> >
> > I think we're missing this.
> And this:
>
> "Bit 8 of word 87 is a copy of word 84 bit 8."
>
> Otherwise the series looks good to me.
>
> Kevin

Oops, you are right, overlooked those bits.
Linux only seems to care that the WWN starts with a 5, and doesn't look 
at the bits, so it didn't turn up in my testing either.
Will send a fixed patch.

Yours sincerely,

Floris Bos