Re: [Qemu-devel] [PATCH 2/2] block: Convert bdrv_first to QTAILQ

[Kevin Wolf <kwolf@redhat.com>]

Am 09.04.2010 16:22, schrieb Stefan Hajnoczi:
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> ---
>  block.c     |   44 +++++++++++++++++++++++---------------------
>  block_int.h |    3 ++-
>  2 files changed, 25 insertions(+), 22 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 61da183..86d2504 100644
> --- a/block.c
> +++ b/block.c
> @@ -55,7 +55,8 @@ static int bdrv_read_em(BlockDriverState *bs, int64_t sector_num,
>  static int bdrv_write_em(BlockDriverState *bs, int64_t sector_num,
>                           const uint8_t *buf, int nb_sectors);
>  
> -static BlockDriverState *bdrv_first;
> +static QTAILQ_HEAD(, BlockDriverState) bdrv_states =
> +    QTAILQ_HEAD_INITIALIZER(bdrv_states);
>  
>  static BlockDriver *first_drv;
>  
> @@ -148,16 +149,12 @@ void bdrv_register(BlockDriver *bdrv)
>  /* create a new block device (by default it is empty) */
>  BlockDriverState *bdrv_new(const char *device_name)
>  {
> -    BlockDriverState **pbs, *bs;
> +    BlockDriverState *bs;
>  
>      bs = qemu_mallocz(sizeof(BlockDriverState));
>      pstrcpy(bs->device_name, sizeof(bs->device_name), device_name);
>      if (device_name[0] != '\0') {
> -        /* insert at the end */
> -        pbs = &bdrv_first;
> -        while (*pbs != NULL)
> -            pbs = &(*pbs)->next;
> -        *pbs = bs;
> +        QTAILQ_INSERT_TAIL(&bdrv_states, bs, list);
>      }
>      return bs;
>  }
> @@ -545,13 +542,15 @@ void bdrv_close(BlockDriverState *bs)
>  
>  void bdrv_delete(BlockDriverState *bs)
>  {
> -    BlockDriverState **pbs;
> +    BlockDriverState *bs1;
>  
> -    pbs = &bdrv_first;
> -    while (*pbs != bs && *pbs != NULL)
> -        pbs = &(*pbs)->next;
> -    if (*pbs == bs)
> -        *pbs = bs->next;
> +    /* remove from list, if necessary */
> +    QTAILQ_FOREACH(bs1, &bdrv_states, list) {
> +        if (bs1 == bs) {
> +            QTAILQ_REMOVE(&bdrv_states, bs, list);
> +            break;
> +        }
> +    }

This loop looks strange, what is it used for? We had only a next pointer
previously, so we needed to search the element. A QTAILQ has both prev
and next pointers though, so you should be able to directly use
QTAILQ_REMOVE.

The rest of it looks good.

Kevin