Re: [Qemu-devel] [PATCH 08/23] block: Eliminate BlockDriverState member device_name[]

2014-09-11 Thread Benoît Canet 
The Wednesday 10 Sep 2014 à 10:13:37 (+0200), Markus Armbruster wrote :
 device_name[] is can become non-empty only in bdrv_new_named() and
 bdrv_move_feature_fields().  The latter is used only to undo damage
 done by bdrv_swap().  The former is called only by blk_new_with_bs().
 Therefore, when a BlockDriverState's device_name[] is non-empty, then
 it's owned by a BlockBackend.
 
 The converse is also true, because blk_attach_bs() is called only by
 blk_new_with_bs() so far.
 
 Furthermore, blk_new_with_bs() keeps the two names equal.
 
 Therefore, device_name[] is redundant.  Eliminate it.
 
 Signed-off-by: Markus Armbruster arm...@redhat.com
 ---
  block-migration.c | 12 +
  block.c   | 63 
 ++-
  block/block-backend.c | 12 -
  block/cow.c   |  2 +-
  block/mirror.c|  3 ++-
  block/qapi.c  |  6 ++---
  block/qcow.c  |  4 +--
  block/qcow2.c |  4 +--
  block/qed.c   |  2 +-
  block/quorum.c|  4 +--
  block/vdi.c   |  2 +-
  block/vhdx.c  |  2 +-
  block/vmdk.c  |  4 +--
  block/vpc.c   |  2 +-
  block/vvfat.c |  2 +-
  blockjob.c|  3 ++-
  include/block/block.h |  3 +--
  include/block/block_int.h |  2 --
  18 files changed, 53 insertions(+), 79 deletions(-)
 
 diff --git a/block-migration.c b/block-migration.c
 index cb3e16c..da30e93 100644
 --- a/block-migration.c
 +++ b/block-migration.c
 @@ -14,7 +14,9 @@
   */
  
  #include qemu-common.h
 -#include block/block_int.h
 +#include block/block.h
 +#include qemu/error-report.h
 +#include qemu/main-loop.h
  #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);
 diff --git a/block.c b/block.c
 index 593d89b..61ea15d 100644
 --- a/block.c
 +++ b/block.c
 @@ -332,31 +332,6 @@ void bdrv_register(BlockDriver *bdrv)
  QLIST_INSERT_HEAD(bdrv_drivers, bdrv, list);
  }
  
 -/* create a new block device (by default it is empty) */
 -BlockDriverState *bdrv_new_named(const char *device_name, Error **errp)
 -{
 -BlockDriverState *bs;
 -
 -assert(*device_name);
 -
 -if (bdrv_find(device_name)) {
 -error_setg(errp, Device with id '%s' already exists,
 -   device_name);
 -return NULL;
 -}
 -if (bdrv_find_node(device_name)) {
 -error_setg(errp, Device with node-name '%s' already exists,
 -   device_name);
 -return NULL;
 -}
 -
 -bs = bdrv_new();
 -
 -pstrcpy(bs-device_name, sizeof(bs-device_name), device_name);
 -
 -return bs;
 -}
 -
  BlockDriverState *bdrv_new(void)
  {
  BlockDriverState *bs;
 @@ -1159,7 +1134,7 @@ void bdrv_set_backing_hd(BlockDriverState *bs, 
 BlockDriverState *backing_hd)
  } else if (backing_hd) {
  error_setg(bs-backing_blocker,
 device is used as backing hd of '%s',
 -   bs-device_name);
 +   bdrv_get_device_name(bs));
  }
  
  bs-backing_hd = backing_hd;
 @@ -1533,7 +1508,7 @@ int bdrv_open(BlockDriverState **pbs, const char 
 *filename,
  } else {
  error_setg(errp, Block format '%s' used by device '%s' doesn't 
 support the option '%s', drv-format_name,
 -   bs-device_name, entry-key);
 +   bdrv_get_device_name(bs), entry-key);
  }
  
  ret = -EINVAL;
 @@ -1740,7 +1715,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, 
 BlockReopenQueue *queue,
  if (!(reopen_state-bs-open_flags  BDRV_O_ALLOW_RDWR) 
  reopen_state-flags  BDRV_O_RDWR) {
  error_set(errp, QERR_DEVICE_IS_READ_ONLY,
 -  reopen_state-bs-device_name);
 +  bdrv_get_device_name(reopen_state-bs));
  goto error;
  }
  
 @@ -1767,7

