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

[Markus Armbruster <armbru@redhat.com>]

device_name[] can become non-empty only in bdrv_new_root() 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 been created with a BlockBackend, and vice versa.  Furthermore,
blk_new_with_bs() keeps the two names equal.

Therefore, device_name[] is redundant.  Eliminate it.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block-migration.c         | 12 ++++----
 block.c                   | 70 ++++++++++++++++++++---------------------------
 block/block-backend.c     | 14 ++++++----
 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     |  4 +--
 include/block/block_int.h |  2 --
 18 files changed, 67 insertions(+), 75 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 50ccafd..c3fa3da 100644
--- a/block.c
+++ b/block.c
@@ -28,6 +28,7 @@
 #include "block/blockjob.h"
 #include "qemu/module.h"
 #include "qapi/qmp/qjson.h"
+#include "sysemu/block-backend.h"
 #include "sysemu/sysemu.h"
 #include "qemu/notify.h"
 #include "block/coroutine.h"
@@ -334,30 +335,11 @@ 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_root(const char *device_name, Error **errp)
+BlockDriverState *bdrv_new_root(void)
 {
-    BlockDriverState *bs;
+    BlockDriverState *bs = bdrv_new();
 
-    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 name '%s' conflicts with an existing node name",
-                   device_name);
-        return NULL;
-    }
-
-    bs = bdrv_new();
-
-    pstrcpy(bs->device_name, sizeof(bs->device_name), device_name);
     QTAILQ_INSERT_TAIL(&bdrv_states, bs, device_list);
-
     return bs;
 }
 
@@ -1163,7 +1145,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;
@@ -1537,7 +1519,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;
@@ -1744,7 +1726,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;
     }
 
@@ -1771,7 +1753,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. */
         error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
-                  drv->format_name, reopen_state->bs->device_name,
+                  drv->format_name, bdrv_get_device_name(reopen_state->bs),
                  "reopening of file");
         ret = -1;
         goto error;
@@ -1959,10 +1941,17 @@ void bdrv_drain_all(void)
    Also, NULL terminate the device_name to prevent double remove */
 void bdrv_make_anon(BlockDriverState *bs)
 {
-    if (bs->device_name[0] != '\0') {
+    /*
+     * Take care to remove bs from bdrv_states only when it's actually
+     * in it.  Note that bs->device_list.tqe_prev is initially null,
+     * and gets set to non-null by QTAILQ_INSERT_TAIL().  Establish
+     * the useful invariant "bs in bdrv_states iff bs->tqe_prev" by
+     * resetting it to null on remove.
+     */
+    if (bs->device_list.tqe_prev) {
         QTAILQ_REMOVE(&bdrv_states, bs, device_list);
+        bs->device_list.tqe_prev = NULL;
     }
-    bs->device_name[0] = '\0';
     if (bs->node_name[0] != '\0') {
         QTAILQ_REMOVE(&graph_bdrv_states, bs, node_list);
     }
@@ -2016,8 +2005,6 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest,
     bs_dest->job                = bs_src->job;
 
     /* keep the same entry in bdrv_states */
-    pstrcpy(bs_dest->device_name, sizeof(bs_dest->device_name),
-            bs_src->device_name);
     bs_dest->device_list = bs_src->device_list;
     memcpy(bs_dest->op_blockers, bs_src->op_blockers,
            sizeof(bs_dest->op_blockers));
@@ -2031,7 +2018,7 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest,
  * This will modify the BlockDriverState fields, and swap contents
  * between bs_new and bs_old. Both bs_new and bs_old are modified.
  *
- * bs_new must be nameless and not attached to a BlockBackend.
+ * bs_new must not be attached to a BlockBackend.
  *
  * This function does not create any image files.
  */
@@ -2050,8 +2037,7 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old)
         QTAILQ_REMOVE(&graph_bdrv_states, bs_old, node_list);
     }
 
-    /* bs_new must be nameless and shouldn't have anything fancy enabled */
-    assert(bs_new->device_name[0] == '\0');
+    /* bs_new must be unattached and shouldn't have anything fancy enabled */
     assert(!bs_new->blk);
     assert(QLIST_EMPTY(&bs_new->dirty_bitmaps));
     assert(bs_new->job == NULL);
