Re: [Qemu-devel] [PATCH 13/16] block/parallels: read disk size from XML if DiskDescriptor.xml is passed

[Kevin Wolf <kwolf@redhat.com>]

Am 15.12.2014 um 09:27 hat Denis V. Lunev geschrieben:
> as an image filename. This is preparational commit to read snapshot
> information from XML. This is the only global parameter which will be
> kept in BDRVParallelsState, the rest should be layered down into
> snapshots information structure.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Jeff Cody <jcody@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/parallels.c | 33 +++++++++++++++++++++++++++++----
>  1 file changed, 29 insertions(+), 4 deletions(-)
> 
> diff --git a/block/parallels.c b/block/parallels.c
> index 718274b..0c0e669 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -107,6 +107,7 @@ static int parallels_open_image(BlockDriverState *bs, Error **errp)
>      int i;
>      struct parallels_header ph;
>      int ret;
> +    int64_t total_sectors;
>  
>      bs->read_only = 1; // no write support yet
>  
> @@ -115,20 +116,35 @@ static int parallels_open_image(BlockDriverState *bs, Error **errp)
>          goto fail;
>      }
>  
> -    bs->total_sectors = le64_to_cpu(ph.nb_sectors);
> +    total_sectors = le64_to_cpu(ph.nb_sectors);
>  
>      if (le32_to_cpu(ph.version) != HEADER_VERSION) {
>          goto fail_format;
>      }
>      if (!memcmp(ph.magic, HEADER_MAGIC, 16)) {
>          s->off_multiplier = 1;
> -        bs->total_sectors = 0xffffffff & bs->total_sectors;
> +        total_sectors = 0xffffffff & total_sectors;
>      } else if (!memcmp(ph.magic, HEADER_MAGIC2, 16)) {
>          s->off_multiplier = le32_to_cpu(ph.tracks);
>      } else {
>          goto fail_format;
>      }
>  
> +    if (total_sectors == 0) {
> +        error_setg(errp, "Invalid image: zero total sectors");
> +        ret = -EINVAL;
> +        goto fail;
> +    }
> +    if (bs->total_sectors == 0) {
> +        /* no descriptor file, standalone image opened */
> +        bs->total_sectors = total_sectors;
> +    }
> +    if (bs->total_sectors != total_sectors) {
> +        error_setg(errp, "Invalid image: wrong total sectors");
> +        ret = -EINVAL;
> +        goto fail;
> +    }
> +
>      s->tracks = le32_to_cpu(ph.tracks);
>      if (s->tracks == 0) {
>          error_setg(errp, "Invalid image: Zero sectors per track");
> @@ -234,7 +250,7 @@ static int parallels_open_xml(BlockDriverState *bs, int flags, Error **errp)
>      int size, ret;
>      xmlDoc *doc = NULL;
>      xmlNode *root, *image;
> -    char *xml = NULL;
> +    char *xml = NULL, *endptr;
>      const char *data;
>      char image_path[PATH_MAX];
>      Error *local_err = NULL;
> @@ -271,7 +287,6 @@ static int parallels_open_xml(BlockDriverState *bs, int flags, Error **errp)
>  
>      data = xml_get_text(root, "Disk_Parameters", "Padding", NULL);
>      if (data != NULL) {
> -        char *endptr;
>          unsigned long pad;
>  
>          pad = strtoul(data, &endptr, 0);
> @@ -281,6 +296,16 @@ static int parallels_open_xml(BlockDriverState *bs, int flags, Error **errp)
>          s->padding = (uint32_t)pad;
>      }
>  
> +    data = xml_get_text(root, "Disk_Parameters", "Disk_size", NULL);
> +    if (data == NULL) {
> +        goto fail;
> +    } else {
> +        bs->total_sectors = strtoull(data, &endptr, 0);

Same comment about strtoull() error checks as in a previous patch.

> +        if (endptr != NULL && *endptr != '\0') {
> +            goto fail;
> +        }
> +    }
> +
>      image = xml_seek(root, "StorageData", "Storage", "Image", NULL);
>      data = ""; /* make gcc happy */
>      for (size = 0; image != NULL; image = image->next) {

Kevin