[Qemu-devel] Re: [PATCH 1/2] block/qcow2.c: rename qcow_ functions to qcow2_

[Kevin Wolf]

Am 16.12.2010 17:05, schrieb address@hidden:
> From: Jes Sorensen <address@hidden>
> 
> It doesn't really make sense for functions in qcow2.c to be named
> qcow_ so convert the names to match correctly.
> 
> Signed-off-by: Jes Sorensen <address@hidden>
> ---
>  block/qcow2-cluster.c  |    6 +-
>  block/qcow2-snapshot.c |    6 +-
>  block/qcow2.c          |  210 
> +++++++++++++++++++++++++-----------------------
>  3 files changed, 116 insertions(+), 106 deletions(-)
> 
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index b040208..6928c63 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -352,8 +352,8 @@ void qcow2_encrypt_sectors(BDRVQcowState *s, int64_t 
> sector_num,
>  }
>  
>  
> -static int qcow_read(BlockDriverState *bs, int64_t sector_num,
> -                     uint8_t *buf, int nb_sectors)
> +static int qcow2_read(BlockDriverState *bs, int64_t sector_num,
> +                      uint8_t *buf, int nb_sectors)
>  {
>      BDRVQcowState *s = bs->opaque;
>      int ret, index_in_cluster, n, n1;
> @@ -419,7 +419,7 @@ static int copy_sectors(BlockDriverState *bs, uint64_t 
> start_sect,
>      if (n <= 0)
>          return 0;
>      BLKDBG_EVENT(bs->file, BLKDBG_COW_READ);
> -    ret = qcow_read(bs, start_sect + n_start, s->cluster_data, n);
> +    ret = qcow2_read(bs, start_sect + n_start, s->cluster_data, n);
>      if (ret < 0)
>          return ret;
>      if (s->crypt_method) {
> diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
> index aacf357..74823a5 100644
> --- a/block/qcow2-snapshot.c
> +++ b/block/qcow2-snapshot.c
> @@ -116,7 +116,7 @@ int qcow2_read_snapshots(BlockDriverState *bs)
>  }
>  
>  /* add at the end of the file a new list of snapshots */
> -static int qcow_write_snapshots(BlockDriverState *bs)
> +static int qcow2_write_snapshots(BlockDriverState *bs)
>  {
>      BDRVQcowState *s = bs->opaque;
>      QCowSnapshot *sn;
> @@ -300,7 +300,7 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
> QEMUSnapshotInfo *sn_info)
>      s->snapshots = snapshots1;
>      s->snapshots[s->nb_snapshots++] = *sn;
>  
> -    if (qcow_write_snapshots(bs) < 0)
> +    if (qcow2_write_snapshots(bs) < 0)
>          goto fail;
>  #ifdef DEBUG_ALLOC
>      qcow2_check_refcounts(bs);
> @@ -378,7 +378,7 @@ int qcow2_snapshot_delete(BlockDriverState *bs, const 
> char *snapshot_id)
>      qemu_free(sn->name);
>      memmove(sn, sn + 1, (s->nb_snapshots - snapshot_index - 1) * 
> sizeof(*sn));
>      s->nb_snapshots--;
> -    ret = qcow_write_snapshots(bs);
> +    ret = qcow2_write_snapshots(bs);
>      if (ret < 0) {
>          /* XXX: restore snapshot if error ? */
>          return ret;
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 537c479..d7fd167 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -50,10 +50,10 @@ typedef struct {
>      uint32_t magic;
>      uint32_t len;
>  } QCowExtension;
> -#define  QCOW_EXT_MAGIC_END 0
> -#define  QCOW_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA
> +#define  QCOW2_EXT_MAGIC_END 0
> +#define  QCOW2_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA
>  
> -static int qcow_probe(const uint8_t *buf, int buf_size, const char *filename)
> +static int qcow2_probe(const uint8_t *buf, int buf_size, const char 
> *filename)
>  {
>      const QCowHeader *cow_header = (const void *)buf;
>  
> @@ -73,14 +73,14 @@ static int qcow_probe(const uint8_t *buf, int buf_size, 
> const char *filename)
>   * unknown magic is skipped (future extension this version knows nothing 
> about)
>   * return 0 upon success, non-0 otherwise
>   */
> -static int qcow_read_extensions(BlockDriverState *bs, uint64_t start_offset,
> -                                uint64_t end_offset)
> +static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
> +                                 uint64_t end_offset)
>  {
>      QCowExtension ext;
>      uint64_t offset;
>  
>  #ifdef DEBUG_EXT
> -    printf("qcow_read_extensions: start=%ld end=%ld\n", start_offset, 
> end_offset);
> +    printf("qcow2_read_extensions: start=%ld end=%ld\n", start_offset, 
> end_offset);
>  #endif
>      offset = start_offset;
>      while (offset < end_offset) {
> @@ -88,13 +88,13 @@ static int qcow_read_extensions(BlockDriverState *bs, 
> uint64_t start_offset,
>  #ifdef DEBUG_EXT
>          /* Sanity check */
>          if (offset > s->cluster_size)
> -            printf("qcow_handle_extension: suspicious offset %lu\n", offset);
> +            printf("qcow_read_extension: suspicious offset %lu\n", offset);

It's now qcow2_read_extensions

>  
>          printf("attemting to read extended header in offset %lu\n", offset);
>  #endif
>  
>          if (bdrv_pread(bs->file, offset, &ext, sizeof(ext)) != sizeof(ext)) {
> -            fprintf(stderr, "qcow_handle_extension: ERROR: "
> +            fprintf(stderr, "qcow_read_extension: ERROR: "

Same here.

>                      "pread fail from offset %" PRIu64 "\n",
>                      offset);
>              return 1;
> @@ -106,10 +106,10 @@ static int qcow_read_extensions(BlockDriverState *bs, 
> uint64_t start_offset,
>          printf("ext.magic = 0x%x\n", ext.magic);
>  #endif
>          switch (ext.magic) {
> -        case QCOW_EXT_MAGIC_END:
> +        case QCOW2_EXT_MAGIC_END:
>              return 0;
>  
> -        case QCOW_EXT_MAGIC_BACKING_FORMAT:
> +        case QCOW2_EXT_MAGIC_BACKING_FORMAT:
>              if (ext.len >= sizeof(bs->backing_format)) {
>                  fprintf(stderr, "ERROR: ext_backing_format: len=%u too large"
>                          " (>=%zu)\n",
> @@ -137,7 +137,7 @@ static int qcow_read_extensions(BlockDriverState *bs, 
> uint64_t start_offset,
>  }
>  
>  
> -static int qcow_open(BlockDriverState *bs, int flags)
> +static int qcow2_open(BlockDriverState *bs, int flags)
>  {
>      BDRVQcowState *s = bs->opaque;
>      int len, i;
> @@ -222,7 +222,7 @@ static int qcow_open(BlockDriverState *bs, int flags)
>          ext_end = header.backing_file_offset;
>      else
>          ext_end = s->cluster_size;
> -    if (qcow_read_extensions(bs, sizeof(header), ext_end))
> +    if (qcow2_read_extensions(bs, sizeof(header), ext_end))
>          goto fail;
>  
>      /* read the backing file name */
> @@ -252,7 +252,7 @@ static int qcow_open(BlockDriverState *bs, int flags)
>      return -1;
>  }
>  
> -static int qcow_set_key(BlockDriverState *bs, const char *key)
> +static int qcow2_set_key(BlockDriverState *bs, const char *key)
>  {
>      BDRVQcowState *s = bs->opaque;
>      uint8_t keybuf[16];
> @@ -294,8 +294,8 @@ static int qcow_set_key(BlockDriverState *bs, const char 
> *key)
>      return 0;
>  }
>  
> -static int qcow_is_allocated(BlockDriverState *bs, int64_t sector_num,
> -                             int nb_sectors, int *pnum)
> +static int qcow2_is_allocated(BlockDriverState *bs, int64_t sector_num,
> +                              int nb_sectors, int *pnum)
>  {
>      uint64_t cluster_offset;
>      int ret;
> @@ -313,7 +313,7 @@ static int qcow_is_allocated(BlockDriverState *bs, 
> int64_t sector_num,
>  
>  /* handle reading after the end of the backing file */
>  int qcow2_backing_read1(BlockDriverState *bs, QEMUIOVector *qiov,
> -                  int64_t sector_num, int nb_sectors)
> +                        int64_t sector_num, int nb_sectors)

This isn't related to renaming functions. Please don't include pure
formatting changes, all they do is making git blame work worse.

>  {
>      int n1;
>      if ((sector_num + nb_sectors) <= bs->total_sectors)
> @@ -344,7 +344,7 @@ typedef struct QCowAIOCB {
>      QLIST_ENTRY(QCowAIOCB) next_depend;
>  } QCowAIOCB;
>  
> -static void qcow_aio_cancel(BlockDriverAIOCB *blockacb)
> +static void qcow2_aio_cancel(BlockDriverAIOCB *blockacb)
>  {
>      QCowAIOCB *acb = container_of(blockacb, QCowAIOCB, common);
>      if (acb->hd_aiocb)
> @@ -352,21 +352,21 @@ static void qcow_aio_cancel(BlockDriverAIOCB *blockacb)
>      qemu_aio_release(acb);
>  }
>  
> -static AIOPool qcow_aio_pool = {
> +static AIOPool qcow2_aio_pool = {
>      .aiocb_size         = sizeof(QCowAIOCB),
> -    .cancel             = qcow_aio_cancel,
> +    .cancel             = qcow2_aio_cancel,
>  };
>  
> -static void qcow_aio_read_cb(void *opaque, int ret);
> -static void qcow_aio_read_bh(void *opaque)
> +static void qcow2_aio_read_cb(void *opaque, int ret);
> +static void qcow2_aio_read_bh(void *opaque)
>  {
>      QCowAIOCB *acb = opaque;
>      qemu_bh_delete(acb->bh);
>      acb->bh = NULL;
> -    qcow_aio_read_cb(opaque, 0);
> +    qcow2_aio_read_cb(opaque, 0);
>  }
>  
> -static int qcow_schedule_bh(QEMUBHFunc *cb, QCowAIOCB *acb)
> +static int qcow2_schedule_bh(QEMUBHFunc *cb, QCowAIOCB *acb)
>  {
>      if (acb->bh)
>          return -EIO;
> @@ -380,7 +380,7 @@ static int qcow_schedule_bh(QEMUBHFunc *cb, QCowAIOCB 
> *acb)
>      return 0;
>  }
>  
> -static void qcow_aio_read_cb(void *opaque, int ret)
> +static void qcow2_aio_read_cb(void *opaque, int ret)
>  {
>      QCowAIOCB *acb = opaque;
>      BlockDriverState *bs = acb->common.bs;
> @@ -399,10 +399,11 @@ static void qcow_aio_read_cb(void *opaque, int ret)
>      } else {
>          if (s->crypt_method) {
>              qcow2_encrypt_sectors(s, acb->sector_num,  acb->cluster_data,
> -                acb->cluster_data, acb->cur_nr_sectors, 0, 
> &s->aes_decrypt_key);
> +                                  acb->cluster_data, acb->cur_nr_sectors,
> +                                  0, &s->aes_decrypt_key);

Same here, plus the old version wasn't obviously indented wrong, but
just not according to your personal style.

The following changes include more lines that need not be changed for
the rename and just change the coding style (even though CODING_STYLE
doesn't make a statement on this, so the old version isn't wrong).
Please leave them out.

> @@ -1266,36 +1276,36 @@ static QEMUOptionParameter qcow_create_options[] = {
>  static BlockDriver bdrv_qcow2 = {
>      .format_name     = "qcow2",
>      .instance_size   = sizeof(BDRVQcowState),
> -    .bdrv_probe              = qcow_probe,
> -    .bdrv_open               = qcow_open,
> -    .bdrv_close              = qcow_close,
> -    .bdrv_create     = qcow_create,
> -    .bdrv_flush              = qcow_flush,
> -    .bdrv_is_allocated       = qcow_is_allocated,
> -    .bdrv_set_key    = qcow_set_key,
> -    .bdrv_make_empty = qcow_make_empty,
> -
> -    .bdrv_aio_readv  = qcow_aio_readv,
> -    .bdrv_aio_writev = qcow_aio_writev,
> -    .bdrv_aio_flush  = qcow_aio_flush,
> +    .bdrv_probe              = qcow2_probe,
> +    .bdrv_open               = qcow2_open,
> +    .bdrv_close              = qcow2_close,
> +    .bdrv_create     = qcow2_create,
> +    .bdrv_flush              = qcow2_flush,
> +    .bdrv_is_allocated       = qcow2_is_allocated,
> +    .bdrv_set_key    = qcow2_set_key,
> +    .bdrv_make_empty = qcow2_make_empty,

Can you take the chance and convert the tabs to spaces?

Kevin