@@ -2068,8 +2054,7 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old)
     bdrv_move_feature_fields(bs_old, bs_new);
     bdrv_move_feature_fields(bs_new, &tmp);
 
-    /* bs_new must remain nameless and unattached */
-    assert(bs_new->device_name[0] == '\0');
+    /* bs_new must remain unattached */
     assert(!bs_new->blk);
 
     /* Check a few fields that should remain attached to the device */
@@ -2097,7 +2082,7 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old)
  * This will modify the BlockDriverState fields, and swap contents
  * between bs_new and bs_top. Both bs_new and bs_top are modified.
  *
- * bs_new must be nameless and not attached to a BlockBackend.
+ * bs_new must not be attached to a BlockBackend.
  *
  * This function does not create any image files.
  */
@@ -3813,7 +3798,7 @@ BlockDriverState *bdrv_find(const char *name)
     BlockDriverState *bs;
 
     QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
-        if (!strcmp(name, bs->device_name)) {
+        if (!strcmp(name, bdrv_get_device_name(bs))) {
             return bs;
         }
     }
@@ -3899,9 +3884,9 @@ BlockDriverState *bdrv_next(BlockDriverState *bs)
     return QTAILQ_NEXT(bs, device_list);
 }
 
-const char *bdrv_get_device_name(BlockDriverState *bs)
+const char *bdrv_get_device_name(const BlockDriverState *bs)
 {
-    return bs->device_name;
+    return bs->blk ? blk_name(bs->blk) : "";
 }
 
 int bdrv_get_flags(BlockDriverState *bs)
@@ -5263,13 +5248,15 @@ int bdrv_media_changed(BlockDriverState *bs)
 void bdrv_eject(BlockDriverState *bs, bool eject_flag)
 {
     BlockDriver *drv = bs->drv;
+    const char *device_name;
 
     if (drv && drv->bdrv_eject) {
         drv->bdrv_eject(bs, eject_flag);
     }
 
-    if (bs->device_name[0] != '\0') {
-        qapi_event_send_device_tray_moved(bdrv_get_device_name(bs),
+    device_name = bdrv_get_device_name(bs);
+    if (device_name[0] != '\0') {
+        qapi_event_send_device_tray_moved(device_name,
                                           eject_flag, &error_abort);
     }
 }
@@ -5479,7 +5466,8 @@ bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp)
         blocker = QLIST_FIRST(&bs->op_blockers[op]);
         if (errp) {
             error_setg(errp, "Device '%s' is busy: %s",
-                       bs->device_name, error_get_pretty(blocker->reason));
+                       bdrv_get_device_name(bs),
+                       error_get_pretty(blocker->reason));
         }
         return true;
     }
diff --git a/block/block-backend.c b/block/block-backend.c
index ed4873e..f14a291 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -61,17 +61,19 @@ BlockBackend *blk_new_with_bs(const char *name, Error **errp)
     BlockBackend *blk;
     BlockDriverState *bs;
 
+    if (bdrv_find_node(name)) {
+        error_setg(errp,
+                   "Device name '%s' conflicts with an existing node name",
+                   name);
+        return NULL;
+    }
+
     blk = blk_new(name, errp);
     if (!blk) {
         return NULL;
     }
 
-    bs = bdrv_new_root(name, errp);
-    if (!bs) {
-        blk_unref(blk);
-        return NULL;
-    }
-
+    bs = bdrv_new_root();
     blk->bs = bs;
     bs->blk = blk;
     return blk;
diff --git a/block/cow.c b/block/cow.c
index c3769fe..ff0d4ae 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -84,7 +84,7 @@ static int cow_open(BlockDriverState *bs, QDict *options, int flags,
         snprintf(version, sizeof(version),
                "COW version %" PRIu32, cow_header.version);
         error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
-            bs->device_name, "cow", version);
+            bdrv_get_device_name(bs), "cow", version);
         ret = -ENOTSUP;
         goto fail;
     }
diff --git a/block/mirror.c b/block/mirror.c
index 18b18e0..829be2f 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -567,7 +567,8 @@ static void mirror_complete(BlockJob *job, Error **errp)
         return;
     }
     if (!s->synced) {
-        error_set(errp, QERR_BLOCK_JOB_NOT_READY, job->bs->device_name);
+        error_set(errp, QERR_BLOCK_JOB_NOT_READY,
+                  bdrv_get_device_name(job->bs));
         return;
     }
 
