Re: [Qemu-block] [PATCH] m25p80: provide a realize to support late inits.

[Paolo Bonzini]

On 15/06/2016 16:00, Cédric Le Goater wrote:
> We also need to realize() the SSISlave part of the object. This is why
> the previous realize() ops is stored in M25P80Class and called in the
> object realize() ops.
> 
> This is fully compatible with the existing users of m25p80 and it
> provides a way to handle errors on the drive backend.
> 
> Signed-off-by: Cédric Le Goater <address@hidden>

I think you should instead:

1) change hw/ssi/ssi.c's ssi_slave_init to be an override of dc->realize

2) change SSISlaveClass's init member to a realize function

Thanks,

Paolo

> ---
> 
>  Should apply on top of :
>  
>     m25p80: fix test on blk_pread() return value
>     https://lists.gnu.org/archive/html/qemu-devel/2016-05/msg05574.html
> 
>  hw/block/m25p80.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 51d85960566f..c47722d3a3e5 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -28,6 +28,7 @@
>  #include "hw/ssi/ssi.h"
>  #include "qemu/bitops.h"
>  #include "qemu/log.h"
> +#include "qapi/error.h"
>  
>  #ifndef M25P80_ERR_DEBUG
>  #define M25P80_ERR_DEBUG 0
> @@ -339,6 +340,7 @@ typedef struct Flash {
>  
>  typedef struct M25P80Class {
>      SSISlaveClass parent_class;
> +    DeviceRealize parent_dc_realize;
>      FlashPartInfo *pi;
>  } M25P80Class;
>  
> @@ -880,7 +882,6 @@ static uint32_t m25p80_transfer8(SSISlave *ss, uint32_t 
> tx)
>  
>  static int m25p80_init(SSISlave *ss)
>  {
> -    DriveInfo *dinfo;
>      Flash *s = M25P80(ss);
>      M25P80Class *mc = M25P80_GET_CLASS(s);
>  
> @@ -888,8 +889,18 @@ static int m25p80_init(SSISlave *ss)
>  
>      s->size = s->pi->sector_size * s->pi->n_sectors;
>      s->dirty_page = -1;
> +    return 0;
> +}
> +
> +static void m25p80_realize(DeviceState *dev, Error **errp)
> +{
> +    Flash *s = M25P80(dev);
> +    M25P80Class *mc = M25P80_GET_CLASS(s);
> +    DriveInfo *dinfo;
> +
> +    /* initialize the SSISlave part */
> +    mc->parent_dc_realize(dev, errp);
>  
> -    /* FIXME use a qdev drive property instead of drive_get_next() */
>      dinfo = drive_get_next(IF_MTD);
>  
>      if (dinfo) {
> @@ -899,18 +910,15 @@ static int m25p80_init(SSISlave *ss)
>  
>          s->storage = blk_blockalign(s->blk, s->size);
>  
> -        /* FIXME: Move to late init */
>          if (blk_pread(s->blk, 0, s->storage, s->size) != s->size) {
> -            fprintf(stderr, "Failed to initialize SPI flash!\n");
> -            return 1;
> +            error_setg(errp, "failed to read the initial flash content");
> +            return;
>          }
>      } else {
>          DB_PRINT_L(0, "No BDRV - binding to RAM\n");
>          s->storage = blk_blockalign(NULL, s->size);
>          memset(s->storage, 0xFF, s->size);
>      }
> -
> -    return 0;
>  }
>  
>  static void m25p80_reset(DeviceState *d)
> @@ -967,6 +975,8 @@ static void m25p80_class_init(ObjectClass *klass, void 
> *data)
>      dc->vmsd = &vmstate_m25p80;
>      dc->props = m25p80_properties;
>      dc->reset = m25p80_reset;
> +    mc->parent_dc_realize = dc->realize;
> +    dc->realize = m25p80_realize;
>      mc->pi = data;
>  }
>  
>