Re: [Qemu-devel] [PATCH 2/2] qemu-nbd: Add --image-size option

[Paolo Bonzini]

On 20/09/2016 11:41, Tomáš Golembiovský wrote:
> When image is part of the file it makes sense to limit the length of the
> image in the file. Otherwise it is assumed that the image spans to the
> end of the file. This assumption may lead to reads/writes outside of the
> image and thus lead to errors or data corruption.
> 
> To limit the assumed image size new option is introduced.

The patch makes sense, but I think the commit message is incorrect
because this bug is already fixed by patch 1.  Also, the option in the
help is --device-size, not --image-size; I would just call it --size.

Thanks,

Paolo

> 
> Signed-off-by: Tomáš Golembiovský <address@hidden>
> ---
>  qemu-nbd.c    | 44 +++++++++++++++++++++++++++++++++++---------
>  qemu-nbd.texi |  4 ++++
>  2 files changed, 39 insertions(+), 9 deletions(-)
> 
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 629bce1..7ed52f7 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -85,6 +85,7 @@ static void usage(const char *name)
>  "\n"
>  "Exposing part of the image:\n"
>  "  -o, --offset=OFFSET       offset into the image\n"
> +"  -S, --device-size=LEN     limit reported device size\n"
>  "  -P, --partition=NUM       only expose partition NUM\n"
>  "\n"
>  "General purpose options:\n"
> @@ -471,10 +472,12 @@ int main(int argc, char **argv)
>      const char *port = NULL;
>      char *sockpath = NULL;
>      char *device = NULL;
> -    off_t fd_size;
> +    off_t real_size = 0;
> +    off_t fd_size = 0;
> +    bool has_image_size = false;
>      QemuOpts *sn_opts = NULL;
>      const char *sn_id_or_name = NULL;
> -    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:x:T:";
> +    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:x:T:S:";
>      struct option lopt[] = {
>          { "help", no_argument, NULL, 'h' },
>          { "version", no_argument, NULL, 'V' },
> @@ -482,6 +485,7 @@ int main(int argc, char **argv)
>          { "port", required_argument, NULL, 'p' },
>          { "socket", required_argument, NULL, 'k' },
>          { "offset", required_argument, NULL, 'o' },
> +        { "image-size", required_argument, NULL, 'S' },
>          { "read-only", no_argument, NULL, 'r' },
>          { "partition", required_argument, NULL, 'P' },
>          { "connect", required_argument, NULL, 'c' },
> @@ -714,6 +718,18 @@ int main(int argc, char **argv)
>              g_free(trace_file);
>              trace_file = trace_opt_parse(optarg);
>              break;
> +        case 'S':
> +            ret = qemu_strtoll(optarg, NULL, 0, &fd_size);
> +            if (ret != 0) {
> +                error_report("Invalid image size `%s'", optarg);
> +                exit(EXIT_FAILURE);
> +            }
> +            if (fd_size <= 0) {
> +                error_report("Image size must be positive `%s'", optarg);
> +                exit(EXIT_FAILURE);
> +            }
> +            has_image_size = true;
> +            break;
>          }
>      }
>  
> @@ -894,19 +910,29 @@ int main(int argc, char **argv)
>      }
>  
>      bs->detect_zeroes = detect_zeroes;
> -    fd_size = blk_getlength(blk);
> -    if (fd_size < 0) {
> +    real_size = blk_getlength(blk);
> +    if (real_size < 0) {
>          error_report("Failed to determine the image length: %s",
> -                     strerror(-fd_size));
> +                     strerror(-real_size));
>          exit(EXIT_FAILURE);
>      }
>  
> -    if (dev_offset >= fd_size) {
> -        error_report("Offset (%lu) has to be smaller than the image size 
> (%lu)",
> -                     dev_offset, fd_size);
> +    if (!has_image_size) {
> +        fd_size = real_size;
> +
> +        if (dev_offset >= fd_size) {
> +            error_report("Offset (%lu) has to be smaller than image size 
> (%lu)",
> +                        dev_offset, fd_size);
> +            exit(EXIT_FAILURE);
> +        }
> +
> +        fd_size -= dev_offset;
> +    } else if ((dev_offset + fd_size) > real_size) {
> +        error_report("Offset (%lu) plus image size (%lu) has to be smaller "
> +                     "than or equal to real image size (%lu)",
> +                     dev_offset, fd_size, real_size);
>          exit(EXIT_FAILURE);
>      }
> -    fd_size -= dev_offset;
>  
>      if (partition != -1) {
>          ret = find_partition(blk, partition, &dev_offset, &fd_size);
> diff --git a/qemu-nbd.texi b/qemu-nbd.texi
> index 91ebf04..c589525 100644
> --- a/qemu-nbd.texi
> +++ b/qemu-nbd.texi
> @@ -30,6 +30,10 @@ credentials for the qemu-nbd server.
>  The TCP port to listen on (default @samp{10809})
>  @item -o, address@hidden
>  The offset into the image
> address@hidden -S, address@hidden
> +The size of the image to present to client. This is useful in combination 
> with
> address@hidden when the image is embedded in file but does not span to the 
> end of the
> +file.
>  @item -b, address@hidden
>  The interface to bind to (default @samp{0.0.0.0})
>  @item -k, address@hidden
>