diff --git a/block/qapi.c b/block/qapi.c
index 9733ebd..d071ee5 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -272,7 +272,7 @@ void bdrv_query_info(BlockDriverState *bs,
     BlockDriverState *bs0;
     ImageInfo **p_image_info;
     Error *local_err = NULL;
-    info->device = g_strdup(bs->device_name);
+    info->device = g_strdup(bdrv_get_device_name(bs));
     info->type = g_strdup("unknown");
     info->locked = bdrv_dev_is_medium_locked(bs);
     info->removable = bdrv_dev_has_removable_media(bs);
@@ -327,9 +327,9 @@ static BlockStats *bdrv_query_stats(const BlockDriverState *bs)
 
     s = g_malloc0(sizeof(*s));
 
-    if (bs->device_name[0]) {
+    if (bdrv_get_device_name(bs)[0]) {
         s->has_device = true;
-        s->device = g_strdup(bs->device_name);
+        s->device = g_strdup(bdrv_get_device_name(bs));
     }
 
     s->stats = g_malloc0(sizeof(*s->stats));
diff --git a/block/qcow.c b/block/qcow.c
index a87bd69..ece2269 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -124,7 +124,7 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
         snprintf(version, sizeof(version), "QCOW version %" PRIu32,
                  header.version);
         error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
-                  bs->device_name, "qcow", version);
+                  bdrv_get_device_name(bs), "qcow", version);
         ret = -ENOTSUP;
         goto fail;
     }
@@ -231,7 +231,7 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
     /* Disable migration when qcow images are used */
     error_set(&s->migration_blocker,
               QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
-              "qcow", bs->device_name, "live migration");
+              "qcow", bdrv_get_device_name(bs), "live migration");
     migrate_add_blocker(s->migration_blocker);
 
     qemu_co_mutex_init(&s->lock);
diff --git a/block/qcow2.c b/block/qcow2.c
index 0daf25c..d5836bb 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -204,8 +204,8 @@ static void GCC_FMT_ATTR(3, 4) report_unsupported(BlockDriverState *bs,
     vsnprintf(msg, sizeof(msg), fmt, ap);
     va_end(ap);
 
-    error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, bs->device_name, "qcow2",
-              msg);
+    error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
+              bdrv_get_device_name(bs), "qcow2", msg);
 }
 
 static void report_unsupported_feature(BlockDriverState *bs,
diff --git a/block/qed.c b/block/qed.c
index f8d9e12..9181340 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -422,7 +422,7 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags,
         snprintf(buf, sizeof(buf), "%" PRIx64,
             s->header.features & ~QED_FEATURE_MASK);
         error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
-            bs->device_name, "QED", buf);
+            bdrv_get_device_name(bs), "QED", buf);
         return -ENOTSUP;
     }
     if (!qed_is_cluster_size_valid(s->header.cluster_size)) {
diff --git a/block/quorum.c b/block/quorum.c
index 093382e..f958269 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -227,8 +227,8 @@ static void quorum_report_bad(QuorumAIOCB *acb, char *node_name, int ret)
 
 static void quorum_report_failure(QuorumAIOCB *acb)
 {
-    const char *reference = acb->common.bs->device_name[0] ?
-                            acb->common.bs->device_name :
+    const char *reference = bdrv_get_device_name(acb->common.bs)[0] ?
+                            bdrv_get_device_name(acb->common.bs) :
                             acb->common.bs->node_name;
 
     qapi_event_send_quorum_failure(reference, acb->sector_num,
diff --git a/block/vdi.c b/block/vdi.c
index cfa08b0..9604721 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -490,7 +490,7 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
     /* Disable migration when vdi images are used */
     error_set(&s->migration_blocker,
               QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
-              "vdi", bs->device_name, "live migration");
+              "vdi", bdrv_get_device_name(bs), "live migration");
     migrate_add_blocker(s->migration_blocker);
 
     return 0;
diff --git a/block/vhdx.c b/block/vhdx.c
index 796b7bd..03b564e 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1003,7 +1003,7 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
     /* Disable migration when VHDX images are used */
     error_set(&s->migration_blocker,
             QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
-            "vhdx", bs->device_name, "live migration");
+            "vhdx", bdrv_get_device_name(bs), "live migration");
     migrate_add_blocker(s->migration_blocker);
 
     return 0;
diff --git a/block/vmdk.c b/block/vmdk.c
index afdea1a..811095b 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -657,7 +657,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
         snprintf(buf, sizeof(buf), "VMDK version %" PRId32,
                  le32_to_cpu(header.version));
         error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
-                  bs->device_name, "vmdk", buf);
+                  bdrv_get_device_name(bs), "vmdk", buf);
         return -ENOTSUP;
     } else if (le32_to_cpu(header.version) == 3 && (flags & BDRV_O_RDWR)) {
         /* VMware KB 2064959 explains that version 3 added support for
@@ -939,7 +939,7 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,
     /* Disable migration when VMDK images are used */
     error_set(&s->migration_blocker,
               QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
-              "vmdk", bs->device_name, "live migration");
+              "vmdk", bdrv_get_device_name(bs), "live migration");
     migrate_add_blocker(s->migration_blocker);
     g_free(buf);
     return 0;
