block: Make bdrv_img_create() size selection easier to read

[Qemu-devel] [PATCH] block: Make bdrv_img_create() size selection easier to read

Posted by Eric Blake 6 years, 9 months ago

All callers of bdrv_img_create() pass in a size, or -1 to read the
size from the backing file.  We then set that size as the QemuOpt
default, which means we will reuse that default rather than the
final parameter to qemu_opt_get_size() several lines later.  But
it is rather confusing to read subsequent checks of 'size == -1'
when it looks (without seeing the full context) like size defaults
to 0; it also doesn't help that a size of 0 is valid (for some
formats).

Rework the logic to make things more legible.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 2dd9262cd0..57195ec40f 100644
--- a/block.c
+++ b/block.c
@@ -4398,7 +4398,7 @@ void bdrv_img_create(const char *filename, const char *fmt,

     /* The size for the image must always be specified, unless we have a backing
      * file and we have not been forbidden from opening it. */
-    size = qemu_opt_get_size(opts, BLOCK_OPT_SIZE, 0);
+    size = qemu_opt_get_size(opts, BLOCK_OPT_SIZE, img_size);
     if (backing_file && !(flags & BDRV_O_NO_BACKING)) {
         BlockDriverState *bs;
         char *full_backing = g_new0(char, PATH_MAX);
-- 
2.13.3

Re: [Qemu-devel] [PATCH] block: Make bdrv_img_create() size selection easier to read

Posted by John Snow 6 years, 8 months ago


On 07/18/2017 12:00 PM, Eric Blake wrote:
> All callers of bdrv_img_create() pass in a size, or -1 to read the
> size from the backing file.  We then set that size as the QemuOpt
> default, which means we will reuse that default rather than the
> final parameter to qemu_opt_get_size() several lines later.  But
> it is rather confusing to read subsequent checks of 'size == -1'
> when it looks (without seeing the full context) like size defaults
> to 0; it also doesn't help that a size of 0 is valid (for some
> formats).
> 
> Rework the logic to make things more legible.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>

Reviewed-by: John Snow <jsnow@redhat.com>

> ---
>   block.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block.c b/block.c
> index 2dd9262cd0..57195ec40f 100644
> --- a/block.c
> +++ b/block.c
> @@ -4398,7 +4398,7 @@ void bdrv_img_create(const char *filename, const char *fmt,
> 
>       /* The size for the image must always be specified, unless we have a backing
>        * file and we have not been forbidden from opening it. */
> -    size = qemu_opt_get_size(opts, BLOCK_OPT_SIZE, 0);
> +    size = qemu_opt_get_size(opts, BLOCK_OPT_SIZE, img_size);
>       if (backing_file && !(flags & BDRV_O_NO_BACKING)) {
>           BlockDriverState *bs;
>           char *full_backing = g_new0(char, PATH_MAX);
>