On Thu, Nov 07, 2013 at 04:01:42PM +0100, Benoît Canet wrote:
> Add the minimum of code to prepare the followings patches.
>
> If no node_name is provided to bdrv_new the bs->node_name is set to
> "undefined".
> This will allow to have some default string to communicate in QMP and HMP.
> This also make "undefined" a reserved string for bs->node_name.
Hi Benoît,
Is it necessary to have a reserved string, or would an empty
null-terminated string be able to implicitly denote the name as
undefined?
>
> Signed-off-by: Benoit Canet <ben...@irqsave.net>
> ---
> block.c | 70
> +++++++++++++++++++++++++++++++++++------------
> block/blkverify.c | 2 +-
> block/iscsi.c | 2 +-
> block/vmdk.c | 2 +-
> block/vvfat.c | 4 +--
> blockdev.c | 8 +++---
> hw/block/xen_disk.c | 2 +-
> include/block/block.h | 3 +-
> include/block/block_int.h | 9 +++++-
> qemu-img.c | 6 ++--
> qemu-io.c | 2 +-
> qemu-nbd.c | 2 +-
> 12 files changed, 77 insertions(+), 35 deletions(-)
>
> diff --git a/block.c b/block.c
> index fd05a80..230e71a 100644
> --- a/block.c
> +++ b/block.c
> @@ -89,6 +89,9 @@ static int coroutine_fn
> bdrv_co_do_write_zeroes(BlockDriverState *bs,
> static QTAILQ_HEAD(, BlockDriverState) bdrv_states =
> QTAILQ_HEAD_INITIALIZER(bdrv_states);
>
> +static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states =
> + QTAILQ_HEAD_INITIALIZER(graph_bdrv_states);
> +
> static QLIST_HEAD(, BlockDriver) bdrv_drivers =
> QLIST_HEAD_INITIALIZER(bdrv_drivers);
>
> @@ -318,14 +321,26 @@ void bdrv_register(BlockDriver *bdrv)
> }
>
> /* create a new block device (by default it is empty) */
> -BlockDriverState *bdrv_new(const char *device_name)
> +BlockDriverState *bdrv_new(const char *device_name, const char *node_name)
> {
> BlockDriverState *bs;
>
> bs = g_malloc0(sizeof(BlockDriverState));
> pstrcpy(bs->device_name, sizeof(bs->device_name), device_name);
> if (device_name[0] != '\0') {
> - QTAILQ_INSERT_TAIL(&bdrv_states, bs, list);
> + QTAILQ_INSERT_TAIL(&bdrv_states, bs, device_list);
> + }
> + /* if node name is given store it in bs and insert bs in the graph bs
> list
> + * note: undefined is a reserved node name
> + */
> + if (node_name &&
> + node_name[0] != '\0' &&
> + strcmp(node_name, "undefined")) {
> + pstrcpy(bs->node_name, sizeof(bs->node_name), node_name);
> + QTAILQ_INSERT_TAIL(&graph_bdrv_states, bs, node_list);
> + /* else set the bs node name to undefined for QMP and HMP */
> + } else {
> + sprintf(bs->node_name, "undefined");
> }
> bdrv_iostatus_disable(bs);
> notifier_list_init(&bs->close_notifiers);
> @@ -870,7 +885,7 @@ int bdrv_file_open(BlockDriverState **pbs, const char
> *filename,
> options = qdict_new();
> }
>
> - bs = bdrv_new("");
> + bs = bdrv_new("", NULL);
> bs->options = options;
> options = qdict_clone_shallow(options);
>
> @@ -992,7 +1007,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict
> *options, Error **errp)
> sizeof(backing_filename));
> }
>
> - bs->backing_hd = bdrv_new("");
> + bs->backing_hd = bdrv_new("", NULL);
>
> if (bs->backing_format[0] != '\0') {
> back_drv = bdrv_find_format(bs->backing_format);
> @@ -1062,7 +1077,7 @@ int bdrv_open(BlockDriverState *bs, const char
> *filename, QDict *options,
> instead of opening 'filename' directly */
>
> /* if there is a backing file, use it */
> - bs1 = bdrv_new("");
> + bs1 = bdrv_new("", NULL);
> ret = bdrv_open(bs1, filename, NULL, 0, drv, &local_err);
> if (ret < 0) {
> bdrv_unref(bs1);
> @@ -1495,7 +1510,7 @@ void bdrv_close_all(void)
> {
> BlockDriverState *bs;
>
> - QTAILQ_FOREACH(bs, &bdrv_states, list) {
> + QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
> bdrv_close(bs);
> }
> }
> @@ -1524,7 +1539,7 @@ static bool bdrv_requests_pending(BlockDriverState *bs)
> static bool bdrv_requests_pending_all(void)
> {
> BlockDriverState *bs;
> - QTAILQ_FOREACH(bs, &bdrv_states, list) {
> + QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
> if (bdrv_requests_pending(bs)) {
> return true;
> }
> @@ -1554,7 +1569,7 @@ void bdrv_drain_all(void)
> /* FIXME: We do not have timer support here, so this is effectively
> * a busy wait.
> */
> - QTAILQ_FOREACH(bs, &bdrv_states, list) {
> + QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
> if (bdrv_start_throttled_reqs(bs)) {
> busy = true;
> }
> @@ -1570,7 +1585,7 @@ void bdrv_drain_all(void)
> void bdrv_make_anon(BlockDriverState *bs)
> {
> if (bs->device_name[0] != '\0') {
> - QTAILQ_REMOVE(&bdrv_states, bs, list);
> + QTAILQ_REMOVE(&bdrv_states, bs, device_list);
> }
> bs->device_name[0] = '\0';
Do you need to do anything here to remove the BDS from your graph list?
e.g. QTAILQ_REMOVE(&graph_bdrv_states, bs, node_list)
> }
> @@ -1626,7 +1641,12 @@ static void bdrv_move_feature_fields(BlockDriverState
> *bs_dest,
> /* keep the same entry in bdrv_states */
> pstrcpy(bs_dest->device_name, sizeof(bs_dest->device_name),
> bs_src->device_name);
> - bs_dest->list = bs_src->list;
> + bs_dest->device_list = bs_src->device_list;
> +
> + /* keep the same entry in graph_bdrv_states */
> + pstrcpy(bs_dest->node_name, sizeof(bs_dest->node_name),
> + bs_src->node_name);
> + bs_dest->node_list = bs_src->node_list;
> }
>
> /*
> @@ -1950,7 +1970,7 @@ int bdrv_commit_all(void)
> {
> BlockDriverState *bs;
>
> - QTAILQ_FOREACH(bs, &bdrv_states, list) {
> + QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
> if (bs->drv && bs->backing_hd) {
> int ret = bdrv_commit(bs);
> if (ret < 0) {
> @@ -3017,11 +3037,12 @@ void bdrv_iterate_format(void (*it)(void *opaque,
> const char *name),
> }
> }
>
> +/* This function is to find block backend bs */
> BlockDriverState *bdrv_find(const char *name)
> {
> BlockDriverState *bs;
>
> - QTAILQ_FOREACH(bs, &bdrv_states, list) {
> + QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
> if (!strcmp(name, bs->device_name)) {
> return bs;
> }
> @@ -3029,19 +3050,32 @@ BlockDriverState *bdrv_find(const char *name)
> return NULL;
> }
>
> +/* This function is to find a node in the bs graph */
> +BlockDriverState *bdrv_find_node(const char *node_name)
> +{
> + BlockDriverState *bs;
> +
> + QTAILQ_FOREACH(bs, &graph_bdrv_states, node_list) {
> + if (!strcmp(node_name, bs->node_name)) {
> + return bs;
> + }
> + }
> + return NULL;
> +}
> +
> BlockDriverState *bdrv_next(BlockDriverState *bs)
> {
> if (!bs) {
> return QTAILQ_FIRST(&bdrv_states);
> }
> - return QTAILQ_NEXT(bs, list);
> + return QTAILQ_NEXT(bs, device_list);
> }
>
> void bdrv_iterate(void (*it)(void *opaque, BlockDriverState *bs), void
> *opaque)
> {
> BlockDriverState *bs;
>
> - QTAILQ_FOREACH(bs, &bdrv_states, list) {
> + QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
> it(opaque, bs);
> }
> }
> @@ -3061,7 +3095,7 @@ int bdrv_flush_all(void)
> BlockDriverState *bs;
> int result = 0;
>
> - QTAILQ_FOREACH(bs, &bdrv_states, list) {
> + QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
> int ret = bdrv_flush(bs);
> if (ret < 0 && !result) {
> result = ret;
> @@ -4127,7 +4161,7 @@ void bdrv_invalidate_cache_all(void)
> {
> BlockDriverState *bs;
>
> - QTAILQ_FOREACH(bs, &bdrv_states, list) {
> + QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
> bdrv_invalidate_cache(bs);
> }
> }
> @@ -4136,7 +4170,7 @@ void bdrv_clear_incoming_migration_all(void)
> {
> BlockDriverState *bs;
>
> - QTAILQ_FOREACH(bs, &bdrv_states, list) {
> + QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
> bs->open_flags = bs->open_flags & ~(BDRV_O_INCOMING);
> }
> }
> @@ -4582,7 +4616,7 @@ void bdrv_img_create(const char *filename, const char
> *fmt,
> back_flags =
> flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
>
> - bs = bdrv_new("");
> + bs = bdrv_new("", NULL);
>
> ret = bdrv_open(bs, backing_file->value.s, NULL, back_flags,
> backing_drv, &local_err);
> diff --git a/block/blkverify.c b/block/blkverify.c
> index 55819a0..674b6a5 100644
> --- a/block/blkverify.c
> +++ b/block/blkverify.c
> @@ -155,7 +155,7 @@ static int blkverify_open(BlockDriverState *bs, QDict
> *options, int flags,
> goto fail;
> }
>
> - s->test_file = bdrv_new("");
> + s->test_file = bdrv_new("", NULL);
> ret = bdrv_open(s->test_file, filename, NULL, flags, NULL, &local_err);
> if (ret < 0) {
> error_propagate(errp, local_err);
> diff --git a/block/iscsi.c b/block/iscsi.c
> index a2a961e..5031593 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -1461,7 +1461,7 @@ static int iscsi_create(const char *filename,
> QEMUOptionParameter *options,
> IscsiLun *iscsilun = NULL;
> QDict *bs_options;
>
> - bs = bdrv_new("");
> + bs = bdrv_new("", NULL);
>
> /* Read out options */
> while (options && options->name) {
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 32ec8b77..97801c2 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -1672,7 +1672,7 @@ static int vmdk_create(const char *filename,
> QEMUOptionParameter *options,
> return -ENOTSUP;
> }
> if (backing_file) {
> - BlockDriverState *bs = bdrv_new("");
> + BlockDriverState *bs = bdrv_new("", NULL);
> ret = bdrv_open(bs, backing_file, NULL, 0, NULL, errp);
> if (ret != 0) {
> bdrv_unref(bs);
> diff --git a/block/vvfat.c b/block/vvfat.c
> index 3ddaa0b..a8b6011 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -2935,7 +2935,7 @@ static int enable_write_target(BDRVVVFATState *s)
> goto err;
> }
>
> - s->qcow = bdrv_new("");
> + s->qcow = bdrv_new("", NULL);
>
> ret = bdrv_open(s->qcow, s->qcow_filename, NULL,
> BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, bdrv_qcow,
> @@ -2951,7 +2951,7 @@ static int enable_write_target(BDRVVVFATState *s)
> unlink(s->qcow_filename);
> #endif
>
> - s->bs->backing_hd = bdrv_new("");
> + s->bs->backing_hd = bdrv_new("", NULL);
> s->bs->backing_hd->drv = &vvfat_write_target;
> s->bs->backing_hd->opaque = g_malloc(sizeof(void*));
> *(void**)s->bs->backing_hd->opaque = s;
> diff --git a/blockdev.c b/blockdev.c
> index b260477..ac47413 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -469,7 +469,7 @@ static DriveInfo *blockdev_init(QDict *bs_opts,
> /* init */
> dinfo = g_malloc0(sizeof(*dinfo));
> dinfo->id = g_strdup(qemu_opts_id(opts));
> - dinfo->bdrv = bdrv_new(dinfo->id);
> + dinfo->bdrv = bdrv_new(dinfo->id, NULL);
> dinfo->bdrv->open_flags = snapshot ? BDRV_O_SNAPSHOT : 0;
> dinfo->bdrv->read_only = ro;
> dinfo->type = type;
> @@ -1254,7 +1254,7 @@ static void
> external_snapshot_prepare(BlkTransactionState *common,
> }
>
> /* We will manually add the backing_hd field to the bs later */
> - state->new_bs = bdrv_new("");
> + state->new_bs = bdrv_new("", NULL);
> /* TODO Inherit bs->options or only take explicit options with an
> * extended QMP command? */
> ret = bdrv_open(state->new_bs, new_image_file, NULL,
> @@ -1921,7 +1921,7 @@ void qmp_drive_backup(const char *device, const char
> *target,
> return;
> }
>
> - target_bs = bdrv_new("");
> + target_bs = bdrv_new("", NULL);
> ret = bdrv_open(target_bs, target, NULL, flags, drv, &local_err);
> if (ret < 0) {
> bdrv_unref(target_bs);
> @@ -2055,7 +2055,7 @@ void qmp_drive_mirror(const char *device, const char
> *target,
> /* Mirroring takes care of copy-on-write using the source's backing
> * file.
> */
> - target_bs = bdrv_new("");
> + target_bs = bdrv_new("", NULL);
> ret = bdrv_open(target_bs, target, NULL, flags | BDRV_O_NO_BACKING, drv,
> &local_err);
> if (ret < 0) {
> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> index 098f6c6..d89e025 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -808,7 +808,7 @@ static int blk_connect(struct XenDevice *xendev)
> if (!blkdev->dinfo) {
> /* setup via xenbus -> create new block driver instance */
> xen_be_printf(&blkdev->xendev, 2, "create new bdrv (xenbus
> setup)\n");
> - blkdev->bs = bdrv_new(blkdev->dev);
> + blkdev->bs = bdrv_new(blkdev->dev, NULL);
> if (blkdev->bs) {
> Error *local_err = NULL;
> BlockDriver *drv =
> bdrv_find_whitelisted_format(blkdev->fileproto,
> diff --git a/include/block/block.h b/include/block/block.h
> index 3560deb..2d27bd9 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -149,7 +149,7 @@ int bdrv_create(BlockDriver *drv, const char* filename,
> QEMUOptionParameter *options, Error **errp);
> int bdrv_create_file(const char* filename, QEMUOptionParameter *options,
> Error **errp);
> -BlockDriverState *bdrv_new(const char *device_name);
> +BlockDriverState *bdrv_new(const char *device_name, const char *node_name);
> void bdrv_make_anon(BlockDriverState *bs);
> void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old);
> void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top);
> @@ -339,6 +339,7 @@ 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);
> BlockDriverState *bdrv_next(BlockDriverState *bs);
> void bdrv_iterate(void (*it)(void *opaque, BlockDriverState *bs),
> void *opaque);
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index a48731d..9e44136 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -297,11 +297,18 @@ struct BlockDriverState {
> BlockdevOnError on_read_error, on_write_error;
> bool iostatus_enabled;
> BlockDeviceIoStatus iostatus;
> +
> + /* the following member give a name to every node on the BlockDriverState
> + * graph.
> + */
> + char node_name[32];
> + QTAILQ_ENTRY(BlockDriverState) node_list;
> + /* Device name is the name associated with the "drive" the guest see */
> char device_name[32];
> + QTAILQ_ENTRY(BlockDriverState) device_list;
> HBitmap *dirty_bitmap;
> int refcnt;
> int in_use; /* users other than guest access, eg. block migration */
> - QTAILQ_ENTRY(BlockDriverState) list;
>
> QLIST_HEAD(, BdrvTrackedRequest) tracked_requests;
>
> diff --git a/qemu-img.c b/qemu-img.c
> index 926f0a0..215b7b2 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -269,7 +269,7 @@ static BlockDriverState *bdrv_new_open(const char
> *filename,
> Error *local_err = NULL;
> int ret;
>
> - bs = bdrv_new("image");
> + bs = bdrv_new("image", NULL);
>
> if (fmt) {
> drv = bdrv_find_format(fmt);
> @@ -2225,7 +2225,7 @@ static int img_rebase(int argc, char **argv)
> } else {
> char backing_name[1024];
>
> - bs_old_backing = bdrv_new("old_backing");
> + bs_old_backing = bdrv_new("old_backing", NULL);
> bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name));
> ret = bdrv_open(bs_old_backing, backing_name, NULL, BDRV_O_FLAGS,
> old_backing_drv, &local_err);
> @@ -2236,7 +2236,7 @@ static int img_rebase(int argc, char **argv)
> goto out;
> }
> if (out_baseimg[0]) {
> - bs_new_backing = bdrv_new("new_backing");
> + bs_new_backing = bdrv_new("new_backing", NULL);
> ret = bdrv_open(bs_new_backing, out_baseimg, NULL, BDRV_O_FLAGS,
> new_backing_drv, &local_err);
> if (ret) {
> diff --git a/qemu-io.c b/qemu-io.c
> index 3b3340a..3e1ea88 100644
> --- a/qemu-io.c
> +++ b/qemu-io.c
> @@ -63,7 +63,7 @@ static int openfile(char *name, int flags, int growable,
> QDict *opts)
> return 1;
> }
> } else {
> - qemuio_bs = bdrv_new("hda");
> + qemuio_bs = bdrv_new("hda", NULL);
>
> if (bdrv_open(qemuio_bs, name, opts, flags, NULL, &local_err) < 0) {
> fprintf(stderr, "%s: can't open device %s: %s\n", progname, name,
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index c26c98e..35ef57c 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -572,7 +572,7 @@ int main(int argc, char **argv)
> drv = NULL;
> }
>
> - bs = bdrv_new("hda");
> + bs = bdrv_new("hda", NULL);
> srcpath = argv[optind];
> ret = bdrv_open(bs, srcpath, NULL, flags, drv, &local_err);
> if (ret < 0) {
> --
> 1.8.3.2
>
>
