From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50000) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFe9g-0003Jo-JX for qemu-devel@nongnu.org; Tue, 18 Feb 2014 01:27:50 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WFe9a-0000sc-Je for qemu-devel@nongnu.org; Tue, 18 Feb 2014 01:27:44 -0500 Received: from mx1.redhat.com ([209.132.183.28]:12979) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFe9a-0000sY-Be for qemu-devel@nongnu.org; Tue, 18 Feb 2014 01:27:38 -0500 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s1I6Rbip019811 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 18 Feb 2014 01:27:37 -0500 Date: Tue, 18 Feb 2014 14:27:46 +0800 From: Fam Zheng Message-ID: <20140218062746.GA21656@T430.redhat.com> References: <1392138233-26407-1-git-send-email-pbonzini@redhat.com> <1392138233-26407-20-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1392138233-26407-20-git-send-email-pbonzini@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 19/20] block: do not abuse EMEDIUMTYPE List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: kwolf@redhat.com, qemu-devel@nongnu.org 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 > --- > 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