[Qemu-devel] [PATCH 0/4] block: Convert bdrv_find to blk_by_name and drop it

[Qemu-devel] [PATCH 0/4] block: Convert bdrv_find to blk_by_name and drop it

[Fam Zheng]

This is a small step towards a more complete separation from BlockDriverState
to block backend users.

Fam Zheng (4):
  monitor: Convert bdrv_find to blk_by_name
  migration: Convert bdrv_find to blk_by_name
  blockdev: Convert bdrv_find to blk_by_name
  block: Drop bdrv_find

 block.c               |  9 -----
 blockdev.c            | 92 +++++++++++++++++++++++++++++++++------------------
 include/block/block.h |  1 -
 migration/block.c     |  7 ++--
 monitor.c             |  9 ++---
 5 files changed, 69 insertions(+), 49 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH 1/4] monitor: Convert bdrv_find to blk_by_name

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 monitor.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/monitor.c b/monitor.c
index 41900da..6ad777d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -72,6 +72,7 @@
 #include "block/qapi.h"
 #include "qapi/qmp-event.h"
 #include "qapi-event.h"
+#include "sysemu/block-backend.h"
 
 /* for hmp_info_irq/pic */
 #if defined(TARGET_SPARC)
@@ -5413,15 +5414,15 @@ int monitor_read_block_device_key(Monitor *mon, const char *device,
                                   BlockCompletionFunc *completion_cb,
                                   void *opaque)
 {
-    BlockDriverState *bs;
+    BlockBackend *blk;
 
-    bs = bdrv_find(device);
-    if (!bs) {
+    blk = blk_by_name(device);
+    if (!blk) {
         monitor_printf(mon, "Device not found %s\n", device);
         return -1;
     }
 
-    return monitor_read_bdrv_key_start(mon, bs, completion_cb, opaque);
+    return monitor_read_bdrv_key_start(mon, blk_bs(blk), completion_cb, opaque);
 }
 
 QemuOptsList qemu_mon_opts = {
-- 
1.9.3

Re: [Qemu-devel] [PATCH 1/4] monitor: Convert bdrv_find to blk_by_name

[Max Reitz]

On 2015-03-02 at 06:36, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>   monitor.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 41900da..6ad777d 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -72,6 +72,7 @@
>   #include "block/qapi.h"
>   #include "qapi/qmp-event.h"
>   #include "qapi-event.h"
> +#include "sysemu/block-backend.h"
>   
>   /* for hmp_info_irq/pic */
>   #if defined(TARGET_SPARC)
> @@ -5413,15 +5414,15 @@ int monitor_read_block_device_key(Monitor *mon, const char *device,
>                                     BlockCompletionFunc *completion_cb,
>                                     void *opaque)
>   {
> -    BlockDriverState *bs;
> +    BlockBackend *blk;
>   
> -    bs = bdrv_find(device);
> -    if (!bs) {
> +    blk = blk_by_name(device);
> +    if (!blk) {
>           monitor_printf(mon, "Device not found %s\n", device);
>           return -1;
>       }
>   
> -    return monitor_read_bdrv_key_start(mon, bs, completion_cb, opaque);
> +    return monitor_read_bdrv_key_start(mon, blk_bs(blk), completion_cb, opaque);
>   }
>   
>   QemuOptsList qemu_mon_opts = {

Reviewed-by: Max Reitz <mreitz@redhat.com>

[Qemu-devel] [PATCH 2/4] migration: Convert bdrv_find to blk_by_name

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 migration/block.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/migration/block.c b/migration/block.c
index 0c76106..085c0fa 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -23,6 +23,7 @@
 #include "migration/block.h"
 #include "migration/migration.h"
 #include "sysemu/blockdev.h"
+#include "sysemu/block-backend.h"
 #include <assert.h>
 
 #define BLOCK_SIZE                       (1 << 20)
@@ -783,6 +784,7 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
     char device_name[256];
     int64_t addr;
     BlockDriverState *bs, *bs_prev = NULL;
+    BlockBackend *blk;
     uint8_t *buf;
     int64_t total_sectors = 0;
     int nr_sectors;
@@ -800,12 +802,13 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
             qemu_get_buffer(f, (uint8_t *)device_name, len);
             device_name[len] = '\0';
 
-            bs = bdrv_find(device_name);
-            if (!bs) {
+            blk = blk_by_name(device_name);
+            if (!blk) {
                 fprintf(stderr, "Error unknown block device %s\n",
                         device_name);
                 return -EINVAL;
             }
+            bs = blk_bs(blk);
 
             if (bs != bs_prev) {
                 bs_prev = bs;
-- 
1.9.3

Re: [Qemu-devel] [PATCH 2/4] migration: Convert bdrv_find to blk_by_name

[Max Reitz]

On 2015-03-02 at 06:36, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>   migration/block.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/migration/block.c b/migration/block.c
> index 0c76106..085c0fa 100644
> --- a/migration/block.c
> +++ b/migration/block.c
> @@ -23,6 +23,7 @@
>   #include "migration/block.h"
>   #include "migration/migration.h"
>   #include "sysemu/blockdev.h"
> +#include "sysemu/block-backend.h"
>   #include <assert.h>
>   
>   #define BLOCK_SIZE                       (1 << 20)
> @@ -783,6 +784,7 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
>       char device_name[256];
>       int64_t addr;
>       BlockDriverState *bs, *bs_prev = NULL;
> +    BlockBackend *blk;
>       uint8_t *buf;
>       int64_t total_sectors = 0;
>       int nr_sectors;
> @@ -800,12 +802,13 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
>               qemu_get_buffer(f, (uint8_t *)device_name, len);
>               device_name[len] = '\0';
>   
> -            bs = bdrv_find(device_name);
> -            if (!bs) {
> +            blk = blk_by_name(device_name);
> +            if (!blk) {
>                   fprintf(stderr, "Error unknown block device %s\n",
>                           device_name);
>                   return -EINVAL;
>               }
> +            bs = blk_bs(blk);
>   
>               if (bs != bs_prev) {
>                   bs_prev = bs;

Hm, okay for a quick stitch. But actually we could get rid of all the 
BDSs in this function (and it should probably be acquiring the 
AioContext of the BB). If you don't feel like it, maybe I'll do it in 
the next iteration of my "BB and media" series (or in the 
bdrv_close_all() series... Both have convenient "Use more BB" patches, 
as far as I remember).

Reviewed-by: Max Reitz <mreitz@redhat.com>

[Qemu-devel] [PATCH 3/4] blockdev: Convert bdrv_find to blk_by_name

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 blockdev.c | 92 ++++++++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 59 insertions(+), 33 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index ae73539..c70d849 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1017,18 +1017,18 @@ fail:
 void hmp_commit(Monitor *mon, const QDict *qdict)
 {
     const char *device = qdict_get_str(qdict, "device");
-    BlockDriverState *bs;
+    BlockBackend *blk;
     int ret;
 
     if (!strcmp(device, "all")) {
         ret = bdrv_commit_all();
     } else {
-        bs = bdrv_find(device);
-        if (!bs) {
+        blk = blk_by_name(device);
+        if (!blk) {
             monitor_printf(mon, "Device '%s' not found\n", device);
             return;
         }
-        ret = bdrv_commit(bs);
+        ret = bdrv_commit(blk_bs(blk));
     }
     if (ret < 0) {
         monitor_printf(mon, "'commit' error for '%s': %s\n", device,
@@ -1093,17 +1093,20 @@ SnapshotInfo *qmp_blockdev_snapshot_delete_internal_sync(const char *device,
                                                          const char *name,
                                                          Error **errp)
 {
-    BlockDriverState *bs = bdrv_find(device);
+    BlockDriverState *bs;
+    BlockBackend *blk;
     AioContext *aio_context;
     QEMUSnapshotInfo sn;
     Error *local_err = NULL;
     SnapshotInfo *info = NULL;
     int ret;
 
-    if (!bs) {
+    blk = blk_by_name(device);
+    if (!blk) {
         error_set(errp, QERR_DEVICE_NOT_FOUND, device);
         return NULL;
     }
+    bs = blk_bs(blk);
 
     if (!has_id) {
         id = NULL;
@@ -1206,6 +1209,7 @@ static void internal_snapshot_prepare(BlkTransactionState *common,
     Error *local_err = NULL;
     const char *device;
     const char *name;
+    BlockBackend *blk;
     BlockDriverState *bs;
     QEMUSnapshotInfo old_sn, *sn;
     bool ret;
@@ -1224,11 +1228,12 @@ static void internal_snapshot_prepare(BlkTransactionState *common,
     name = internal->name;
 
     /* 2. check for validation */
-    bs = bdrv_find(device);
-    if (!bs) {
+    blk = blk_by_name(device);
+    if (!blk) {
         error_set(errp, QERR_DEVICE_NOT_FOUND, device);
         return;
     }
+    bs = blk_bs(blk);
 
     /* AioContext is released in .clean() */
     state->aio_context = bdrv_get_aio_context(bs);
@@ -1495,17 +1500,19 @@ static void drive_backup_prepare(BlkTransactionState *common, Error **errp)
 {
     DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
     BlockDriverState *bs;
+    BlockBackend *blk;
     DriveBackup *backup;
     Error *local_err = NULL;
 
     assert(common->action->kind == TRANSACTION_ACTION_KIND_DRIVE_BACKUP);
     backup = common->action->drive_backup;
 
-    bs = bdrv_find(backup->device);
-    if (!bs) {
+    blk = blk_by_name(backup->device);
+    if (!blk) {
         error_set(errp, QERR_DEVICE_NOT_FOUND, backup->device);
         return;
     }
+    bs = blk_bs(blk);
 
     /* AioContext is released in .clean() */
     state->aio_context = bdrv_get_aio_context(bs);
@@ -1560,22 +1567,25 @@ static void blockdev_backup_prepare(BlkTransactionState *common, Error **errp)
     BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);
     BlockdevBackup *backup;
     BlockDriverState *bs, *target;
+    BlockBackend *blk;
     Error *local_err = NULL;
 
     assert(common->action->kind == TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP);
     backup = common->action->blockdev_backup;
 
-    bs = bdrv_find(backup->device);
-    if (!bs) {
+    blk = blk_by_name(backup->device);
+    if (!blk) {
         error_set(errp, QERR_DEVICE_NOT_FOUND, backup->device);
         return;
     }
+    bs = blk_bs(blk);
 
-    target = bdrv_find(backup->target);
-    if (!target) {
+    blk = blk_by_name(backup->target);
+    if (!blk) {
         error_set(errp, QERR_DEVICE_NOT_FOUND, backup->target);
         return;
     }
+    target = blk_bs(blk);
 
     /* AioContext is released in .clean() */
     state->aio_context = bdrv_get_aio_context(bs);
@@ -1882,13 +1892,15 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
 {
     ThrottleConfig cfg;
     BlockDriverState *bs;
+    BlockBackend *blk;
     AioContext *aio_context;
 
-    bs = bdrv_find(device);
-    if (!bs) {
+    blk = blk_by_name(device);
+    if (!blk) {
         error_set(errp, QERR_DEVICE_NOT_FOUND, device);
         return;
     }
+    bs = blk_bs(blk);
 
     memset(&cfg, 0, sizeof(cfg));
     cfg.buckets[THROTTLE_BPS_TOTAL].avg = bps;
@@ -2092,6 +2104,7 @@ void qmp_block_stream(const char *device,
                       bool has_on_error, BlockdevOnError on_error,
                       Error **errp)
 {
+    BlockBackend *blk;
     BlockDriverState *bs;
     BlockDriverState *base_bs = NULL;
     AioContext *aio_context;
@@ -2102,11 +2115,12 @@ void qmp_block_stream(const char *device,
         on_error = BLOCKDEV_ON_ERROR_REPORT;
     }
 
-    bs = bdrv_find(device);
-    if (!bs) {
+    blk = blk_by_name(device);
+    if (!blk) {
         error_set(errp, QERR_DEVICE_NOT_FOUND, device);
         return;
     }
+    bs = blk_bs(blk);
 
     aio_context = bdrv_get_aio_context(bs);
     aio_context_acquire(aio_context);
@@ -2156,6 +2170,7 @@ void qmp_block_commit(const char *device,
                       bool has_speed, int64_t speed,
                       Error **errp)
 {
+    BlockBackend *blk;
     BlockDriverState *bs;
     BlockDriverState *base_bs, *top_bs;
     AioContext *aio_context;
@@ -2174,11 +2189,12 @@ void qmp_block_commit(const char *device,
      *  live commit feature versions; for this to work, we must make sure to
      *  perform the device lookup before any generic errors that may occur in a
      *  scenario in which all optional arguments are omitted. */
-    bs = bdrv_find(device);
-    if (!bs) {
+    blk = blk_by_name(device);
+    if (!blk) {
         error_set(errp, QERR_DEVICE_NOT_FOUND, device);
         return;
     }
+    bs = blk_bs(blk);
 
     aio_context = bdrv_get_aio_context(bs);
     aio_context_acquire(aio_context);
@@ -2259,6 +2275,7 @@ void qmp_drive_backup(const char *device, const char *target,
                       bool has_on_target_error, BlockdevOnError on_target_error,
                       Error **errp)
 {
+    BlockBackend *blk;
     BlockDriverState *bs;
     BlockDriverState *target_bs;
     BlockDriverState *source = NULL;
@@ -2282,11 +2299,12 @@ void qmp_drive_backup(const char *device, const char *target,
         mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
     }
 
-    bs = bdrv_find(device);
-    if (!bs) {
+    blk = blk_by_name(device);
+    if (!blk) {
         error_set(errp, QERR_DEVICE_NOT_FOUND, device);
         return;
     }
+    bs = blk_bs(blk);
 
     aio_context = bdrv_get_aio_context(bs);
     aio_context_acquire(aio_context);
@@ -2386,6 +2404,7 @@ void qmp_blockdev_backup(const char *device, const char *target,
                          BlockdevOnError on_target_error,
                          Error **errp)
 {
+    BlockBackend *blk;
     BlockDriverState *bs;
     BlockDriverState *target_bs;
     Error *local_err = NULL;
@@ -2401,20 +2420,22 @@ void qmp_blockdev_backup(const char *device, const char *target,
         on_target_error = BLOCKDEV_ON_ERROR_REPORT;
     }
 
-    bs = bdrv_find(device);
-    if (!bs) {
+    blk = blk_by_name(device);
+    if (!blk) {
         error_set(errp, QERR_DEVICE_NOT_FOUND, device);
         return;
     }
+    bs = blk_bs(blk);
 
     aio_context = bdrv_get_aio_context(bs);
     aio_context_acquire(aio_context);
 
-    target_bs = bdrv_find(target);
-    if (!target_bs) {
+    blk = blk_by_name(target);
+    if (!blk) {
         error_set(errp, QERR_DEVICE_NOT_FOUND, target);
         goto out;
     }
+    target_bs = blk_bs(blk);
 
     bdrv_ref(target_bs);
     bdrv_set_aio_context(target_bs, aio_context);
@@ -2443,6 +2464,7 @@ void qmp_drive_mirror(const char *device, const char *target,
                       bool has_on_target_error, BlockdevOnError on_target_error,
                       Error **errp)
 {
+    BlockBackend *blk;
     BlockDriverState *bs;
     BlockDriverState *source, *target_bs;
     AioContext *aio_context;
@@ -2482,11 +2504,12 @@ void qmp_drive_mirror(const char *device, const char *target,
         return;
     }
 
-    bs = bdrv_find(device);
-    if (!bs) {
+    blk = blk_by_name(device);
+    if (!blk) {
         error_set(errp, QERR_DEVICE_NOT_FOUND, device);
         return;
     }
+    bs = blk_bs(blk);
 
     aio_context = bdrv_get_aio_context(bs);
     aio_context_acquire(aio_context);
@@ -2624,12 +2647,14 @@ out:
 static BlockJob *find_block_job(const char *device, AioContext **aio_context,
                                 Error **errp)
 {
+    BlockBackend *blk;
     BlockDriverState *bs;
 
-    bs = bdrv_find(device);
-    if (!bs) {
+    blk = blk_by_name(device);
+    if (!blk) {
         goto notfound;
     }
+    bs = blk_bs(blk);
 
     *aio_context = bdrv_get_aio_context(bs);
     aio_context_acquire(*aio_context);
@@ -2734,6 +2759,7 @@ void qmp_change_backing_file(const char *device,
                              const char *backing_file,
                              Error **errp)
 {
+    BlockBackend *blk;
     BlockDriverState *bs = NULL;
     AioContext *aio_context;
     BlockDriverState *image_bs = NULL;
@@ -2742,12 +2768,12 @@ void qmp_change_backing_file(const char *device,
     int open_flags;
     int ret;
 
-    /* find the top layer BDS of the chain */
-    bs = bdrv_find(device);
-    if (!bs) {
+    blk = blk_by_name(device);
+    if (!blk) {
         error_set(errp, QERR_DEVICE_NOT_FOUND, device);
         return;
     }
+    bs = blk_bs(blk);
 
     aio_context = bdrv_get_aio_context(bs);
     aio_context_acquire(aio_context);
-- 
1.9.3

Re: [Qemu-devel] [PATCH 3/4] blockdev: Convert bdrv_find to blk_by_name

[Max Reitz]

On 2015-03-02 at 06:36, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>   blockdev.c | 92 ++++++++++++++++++++++++++++++++++++++++----------------------
>   1 file changed, 59 insertions(+), 33 deletions(-)

I think I touched the same problem in my "BB and media" series, and I 
think I replaced the bdrv_get_aio_context() calls by 
blk_get_aio_context() (because it looked nicer to me to call blk_bs() 
only after the AIO context has been acquired).

However, at least before that series it is not possible to change the 
BDS attached to a BB while the BB is already in use in another AIO 
context, so:

Reviewed-by: Max Reitz <mreitz@redhat.com>

[Qemu-devel] [PATCH 4/4] block: Drop bdrv_find

[Fam Zheng]

All callers are converted, so drop it.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c               | 9 ---------
 include/block/block.h | 1 -
 2 files changed, 10 deletions(-)

diff --git a/block.c b/block.c
index 9b707e3..742f82c 100644
--- a/block.c
+++ b/block.c
@@ -3780,15 +3780,6 @@ void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
     g_free(formats);
 }
 
-/* This function is to find block backend bs */
-/* TODO convert callers to blk_by_name(), then remove */
-BlockDriverState *bdrv_find(const char *name)
-{
-    BlockBackend *blk = blk_by_name(name);
-
-    return blk ? blk_bs(blk) : NULL;
-}
-
 /* This function is to find a node in the bs graph */
 BlockDriverState *bdrv_find_node(const char *node_name)
 {
diff --git a/include/block/block.h b/include/block/block.h
index 471d11d..2c121a5 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -370,7 +370,6 @@ int bdrv_media_changed(BlockDriverState *bs);
 void bdrv_lock_medium(BlockDriverState *bs, bool locked);
 void bdrv_eject(BlockDriverState *bs, bool eject_flag);
 const char *bdrv_get_format_name(BlockDriverState *bs);
-BlockDriverState *bdrv_find(const char *name);
 BlockDriverState *bdrv_find_node(const char *node_name);
 BlockDeviceInfoList *bdrv_named_nodes_list(void);
 BlockDriverState *bdrv_lookup_bs(const char *device,
-- 
1.9.3

Re: [Qemu-devel] [PATCH 4/4] block: Drop bdrv_find

[Max Reitz]

On 2015-03-02 at 06:36, Fam Zheng wrote:
> All callers are converted, so drop it.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>   block.c               | 9 ---------
>   include/block/block.h | 1 -
>   2 files changed, 10 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

Re: [Qemu-devel] [PATCH 0/4] block: Convert bdrv_find to blk_by_name and drop it

[Max Reitz]

On 2015-03-02 at 06:36, Fam Zheng wrote:
> This is a small step towards a more complete separation from BlockDriverState
> to block backend users.
>
> Fam Zheng (4):
>    monitor: Convert bdrv_find to blk_by_name
>    migration: Convert bdrv_find to blk_by_name
>    blockdev: Convert bdrv_find to blk_by_name
>    block: Drop bdrv_find
>
>   block.c               |  9 -----
>   blockdev.c            | 92 +++++++++++++++++++++++++++++++++------------------
>   include/block/block.h |  1 -
>   migration/block.c     |  7 ++--
>   monitor.c             |  9 ++---
>   5 files changed, 69 insertions(+), 49 deletions(-)

Thanks, applied to my block tree:

https://github.com/XanClic/qemu/commits/block