Re: [Qemu-devel] [PATCH v2 19/20] block: do not abuse EMEDIUMTYPE

[Fam Zheng]

On Tue, 02/11 18:03, Paolo Bonzini wrote:
> Returning "Wrong medium type" for an image that does not have a valid
> header is a bit weird.  Improve the error by mentioning what format
> was trying to open it.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  block/bochs.c     | 3 ++-
>  block/cow.c       | 3 ++-
>  block/parallels.c | 3 ++-
>  block/qcow.c      | 3 ++-
>  block/qcow2.c     | 2 +-
>  block/qed.c       | 3 ++-
>  block/vdi.c       | 4 ++--
>  block/vmdk.c      | 6 ++++--
>  block/vpc.c       | 3 ++-
>  9 files changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/block/bochs.c b/block/bochs.c
> index 51d9a90..f0f9a7e 100644
> --- a/block/bochs.c
> +++ b/block/bochs.c
> @@ -129,7 +129,8 @@ static int bochs_open(BlockDriverState *bs, QDict 
> *options, int flags,
>          strcmp(bochs.subtype, GROWING_TYPE) ||
>       ((le32_to_cpu(bochs.version) != HEADER_VERSION) &&
>       (le32_to_cpu(bochs.version) != HEADER_V1))) {
> -        return -EMEDIUMTYPE;
> +        error_setg(errp, "Image not in Bochs format");
> +        return -EINVAL;
>      }
>  
>      if (le32_to_cpu(bochs.version) == HEADER_V1) {
> diff --git a/block/cow.c b/block/cow.c
> index 43a2150..af7b824 100644
> --- a/block/cow.c
> +++ b/block/cow.c
> @@ -74,7 +74,8 @@ static int cow_open(BlockDriverState *bs, QDict *options, 
> int flags,
>      }
>  
>      if (be32_to_cpu(cow_header.magic) != COW_MAGIC) {
> -        ret = -EMEDIUMTYPE;
> +        error_setg(errp, "Image not in COW format");
> +        ret = -EINVAL;
>          goto fail;
>      }
>  
> diff --git a/block/parallels.c b/block/parallels.c
> index 2121e43..5c032f5 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -85,7 +85,8 @@ static int parallels_open(BlockDriverState *bs, QDict 
> *options, int flags,
>  
>      if (memcmp(ph.magic, HEADER_MAGIC, 16) ||
>          (le32_to_cpu(ph.version) != HEADER_VERSION)) {
> -        ret = -EMEDIUMTYPE;
> +        error_setg(errp, "Image not in Parallels format");
> +        ret = -EINVAL;
>          goto fail;
>      }
>  
> diff --git a/block/qcow.c b/block/qcow.c
> index 23bc691..292a314 100644
> --- a/block/qcow.c
> +++ b/block/qcow.c
> @@ -113,7 +113,8 @@ static int qcow_open(BlockDriverState *bs, QDict 
> *options, int flags,
>      be64_to_cpus(&header.l1_table_offset);
>  
>      if (header.magic != QCOW_MAGIC) {
> -        ret = -EMEDIUMTYPE;
> +        error_setg(errp, "Image not in qcow format");
> +        ret = -EINVAL;
>          goto fail;
>      }
>      if (header.version != QCOW_VERSION) {
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 2da62b8..fa63d37 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -449,7 +449,7 @@ static int qcow2_open(BlockDriverState *bs, QDict 
> *options, int flags,
>  
>      if (header.magic != QCOW_MAGIC) {
>          error_setg(errp, "Image is not in qcow2 format");
> -        ret = -EMEDIUMTYPE;
> +        ret = -EINVAL;
>          goto fail;
>      }
>      if (header.version < 2 || header.version > 3) {
> diff --git a/block/qed.c b/block/qed.c
> index 59bdd58..7efbc3b 100644
> --- a/block/qed.c
> +++ b/block/qed.c
> @@ -391,7 +391,8 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict 
> *options, int flags,
>      qed_header_le_to_cpu(&le_header, &s->header);
>  
>      if (s->header.magic != QED_MAGIC) {
> -        return -EMEDIUMTYPE;
> +        error_setg(errp, "Image not in QED format");
> +        return -EINVAL;
>      }
>      if (s->header.features & ~QED_FEATURE_MASK) {
>          /* image uses unsupported feature bits */
> diff --git a/block/vdi.c b/block/vdi.c
> index 2d7490f..68e152c 100644
> --- a/block/vdi.c
> +++ b/block/vdi.c
> @@ -395,8 +395,8 @@ static int vdi_open(BlockDriverState *bs, QDict *options, 
> int flags,
>      }
>  
>      if (header.signature != VDI_SIGNATURE) {
> -        logout("bad vdi signature %08x\n", header.signature);
> -        ret = -EMEDIUMTYPE;
> +        error_setg(errp, "Image not in VDI format (bad signature %08x)", 
> header.signature);
> +        ret = -EINVAL;
>          goto fail;
>      } else if (header.version != VDI_VERSION_1_1) {
>          logout("unsupported version %u.%u\n",
> diff --git a/block/vmdk.c b/block/vmdk.c
> index f148164..888a963 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -747,7 +747,8 @@ static int vmdk_open_sparse(BlockDriverState *bs,
>              return vmdk_open_vmdk4(bs, file, flags, errp);
>              break;
>          default:
> -            return -EMEDIUMTYPE;
> +            error_setg(errp, "Image not in VMDK format");
> +            return -EINVAL;
>              break;
>      }
>  }
> @@ -859,7 +860,8 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int 
> flags, char *buf,
>      BDRVVmdkState *s = bs->opaque;
>  
>      if (vmdk_parse_description(buf, "createType", ct, sizeof(ct))) {
> -        ret = -EMEDIUMTYPE;
> +        error_setg(errp, "invalid VMDK image descriptor");
> +        ret = -EINVAL;
>          goto exit;
>      }
>      if (strcmp(ct, "monolithicFlat") &&
> diff --git a/block/vpc.c b/block/vpc.c
> index 1d326cb..238d91a 100644
> --- a/block/vpc.c
> +++ b/block/vpc.c
> @@ -190,7 +190,8 @@ static int vpc_open(BlockDriverState *bs, QDict *options, 
> int flags,
>              goto fail;
>          }
>          if (strncmp(footer->creator, "conectix", 8)) {
> -            ret = -EMEDIUMTYPE;
> +            error_setg(errp, "invalid VPC image");

This is also the magic check but not following the "Image not in FOO format"
pattern in this patch. But it's not a stopper.

Fam