Re: [Qemu-devel] [PATCH 08/23] block: Eliminate BlockDriverState member device_name[]

2014-09-11 Thread Benoît Canet 
The Thursday 11 Sep 2014 à 13:34:33 (+0200), Benoît Canet wrote :
 The Wednesday 10 Sep 2014 à 10:13:37 (+0200), Markus Armbruster wrote :
  device_name[] is can become non-empty only in bdrv_new_named() and
  bdrv_move_feature_fields().  The latter is used only to undo damage
  done by bdrv_swap().  The former is called only by blk_new_with_bs().
  Therefore, when a BlockDriverState's device_name[] is non-empty, then
  it's owned by a BlockBackend.
  
  The converse is also true, because blk_attach_bs() is called only by
  blk_new_with_bs() so far.
  
  Furthermore, blk_new_with_bs() keeps the two names equal.
  
  Therefore, device_name[] is redundant.  Eliminate it.
  
  Signed-off-by: Markus Armbruster arm...@redhat.com
  ---
   block-migration.c | 12 +
   block.c   | 63 
  ++-
   block/block-backend.c | 12 -
   block/cow.c   |  2 +-
   block/mirror.c|  3 ++-
   block/qapi.c  |  6 ++---
   block/qcow.c  |  4 +--
   block/qcow2.c |  4 +--
   block/qed.c   |  2 +-
   block/quorum.c|  4 +--
   block/vdi.c   |  2 +-
   block/vhdx.c  |  2 +-
   block/vmdk.c  |  4 +--
   block/vpc.c   |  2 +-
   block/vvfat.c |  2 +-
   blockjob.c|  3 ++-
   include/block/block.h |  3 +--
   include/block/block_int.h |  2 --
   18 files changed, 53 insertions(+), 79 deletions(-)
  
  diff --git a/block-migration.c b/block-migration.c
  index cb3e16c..da30e93 100644
  --- a/block-migration.c
  +++ b/block-migration.c
  @@ -14,7 +14,9 @@
