qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 08/23] block: Eliminate BlockDriverState memb


From: Benoît Canet
Subject: Re: [Qemu-devel] [PATCH v2 08/23] block: Eliminate BlockDriverState member device_name[]
Date: Tue, 16 Sep 2014 13:18:31 +0000
User-agent: Mutt/1.5.21 (2010-09-15)


>  #include "qemu-common.h"
> -#include "block/block_int.h"
> +#include "block/block.h"
> +#include "qemu/error-report.h"
> +#include "qemu/main-loop.h"

That's a lot of include fiddling I am not sure to understand them while looking
at the following diff.

>  #include "hw/hw.h"
>  #include "qemu/queue.h"
>  #include "qemu/timer.h"
> @@ -130,9 +132,9 @@ static void blk_send(QEMUFile *f, BlkMigBlock * blk)
>                       | flags);
>  
>      /* device name */
> -    len = strlen(blk->bmds->bs->device_name);
> +    len = strlen(bdrv_get_device_name(blk->bmds->bs));
>      qemu_put_byte(f, len);
> -    qemu_put_buffer(f, (uint8_t *)blk->bmds->bs->device_name, len);
> +    qemu_put_buffer(f, (uint8_t *)bdrv_get_device_name(blk->bmds->bs), len);
>  
>      /* if a block is zero we need to flush here since the network
>       * bandwidth is now a lot higher than the storage device bandwidth.
> @@ -382,9 +384,9 @@ static void init_blk_migration(QEMUFile *f)
>  
>          if (bmds->shared_base) {
>              DPRINTF("Start migration for %s with shared base image\n",
> -                    bs->device_name);
> +                    bdrv_get_device_name(bs));
>          } else {
> -            DPRINTF("Start full migration for %s\n", bs->device_name);
> +            DPRINTF("Start full migration for %s\n", 
> bdrv_get_device_name(bs));
>          }
>  
>          QSIMPLEQ_INSERT_TAIL(&block_mig_state.bmds_list, bmds, entry);


> @@ -1959,10 +1941,10 @@ void bdrv_drain_all(void)
>   V  Also, NULL terminate the device_name to prevent double remove */
>  void bdrv_make_anon(BlockDriverState *bs)
>  {
> -    if (bs->device_name[0] != '\0') {


> +    if (bs->device_list.tqe_prev) {
>          QTAILQ_REMOVE(&bdrv_states, bs, device_list);
> +        bs->device_list.tqe_prev = NULL;

I think a comments explaining the trick you are doing here would be worthy:
after all you are touching directly the inner parts of a linked list and
bypassing the list API.


>      }
> -    bs->device_name[0] = '\0';
>      if (bs->node_name[0] != '\0') {
>          QTAILQ_REMOVE(&graph_bdrv_states, bs, node_list);
>      }



reply via email to

[Prev in Thread] Current Thread [Next in Thread]