diff --git a/block/vpc.c b/block/vpc.c
index 4947369..cdc6896 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -320,7 +320,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
     /* Disable migration when VHD images are used */
     error_set(&s->migration_blocker,
               QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
-              "vpc", bs->device_name, "live migration");
+              "vpc", bdrv_get_device_name(bs), "live migration");
     migrate_add_blocker(s->migration_blocker);
 
     return 0;
diff --git a/block/vvfat.c b/block/vvfat.c
index 6c9fde0..cefe3a4 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1182,7 +1182,7 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
     if (s->qcow) {
         error_set(&s->migration_blocker,
                   QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
-                  "vvfat (rw)", bs->device_name, "live migration");
+                  "vvfat (rw)", bdrv_get_device_name(bs), "live migration");
         migrate_add_blocker(s->migration_blocker);
     }
 
diff --git a/blockjob.c b/blockjob.c
index 0689fdd..3af0f6c 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -107,7 +107,8 @@ void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
 void block_job_complete(BlockJob *job, Error **errp)
 {
     if (job->paused || job->cancelled || !job->driver->complete) {
-        error_set(errp, QERR_BLOCK_JOB_NOT_READY, job->bs->device_name);
+        error_set(errp, QERR_BLOCK_JOB_NOT_READY,
+                  bdrv_get_device_name(job->bs));
         return;
     }
 
diff --git a/include/block/block.h b/include/block/block.h
index 02cf956..27d002d 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -204,7 +204,7 @@ BlockDriver *bdrv_find_whitelisted_format(const char *format_name,
 int bdrv_create(BlockDriver *drv, const char* filename,
                 QemuOpts *opts, Error **errp);
 int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp);
-BlockDriverState *bdrv_new_root(const char *device_name, Error **errp);
+BlockDriverState *bdrv_new_root(void);
 BlockDriverState *bdrv_new(void);
 void bdrv_make_anon(BlockDriverState *bs);
 void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old);
@@ -416,7 +416,7 @@ int bdrv_set_key(BlockDriverState *bs, const char *key);
 int bdrv_query_missing_keys(void);
 void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
                          void *opaque);
-const char *bdrv_get_device_name(BlockDriverState *bs);
+const char *bdrv_get_device_name(const BlockDriverState *bs);
 int bdrv_get_flags(BlockDriverState *bs);
 int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num,
                           const uint8_t *buf, int nb_sectors);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 14e0b7c..da00aa5 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -392,8 +392,6 @@ struct BlockDriverState {
     char node_name[32];
     /* element of the list of named nodes building the graph */
     QTAILQ_ENTRY(BlockDriverState) node_list;
-    /* Device name is the name associated with the "drive" the guest sees */
-    char device_name[32];
     /* element of the list of "drives" the guest sees */
     QTAILQ_ENTRY(BlockDriverState) device_list;
     QLIST_HEAD(, BdrvDirtyBitmap) dirty_bitmaps;
-- 
1.9.3