*/
   
   #include qemu-common.h
  -#include block/block_int.h
  +#include block/block.h
  +#include qemu/error-report.h
  +#include qemu/main-loop.h
   #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);
  diff --git a/block.c b/block.c
  index 593d89b..61ea15d 100644
  --- a/block.c
  +++ b/block.c
  @@ -332,31 +332,6 @@ void bdrv_register(BlockDriver *bdrv)
   QLIST_INSERT_HEAD(bdrv_drivers, bdrv, list);
   }
   
  -/* create a new block device (by default it is empty) */
  -BlockDriverState *bdrv_new_named(const char *device_name, Error **errp)
  -{
  -BlockDriverState *bs;
  -
  -assert(*device_name);
  -
  -if (bdrv_find(device_name)) {
  -error_setg(errp, Device with id '%s' already exists,
  -   device_name);
  -return NULL;
  -}
  -if (bdrv_find_node(device_name)) {
  -error_setg(errp, Device with node-name '%s' already exists,
  -   device_name);
  -return NULL;
  -}
  -
  -bs = bdrv_new();
  -
  -pstrcpy(bs-device_name, sizeof(bs-device_name), device_name);
  -
  -return bs;
  -}
  -
   BlockDriverState *bdrv_new(void)
   {
   BlockDriverState *bs;
  @@ -1159,7 +1134,7 @@ void bdrv_set_backing_hd(BlockDriverState *bs, 
  BlockDriverState *backing_hd)
   } else if (backing_hd) {
   error_setg(bs-backing_blocker,
  device is used as backing hd of '%s',
  -   bs-device_name);
  +   bdrv_get_device_name(bs));
   }
   
   bs-backing_hd = backing_hd;
  @@ -1533,7 +1508,7 @@ int bdrv_open(BlockDriverState **pbs, const char 
  *filename,
   } else {
   error_setg(errp, Block format '%s' used by device '%s' 
  doesn't 
  support the option '%s', drv-format_name,
  -   bs-device_name, entry-key);
  +   bdrv_get_device_name(bs), entry-key);
   }
   
   ret = -EINVAL;
  @@ -1740,7 +1715,7 @@ int bdrv_reopen_prepare(BDRVReopenState 
  *reopen_state, BlockReopenQueue *queue,
   if (!(reopen_state-bs-open_flags  BDRV_O_ALLOW_RDWR) 
   reopen_state-flags  BDRV_O_RDWR)

Re: [Qemu-devel] [PATCH 08/23] block: Eliminate BlockDriverState member device_name[]

2014-09-11 Thread Eric Blake 
On 09/11/2014 05:34 AM, Benoît Canet wrote:
 The Wednesday 10 Sep 2014 à 10:13:37 (+0200), Markus Armbruster wrote :
 device_name[] is can become non-empty only in bdrv_new_named() and
 bdrv_move_feature_fields().  The latter is used only to undo damage
 done by bdrv_swap().  The former is called only by blk_new_with_bs().
 Therefore, when a BlockDriverState's device_name[] is non-empty, then
 it's owned by a BlockBackend.

[lots of lines trimmed - it's not only okay, but desirable to trim out
portions of a patch that you are okay with, in order to call attention
to the problem spots that you are commenting on without making the
reader have to scroll through pages of quoted context]

  
 -const char *bdrv_get_device_name(BlockDriverState *bs)
 +const char *bdrv_get_device_name(const BlockDriverState *bs)
  {
 -return bs-device_name;
 +const char *name = bs-blk ? blk_name(bs-blk) : NULL;
 +return name ?: ;
  }
 
 Why not ?
 
 return bs-blk ? blk_name(bs-blk) : ;

If I understand right, it was because blk_name(bs-blk) may return NULL,
but this function is guaranteed to return non-NULL.

 
 or 
 
 if (bs-blk) {
 return blk_name(bs-blk);
 }
 
 return ;
 
 Would it fail to do the job ?
 
 Also we could have made sure that bdrv_get_device_name return either
 a non empty string or NULL.
 
 (We know blk_name will return a non empty string it's asserted)
 
 This would need to change a few test all around the code but the final
 code logic would be less convoluted:
 -convert NULL to  then test for not 
 would turn into
 -return NULL test for not NULL

Indeed, the logic of NULL vs. string is slightly easier than the logic
of  vs non-empty string; if we can guarantee that a non-NULL string
will be non-empty.  I'm not sure how much churn it would cause, though.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 08/23] block: Eliminate BlockDriverState member device_name[]

2014-09-11 Thread Benoît Canet 
The Thursday 11 Sep 2014 à 07:00:41 (-0600), Eric Blake wrote :
 On 09/11/2014 05:34 AM, Benoît Canet wrote:
  The Wednesday 10 Sep 2014 à 10:13:37 (+0200), Markus Armbruster wrote :
  device_name[] is can become non-empty only in bdrv_new_named() and
  bdrv_move_feature_fields().  The latter is used only to undo damage
  done by bdrv_swap().  The former is called only by blk_new_with_bs().
  Therefore, when a BlockDriverState's device_name[] is non-empty, then
  it's owned by a BlockBackend.
 
 [lots of lines trimmed - it's not only okay, but desirable to trim out
 portions of a patch that you are okay with, in order to call attention
 to the problem spots that you are commenting on without making the
 reader have to scroll through pages of quoted context]
 
   
  -const char *bdrv_get_device_name(BlockDriverState *bs)
  +const char *bdrv_get_device_name(const BlockDriverState *bs)
   {
  -return bs-device_name;
  +const char *name = bs-blk ? blk_name(bs-blk) : NULL;
  +return name ?: ;
   }
  
  Why not ?
  
  return bs-blk ? blk_name(bs-blk) : ;
 
 If I understand right, it was because blk_name(bs-blk) may return NULL,

It think it can't: see patch 2 extract:

 +BlockBackend *blk_new(const char *name, Error **errp)
 +{
 +BlockBackend *blk = g_new0(BlockBackend, 1);
 + 
 +assert(name  name[0]);

 but this function is guaranteed to return non-NULL.

Re: [Qemu-devel] [PATCH 08/23] block: Eliminate BlockDriverState member device_name[]

2014-09-11 Thread Markus Armbruster 
Eric Blake ebl...@redhat.com writes:

 On 09/10/2014 02:13 AM, Markus Armbruster wrote:
 device_name[] is can become non-empty only in bdrv_new_named() and

 s/is //

Fixing, thanks!

 bdrv_move_feature_fields().  The latter is used only to undo damage
 done by bdrv_swap().  The former is called only by blk_new_with_bs().
 Therefore, when a BlockDriverState's device_name[] is non-empty, then
 it's owned by a BlockBackend.
 
 The converse is also true, because blk_attach_bs() is called only by
 blk_new_with_bs() so far.
 
 Furthermore, blk_new_with_bs() keeps the two names equal.
 
 Therefore, device_name[] is redundant.  Eliminate it.
 
 Signed-off-by: Markus Armbruster arm...@redhat.com
 ---

Re: [Qemu-devel] [PATCH 08/23] block: Eliminate BlockDriverState member device_name[]

2014-09-11 Thread Markus Armbruster 
Benoît Canet benoit.ca...@irqsave.net writes:

 The Wednesday 10 Sep 2014 à 10:13:37 (+0200), Markus Armbruster wrote :
 device_name[] is can become non-empty only in bdrv_new_named() and
 bdrv_move_feature_fields().  The latter is used only to undo damage
 done by bdrv_swap().  The former is called only by blk_new_with_bs().
 Therefore, when a BlockDriverState's device_name[] is non-empty, then
 it's owned by a BlockBackend.
 
 The converse is also true, because blk_attach_bs() is called only by
 blk_new_with_bs() so far.
 
 Furthermore, blk_new_with_bs() keeps the two names equal.
 
 Therefore, device_name[] is redundant.  Eliminate it.
 
 Signed-off-by: Markus Armbruster arm...@redhat.com
 ---
  block-migration.c | 12 +
  block.c   | 63 
 ++-
  block/block-backend.c | 12 -
  block/cow.c   |  2 +-
  block/mirror.c|  3 ++-
  block/qapi.c  |  6 ++---
  block/qcow.c  |  4 +--
  block/qcow2.c |  4 +--
  block/qed.c   |  2 +-
  block/quorum.c|  4 +--
  block/vdi.c   |  2 +-
  block/vhdx.c  |  2 +-
  block/vmdk.c  |  4 +--
  block/vpc.c   |  2 +-
  block/vvfat.c |  2 +-
  blockjob.c|  3 ++-
  include/block/block.h |  3 +--
  include/block/block_int.h |  2 --
  18 files changed, 53 insertions(+), 79 deletions(-)
 
 diff --git a/block-migration.c b/block-migration.c
 index cb3e16c..da30e93 100644
 --- a/block-migration.c
 +++ b/block-migration.c
 @@ -14,7 +14,9 @@
   */
  
  #include qemu-common.h
 -#include block/block_int.h
 +#include block/block.h
 +#include qemu/error-report.h
 +#include qemu/main-loop.h
  #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);
 diff --git a/block.c b/block.c
 index 593d89b..61ea15d 100644
 --- a/block.c
 +++ b/block.c
 @@ -332,31 +332,6 @@ void bdrv_register(BlockDriver *bdrv)
  QLIST_INSERT_HEAD(bdrv_drivers, bdrv, list);
  }
  
 -/* create a new block device (by default it is empty) */
 -BlockDriverState *bdrv_new_named(const char *device_name, Error **errp)
 -{
 -BlockDriverState *bs;
 -
 -assert(*device_name);
 -
 -if (bdrv_find(device_name)) {
 -error_setg(errp, Device with id '%s' already exists,
 -   device_name);
 -return NULL;
 -}
 -if (bdrv_find_node(device_name)) {
 -error_setg(errp, Device with node-name '%s' already exists,
 -   device_name);
 -return NULL;
 -}
 -
 -bs = bdrv_new();
 -
 -pstrcpy(bs-device_name, sizeof(bs-device_name), device_name);
 -
 -return bs;
 -}
 -
  BlockDriverState *bdrv_new(void)
  {
  BlockDriverState *bs;
 @@ -1159,7 +1134,7 @@ void bdrv_set_backing_hd(BlockDriverState *bs, 
 BlockDriverState *backing_hd)
  } else if (backing_hd) {
  error_setg(bs-backing_blocker,
 device is used as backing hd of '%s',
 -   bs-device_name);
 +   bdrv_get_device_name(bs));
  }
  
  bs-backing_hd = backing_hd;
 @@ -1533,7 +1508,7 @@ int bdrv_open(BlockDriverState **pbs, const char 
 *filename,
  } else {
  error_setg(errp, Block format '%s' used by device '%s' doesn't 
 
 support the option '%s', drv-format_name,
 -   bs-device_name, entry-key);
 +   bdrv_get_device_name(bs), entry-key);
  }
  
  ret = -EINVAL;
 @@ -1740,7 +1715,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, 
 BlockReopenQueue *queue,
  if (!(reopen_state-bs-open_flags  BDRV_O_ALLOW_RDWR) 
  reopen_state-flags  BDRV_O_RDWR) {
  error_set(errp, QERR_DEVICE_IS_READ_ONLY,
 -  reopen_state-bs-device_name);
 +

Re: [Qemu-devel] [PATCH 08/23] block: Eliminate BlockDriverState member device_name[]

2014-09-11 Thread Markus Armbruster 
Benoît Canet benoit.ca...@irqsave.net writes:

 The Thursday 11 Sep 2014 à 07:00:41 (-0600), Eric Blake wrote :
 On 09/11/2014 05:34 AM, Benoît Canet wrote:
  The Wednesday 10 Sep 2014 à 10:13:37 (+0200), Markus Armbruster wrote :
  device_name[] is can become non-empty only in bdrv_new_named() and
  bdrv_move_feature_fields().  The latter is used only to undo damage
  done by bdrv_swap().  The former is called only by blk_new_with_bs().
  Therefore, when a BlockDriverState's device_name[] is non-empty, then
  it's owned by a BlockBackend.
 
 [lots of lines trimmed - it's not only okay, but desirable to trim out
 portions of a patch that you are okay with, in order to call attention
 to the problem spots that you are commenting on without making the
 reader have to scroll through pages of quoted context]
 
   
  -const char *bdrv_get_device_name(BlockDriverState *bs)
  +const char *bdrv_get_device_name(const BlockDriverState *bs)
   {
  -return bs-device_name;
  +const char *name = bs-blk ? blk_name(bs-blk) : NULL;
  +return name ?: ;
   }
  
  Why not ?
  
  return bs-blk ? blk_name(bs-blk) : ;
 
 If I understand right, it was because blk_name(bs-blk) may return NULL,

 It think it can't: see patch 2 extract:

 +BlockBackend *blk_new(const char *name, Error **errp)
 +{
 +BlockBackend *blk = g_new0(BlockBackend, 1);
 + 
 +assert(name  name[0]);

 but this function is guaranteed to return non-NULL.

You're right, blk_new() always returns a non-null, non-empty string.

The condition to check here is bs is not owned by a BB.  Benoît's

bs-blk ? blk_name(bs-blk) : 

does that nicely.

Of course, Eric is right in that returning NULL on not owned by a BB
would be cleaner than returning .  However, doing that in the middle
of a series with a four-digit diffstat doesn't strike me as a good
idea.  Better do it on top.

[Qemu-devel] [PATCH 08/23] block: Eliminate BlockDriverState member device_name[]

2014-09-10 Thread Markus Armbruster 
device_name[] is can become non-empty only in bdrv_new_named() and
bdrv_move_feature_fields().  The latter is used only to undo damage
done by bdrv_swap().  The former is called only by blk_new_with_bs().
Therefore, when a BlockDriverState's device_name[] is non-empty, then
it's owned by a BlockBackend.

The converse is also true, because blk_attach_bs() is called only by
blk_new_with_bs() so far.

Furthermore, blk_new_with_bs() keeps the two names equal.

Therefore, device_name[] is redundant.  Eliminate it.

Signed-off-by: Markus Armbruster arm...@redhat.com
---
 block-migration.c | 12 +
 block.c   | 63 ++-
 block/block-backend.c | 12 -
 block/cow.c   |  2 +-
 block/mirror.c|  3 ++-
 block/qapi.c  |  6 ++---
 block/qcow.c  |  4 +--
 block/qcow2.c |  4 +--
 block/qed.c   |  2 +-
 block/quorum.c|  4 +--
 block/vdi.c   |  2 +-
 block/vhdx.c  |  2 +-
 block/vmdk.c  |  4 +--
 block/vpc.c   |  2 +-
 block/vvfat.c |  2 +-
 blockjob.c|  3 ++-
 include/block/block.h |  3 +--
 include/block/block_int.h |  2 --
 18 files changed, 53 insertions(+), 79 deletions(-)

diff --git a/block-migration.c b/block-migration.c
index cb3e16c..da30e93 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -14,7 +14,9 @@
  */
 
 #include qemu-common.h
-#include block/block_int.h
+#include block/block.h
+#include qemu/error-report.h
+#include qemu/main-loop.h
 #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);
diff --git a/block.c b/block.c
index 593d89b..61ea15d 100644
--- a/block.c
+++ b/block.c
@@ -332,31 +332,6 @@ void bdrv_register(BlockDriver *bdrv)
 QLIST_INSERT_HEAD(bdrv_drivers, bdrv, list);
 }
 
-/* create a new block device (by default it is empty) */
-BlockDriverState *bdrv_new_named(const char *device_name, Error **errp)
-{
-BlockDriverState *bs;
-
-assert(*device_name);
-
-if (bdrv_find(device_name)) {
-error_setg(errp, Device with id '%s' already exists,
-   device_name);
-return NULL;
-}
-if (bdrv_find_node(device_name)) {
-error_setg(errp, Device with node-name '%s' already exists,
-   device_name);
-return NULL;
-}
-
-bs = bdrv_new();
-
-pstrcpy(bs-device_name, sizeof(bs-device_name), device_name);
-
-return bs;
-}
-
 BlockDriverState *bdrv_new(void)
 {
 BlockDriverState *bs;
@@ -1159,7 +1134,7 @@ void bdrv_set_backing_hd(BlockDriverState *bs, 
BlockDriverState *backing_hd)
 } else if (backing_hd) {
 error_setg(bs-backing_blocker,
device is used as backing hd of '%s',
-   bs-device_name);
+   bdrv_get_device_name(bs));
 }
 
 bs-backing_hd = backing_hd;
