[Qemu-devel] Re: [PATCH 05/14] ide: Remove redundant IDEState member conf

[Kevin Wolf]

Am 28.05.2010 15:38, schrieb Markus Armbruster:
> Commit 428c149b added IDEState member conf to let commit 0009baf1 find
> the BlockConf from there.  It exists only for qdev drives, created via
> ide_drive_initfn(), not for drives created via ide_init2().
> 
> But for a qdev drive, we can just as well reach its IDEDevice, which
> contains the BlockConf.  Do that, and revert the parts of commit
> 428c149b that add IDEState member conf.
> 
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>  hw/ide/core.c     |   16 +++++++---------
>  hw/ide/internal.h |    4 +---
>  hw/ide/qdev.c     |    3 +--
>  3 files changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 066fecb..c3334b1 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -98,6 +98,7 @@ static void ide_identify(IDEState *s)
>  {
>      uint16_t *p;
>      unsigned int oldsize;
> +    IDEDevice *dev;
>  
>      if (s->identify_set) {
>       memcpy(s->io_buffer, s->identify_data, sizeof(s->identify_data));
> @@ -165,8 +166,9 @@ static void ide_identify(IDEState *s)
>      put_le16(p + 101, s->nb_sectors >> 16);
>      put_le16(p + 102, s->nb_sectors >> 32);
>      put_le16(p + 103, s->nb_sectors >> 48);
> -    if (s->conf && s->conf->physical_block_size)
> -        put_le16(p + 106, 0x6000 | get_physical_block_exp(s->conf));
> +    dev = s->unit ? s->bus->slave : s->bus->master;
> +    if (dev && dev->conf.physical_block_size)

If this is meant to be the same condition it would be dev->conf && ...

I can't really tell right now if it can happen that either dev or
dev->conf can be NULL here. In a quick attempt I couldn't get either one.

So, which version is correct, or should the first part be dropped
completely?

Kevin