Message ID | 1487006583-24350-23-git-send-email-kwolf@redhat.com |
---|---|
State | New |
Headers | show |
On 13.02.2017 18:22, Kevin Wolf wrote: > For meaningful error messages in the permission system, we want to allow > the parent of a BdrvChild to generate some kind of human-readable > identifier for the link represented by the BdrvChild. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > block.c | 8 ++++++++ > block/block-backend.c | 20 ++++++++++++++++++++ > include/block/block_int.h | 4 ++++ > 3 files changed, 32 insertions(+) > > diff --git a/block.c b/block.c > index fd06549..2116542 100644 > --- a/block.c > +++ b/block.c > @@ -800,6 +800,13 @@ const BdrvChildRole child_format = { > .drained_end = bdrv_child_cb_drained_end, > }; > > +static char *bdrv_backing_link_name(BdrvChild *c) > +{ > + BlockDriverState *parent = c->opaque; > + return g_strdup_printf("backing file link from %s", > + bdrv_get_device_or_node_name(parent)); So in this case it's really about the edge in the BDS graph. > +} > + > /* > * Returns the options and flags that bs->backing should get, based on the > * given options and flags for the parent BDS > @@ -825,6 +832,7 @@ static void bdrv_backing_options(int *child_flags, QDict *child_options, > } > > static const BdrvChildRole child_backing = { > + .get_link_name = bdrv_backing_link_name, > .inherit_options = bdrv_backing_options, > .drained_begin = bdrv_child_cb_drained_begin, > .drained_end = bdrv_child_cb_drained_end, > diff --git a/block/block-backend.c b/block/block-backend.c > index 0c23add..a314284 100644 > --- a/block/block-backend.c > +++ b/block/block-backend.c > @@ -80,6 +80,7 @@ static const AIOCBInfo block_backend_aiocb_info = { > > static void drive_info_del(DriveInfo *dinfo); > static BlockBackend *bdrv_first_blk(BlockDriverState *bs); > +static char *blk_get_attached_dev_id(BlockBackend *blk); > > /* All BlockBackends */ > static QTAILQ_HEAD(, BlockBackend) block_backends = > @@ -102,6 +103,24 @@ static void blk_root_drained_end(BdrvChild *child); > static void blk_root_change_media(BdrvChild *child, bool load); > static void blk_root_resize(BdrvChild *child); > > +static char *blk_root_get_link_name(BdrvChild *child) > +{ > + BlockBackend *blk = child->opaque; > + char *dev_id; > + > + if (blk->name) { > + return g_strdup(blk->name); This is considering the BB to be an edge in the BDS graph. I find that a bit weird, and the fact that it makes complete and total sense after thinking for some time doesn't change the fact that it is weird. > + } > + > + dev_id = blk_get_attached_dev_id(blk); > + if (*dev_id) { > + return dev_id; But the device model OTOH is definitely not an edge in the BDS graph. > + } else { > + g_free(dev_id); > + return g_strdup("a block device"); > + } > +} > + > static const char *blk_root_get_name(BdrvChild *child) > { > return blk_name(child->opaque); [...] Also, I'm curious what you are going to choose as a replacement if the function pointer is not set. I have a couple of ideas, but most of them are either not correct or would remove the need for bdrv_backing_link_name()... Well, I'm curious. :-) Max
diff --git a/block.c b/block.c index fd06549..2116542 100644 --- a/block.c +++ b/block.c @@ -800,6 +800,13 @@ const BdrvChildRole child_format = { .drained_end = bdrv_child_cb_drained_end, }; +static char *bdrv_backing_link_name(BdrvChild *c) +{ + BlockDriverState *parent = c->opaque; + return g_strdup_printf("backing file link from %s", + bdrv_get_device_or_node_name(parent)); +} + /* * Returns the options and flags that bs->backing should get, based on the * given options and flags for the parent BDS @@ -825,6 +832,7 @@ static void bdrv_backing_options(int *child_flags, QDict *child_options, } static const BdrvChildRole child_backing = { + .get_link_name = bdrv_backing_link_name, .inherit_options = bdrv_backing_options, .drained_begin = bdrv_child_cb_drained_begin, .drained_end = bdrv_child_cb_drained_end, diff --git a/block/block-backend.c b/block/block-backend.c index 0c23add..a314284 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -80,6 +80,7 @@ static const AIOCBInfo block_backend_aiocb_info = { static void drive_info_del(DriveInfo *dinfo); static BlockBackend *bdrv_first_blk(BlockDriverState *bs); +static char *blk_get_attached_dev_id(BlockBackend *blk); /* All BlockBackends */ static QTAILQ_HEAD(, BlockBackend) block_backends = @@ -102,6 +103,24 @@ static void blk_root_drained_end(BdrvChild *child); static void blk_root_change_media(BdrvChild *child, bool load); static void blk_root_resize(BdrvChild *child); +static char *blk_root_get_link_name(BdrvChild *child) +{ + BlockBackend *blk = child->opaque; + char *dev_id; + + if (blk->name) { + return g_strdup(blk->name); + } + + dev_id = blk_get_attached_dev_id(blk); + if (*dev_id) { + return dev_id; + } else { + g_free(dev_id); + return g_strdup("a block device"); + } +} + static const char *blk_root_get_name(BdrvChild *child) { return blk_name(child->opaque); @@ -113,6 +132,7 @@ static const BdrvChildRole child_root = { .change_media = blk_root_change_media, .resize = blk_root_resize, .get_name = blk_root_get_name, + .get_link_name = blk_root_get_link_name, .drained_begin = blk_root_drained_begin, .drained_end = blk_root_drained_end, diff --git a/include/block/block_int.h b/include/block/block_int.h index 7558f99..a5557b1 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -438,6 +438,10 @@ struct BdrvChildRole { * name), or NULL if the parent can't provide a better name. */ const char* (*get_name)(BdrvChild *child); + /* Returns a malloced name that describes the link from the parent to the + * child. The caller is responsible for freeing the memory. */ + char* (*get_link_name)(BdrvChild *child); + /* * If this pair of functions is implemented, the parent doesn't issue new * requests after returning from .drained_begin() until .drained_end() is
For meaningful error messages in the permission system, we want to allow the parent of a BdrvChild to generate some kind of human-readable identifier for the link represented by the BdrvChild. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block.c | 8 ++++++++ block/block-backend.c | 20 ++++++++++++++++++++ include/block/block_int.h | 4 ++++ 3 files changed, 32 insertions(+)