Re: [Qemu-block] [PATCH v5 3/6] block: Add bdrv_filename()

2015-10-26 Thread Kevin Wolf
Am 19.10.2015 um 20:49 hat Max Reitz geschrieben:
> Split the part which actually refreshes the BlockDriverState.filename
> field off of bdrv_refresh_filename() into a more generic function
> bdrv_filename(), which first calls bdrv_refresh_filename() and then
> stores a qemu-usable filename in the given buffer instead of
> BlockDriverState.filename.
> 
> Since bdrv_refresh_filename() therefore no longer refreshes that field,
> some calls to that function have to be replaced by calls to
> bdrv_filename() "manually" refreshing the BDS filename field (this is
> only temporary).
> 
> Signed-off-by: Max Reitz 

Reviewed-by: Kevin Wolf 



Re: [Qemu-block] [PATCH v5 3/6] block: Add bdrv_filename()

2015-10-19 Thread Eric Blake
On 10/19/2015 12:49 PM, Max Reitz wrote:
> Split the part which actually refreshes the BlockDriverState.filename
> field off of bdrv_refresh_filename() into a more generic function
> bdrv_filename(), which first calls bdrv_refresh_filename() and then
> stores a qemu-usable filename in the given buffer instead of
> BlockDriverState.filename.
> 
> Since bdrv_refresh_filename() therefore no longer refreshes that field,
> some calls to that function have to be replaced by calls to
> bdrv_filename() "manually" refreshing the BDS filename field (this is
> only temporary).
> 
> Signed-off-by: Max Reitz 
> ---
>  block.c   | 43 ---
>  block/blkverify.c |  3 ++-
>  block/quorum.c|  3 ++-
>  include/block/block.h |  1 +
>  4 files changed, 41 insertions(+), 9 deletions(-)
> 

> +
> +/* First refreshes exact_filename and full_open_options by calling
> + * bdrv_refresh_filename(). Then, if exact_filename is set, it is copied into
> + * the target buffer. Otherwise, full_open_options is converted to a JSON
> + * object, prefixed with "json:" (for use through the JSON pseudo protocol) 
> and
> + * put there.
> + *
> + * If @sz > 0, the string put into the buffer will always be null-terminated.

Maybe add a warning:

...always be null-terminated, even if this truncates the result.

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature


[Qemu-block] [PATCH v5 3/6] block: Add bdrv_filename()

2015-10-19 Thread Max Reitz
Split the part which actually refreshes the BlockDriverState.filename
field off of bdrv_refresh_filename() into a more generic function
bdrv_filename(), which first calls bdrv_refresh_filename() and then
stores a qemu-usable filename in the given buffer instead of
BlockDriverState.filename.

Since bdrv_refresh_filename() therefore no longer refreshes that field,
some calls to that function have to be replaced by calls to
bdrv_filename() "manually" refreshing the BDS filename field (this is
only temporary).

Signed-off-by: Max Reitz 
---
 block.c   | 43 ---
 block/blkverify.c |  3 ++-
 block/quorum.c|  3 ++-
 include/block/block.h |  1 +
 4 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/block.c b/block.c
index 0f08d7c..1061d80 100644
--- a/block.c
+++ b/block.c
@@ -1527,7 +1527,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
const char *filename,
 }
 }
 
-bdrv_refresh_filename(bs);
+bdrv_filename(bs, bs->filename, sizeof(bs->filename));
 
 /* Check if any unknown options were used */
 if (options && (qdict_size(options) != 0)) {
@@ -3937,9 +3937,6 @@ static bool append_open_options(QDict *d, 
BlockDriverState *bs)
  *  - full_open_options: Options which, when given when opening a block device
  *   (without a filename), result in a BDS (mostly)
  *   equalling the given one
- *  - filename: If exact_filename is set, it is copied here. Otherwise,
- *  full_open_options is converted to a JSON object, prefixed with
- *  "json:" (for use through the JSON pseudo protocol) and put 
here.
  */
 void bdrv_refresh_filename(BlockDriverState *bs)
 {
@@ -4027,13 +4024,45 @@ void bdrv_refresh_filename(BlockDriverState *bs)
 
 bs->full_open_options = opts;
 }
+}
+
+/* First refreshes exact_filename and full_open_options by calling
+ * bdrv_refresh_filename(). Then, if exact_filename is set, it is copied into
+ * the target buffer. Otherwise, full_open_options is converted to a JSON
+ * object, prefixed with "json:" (for use through the JSON pseudo protocol) and
+ * put there.
+ *
+ * If @sz > 0, the string put into the buffer will always be null-terminated.
+ *
+ * If @dest is NULL, @sz is ignored and a new buffer will be allocated which is
+ * large enough to hold the filename and the trailing '\0'. This buffer is then
+ * returned and has to be freed by the caller when it is no longer needed.
+ *
+ * Returns @dest if it is not NULL, and the newly allocated buffer otherwise.
+ */
+char *bdrv_filename(BlockDriverState *bs, char *dest, size_t sz)
+{
+bdrv_refresh_filename(bs);
+
+if (sz > INT_MAX) {
+sz = INT_MAX;
+}
 
 if (bs->exact_filename[0]) {
-pstrcpy(bs->filename, sizeof(bs->filename), bs->exact_filename);
+if (dest) {
+pstrcpy(dest, sz, bs->exact_filename);
+} else {
+dest = g_strdup(bs->exact_filename);
+}
 } else if (bs->full_open_options) {
 QString *json = qobject_to_json(QOBJECT(bs->full_open_options));
-snprintf(bs->filename, sizeof(bs->filename), "json:%s",
- qstring_get_str(json));
+if (dest) {
+snprintf(dest, sz, "json:%s", qstring_get_str(json));
+} else {
+dest = g_strdup_printf("json:%s", qstring_get_str(json));
+}
 QDECREF(json);
 }
+
+return dest;
 }
diff --git a/block/blkverify.c b/block/blkverify.c
index c5f8e8d..3af37f8 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -312,7 +312,8 @@ static void blkverify_refresh_filename(BlockDriverState *bs)
 BDRVBlkverifyState *s = bs->opaque;
 
 /* bs->file->bs has already been refreshed */
-bdrv_refresh_filename(s->test_file->bs);
+bdrv_filename(s->test_file->bs, s->test_file->bs->filename,
+  sizeof(s->test_file->bs->filename));
 
 if (bs->file->bs->full_open_options
 && s->test_file->bs->full_open_options)
diff --git a/block/quorum.c b/block/quorum.c
index b9ba028..16ec188 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -1005,7 +1005,8 @@ static void quorum_refresh_filename(BlockDriverState *bs)
 int i;
 
 for (i = 0; i < s->num_children; i++) {
-bdrv_refresh_filename(s->children[i]->bs);
+bdrv_filename(s->children[i]->bs, s->children[i]->bs->filename,
+  sizeof(s->children[i]->bs->filename));
 if (!s->children[i]->bs->full_open_options) {
 return;
 }
diff --git a/include/block/block.h b/include/block/block.h
index 340c00f..9273e62 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -263,6 +263,7 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState 
*bs,
 const char *backing_file);
 int bdrv_get_backing_file_depth(BlockDriverState *bs);
 void bdrv_refresh_filename(BlockDriverState *bs);
+char *bdrv_filename(BlockDriverState *bs, char *d