@@ -1533,7 +1508,7 @@ int bdrv_open(BlockDriverState **pbs, const char 
*filename,
 } else {
 error_setg(errp, Block format '%s' used by device '%s' doesn't 
support the option '%s', drv-format_name,
-   bs-device_name, entry-key);
+   bdrv_get_device_name(bs), entry-key);
 }
 
 ret = -EINVAL;
@@ -1740,7 +1715,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, 
BlockReopenQueue *queue,
 if (!(reopen_state-bs-open_flags  BDRV_O_ALLOW_RDWR) 
 reopen_state-flags  BDRV_O_RDWR) {
 error_set(errp, QERR_DEVICE_IS_READ_ONLY,
-  reopen_state-bs-device_name);
+  bdrv_get_device_name(reopen_state-bs));
 goto error;
 }
 
@@ -1767,7 +1742,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, 
BlockReopenQueue *queue,
 /* It is currently mandatory to have a bdrv_reopen_prepare()
  * handler for each supported drv. */

Re: [Qemu-devel] [PATCH 08/23] block: Eliminate BlockDriverState member device_name[]

2014-09-10 Thread Eric Blake 
On 09/10/2014 02:13 AM, Markus Armbruster wrote:
 device_name[] is can become non-empty only in bdrv_new_named() and

s/is //

 bdrv_move_feature_fields().  The latter is used only to undo damage
 done by bdrv_swap().  The former is called only by blk_new_with_bs().
 Therefore, when a BlockDriverState's device_name[] is non-empty, then
 it's owned by a BlockBackend.
 
 The converse is also true, because blk_attach_bs() is called only by
 blk_new_with_bs() so far.
 
 Furthermore, blk_new_with_bs() keeps the two names equal.
 
 Therefore, device_name[] is redundant.  Eliminate it.
 
 Signed-off-by: Markus Armbruster arm...@redhat.com
 ---


-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature