[Qemu-devel] [PATCH v2 0/5] block: Fix error report for wrong file format

[Qemu-devel] [PATCH v2 0/5] block: Fix error report for wrong file format

[Stefan Weil]

These patches improve the error report if the file format was
specified explicitly (example: -drive file=myfile,format=qcow2)
and the given format does not match the real format.

They fix those bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=556482
https://bugs.launchpad.net/qemu/+bug/1090600

Changes in v2:

* Use error code EMEDIUMTYPE instead of inventing a new one.
* Split modifications in block/vdi.c into 3 separate patches.

I decided to use EMEDIUMTYPE because I think that it matches best
the kind of error which we want to describe here.

EINVAL is too unspecific, and ENOTTY means something completely different,
therefore I did not use one of those suggested values.

Cheers,
Stefan

[PATCH v2 1/5] block: Add special error code for wrong format
[PATCH v2 2/5] block: Use error code EMEDIUMTYPE for wrong format in
[PATCH v2 3/5] block/vdi: Improve debug output for signature
[PATCH v2 4/5] block/vdi: Improved return values from vdi_open
[PATCH v2 5/5] block/vdi: Check for bad signature

[Qemu-devel] [PATCH v2 1/5] block: Add special error code for wrong format

[Stefan Weil]

The block drivers need a special error code for "wrong format".
>From the available error codes EMEDIUMTYPE fits best.
It is not available on all platforms, so a definition in
qemu-common.h and a specific error report are needed.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 blockdev.c            |    9 +++++++--
 include/qemu-common.h |    3 +++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 9126587..34cde32 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -617,8 +617,13 @@ DriveInfo *drive_init(QemuOpts *opts, BlockInterfaceType block_default_type)
 
     ret = bdrv_open(dinfo->bdrv, file, bdrv_flags, drv);
     if (ret < 0) {
-        error_report("could not open disk image %s: %s",
-                     file, strerror(-ret));
+        if (ret == -EMEDIUMTYPE) {
+            error_report("could not open disk image %s: not in %s format",
+                         file, drv->format_name);
+        } else {
+            error_report("could not open disk image %s: %s",
+                         file, strerror(-ret));
+        }
         goto err;
     }
 
diff --git a/include/qemu-common.h b/include/qemu-common.h
index ca464bb..af2379f 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -68,6 +68,9 @@
 #if !defined(ECANCELED)
 #define ECANCELED 4097
 #endif
+#if !defined(EMEDIUMTYPE)
+#define EMEDIUMTYPE 4098
+#endif
 #ifndef TIME_MAX
 #define TIME_MAX LONG_MAX
 #endif
-- 
1.7.10.4

[Qemu-devel] [PATCH v2 2/5] block: Use error code EMEDIUMTYPE for wrong format in some block drivers

[Stefan Weil]

This improves error reports for bochs, cow, qcow, qcow2, qed and vmdk
when a file with the wrong format is selected.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 block/bochs.c |    2 +-
 block/cow.c   |    2 +-
 block/qcow.c  |    2 +-
 block/qcow2.c |    2 +-
 block/qed.c   |    2 +-
 block/vmdk.c  |    4 ++--
 6 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/block/bochs.c b/block/bochs.c
index 1b1d9cd..3737583 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -126,7 +126,7 @@ static int bochs_open(BlockDriverState *bs, int flags)
         strcmp(bochs.subtype, GROWING_TYPE) ||
 	((le32_to_cpu(bochs.version) != HEADER_VERSION) &&
 	(le32_to_cpu(bochs.version) != HEADER_V1))) {
-        goto fail;
+        return -EMEDIUMTYPE;
     }
 
     if (le32_to_cpu(bochs.version) == HEADER_V1) {
diff --git a/block/cow.c b/block/cow.c
index a33ce95..4baf904 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -73,7 +73,7 @@ static int cow_open(BlockDriverState *bs, int flags)
     }
 
     if (be32_to_cpu(cow_header.magic) != COW_MAGIC) {
-        ret = -EINVAL;
+        ret = -EMEDIUMTYPE;
         goto fail;
     }
 
diff --git a/block/qcow.c b/block/qcow.c
index 4276610..a7135ee 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -112,7 +112,7 @@ static int qcow_open(BlockDriverState *bs, int flags)
     be64_to_cpus(&header.l1_table_offset);
 
     if (header.magic != QCOW_MAGIC) {
-        ret = -EINVAL;
+        ret = -EMEDIUMTYPE;
         goto fail;
     }
     if (header.version != QCOW_VERSION) {
diff --git a/block/qcow2.c b/block/qcow2.c
index f6abff6..7610e56 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -311,7 +311,7 @@ static int qcow2_open(BlockDriverState *bs, int flags)
     be32_to_cpus(&header.nb_snapshots);
 
     if (header.magic != QCOW_MAGIC) {
-        ret = -EINVAL;
+        ret = -EMEDIUMTYPE;
         goto fail;
     }
     if (header.version < 2 || header.version > 3) {
diff --git a/block/qed.c b/block/qed.c
index cf85d8f..b8515e5 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -390,7 +390,7 @@ static int bdrv_qed_open(BlockDriverState *bs, int flags)
     qed_header_le_to_cpu(&le_header, &s->header);
 
     if (s->header.magic != QED_MAGIC) {
-        return -EINVAL;
+        return -EMEDIUMTYPE;
     }
     if (s->header.features & ~QED_FEATURE_MASK) {
         /* image uses unsupported feature bits */
diff --git a/block/vmdk.c b/block/vmdk.c
index 19298c2..8333afb 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -616,7 +616,7 @@ static int vmdk_open_sparse(BlockDriverState *bs,
             return vmdk_open_vmdk4(bs, file, flags);
             break;
         default:
-            return -EINVAL;
+            return -EMEDIUMTYPE;
             break;
     }
 }
@@ -718,7 +718,7 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags,
     }
     buf[2047] = '\0';
     if (vmdk_parse_description(buf, "createType", ct, sizeof(ct))) {
-        return -EINVAL;
+        return -EMEDIUMTYPE;
     }
     if (strcmp(ct, "monolithicFlat") &&
         strcmp(ct, "twoGbMaxExtentSparse") &&
-- 
1.7.10.4

[Qemu-devel] [PATCH v2 3/5] block/vdi: Improve debug output for signature

[Stefan Weil]

The signature is a 32 bit value and needs up to 8 hex digits for printing.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 block/vdi.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/vdi.c b/block/vdi.c
index 021abaa..0e1ed61 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -246,7 +246,7 @@ static void vdi_header_print(VdiHeader *header)
 {
     char uuid[37];
     logout("text        %s", header->text);
-    logout("signature   0x%04x\n", header->signature);
+    logout("signature   0x%08x\n", header->signature);
     logout("header size 0x%04x\n", header->header_size);
     logout("image type  0x%04x\n", header->image_type);
     logout("image flags 0x%04x\n", header->image_flags);
-- 
1.7.10.4

[Qemu-devel] [PATCH v2 4/5] block/vdi: Improved return values from vdi_open

[Stefan Weil]

vdi_open returned -1 in case of any error, but it should return an
error code (negative value of errno or -EMEDIUMTYPE).

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 block/vdi.c |   17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/block/vdi.c b/block/vdi.c
index 0e1ed61..8b768bf 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -369,10 +369,12 @@ static int vdi_open(BlockDriverState *bs, int flags)
     BDRVVdiState *s = bs->opaque;
     VdiHeader header;
     size_t bmap_size;
+    int ret;
 
     logout("\n");
 
-    if (bdrv_read(bs->file, 0, (uint8_t *)&header, 1) < 0) {
+    ret = bdrv_read(bs->file, 0, (uint8_t *)&header, 1);
+    if (ret < 0) {
         goto fail;
     }
 
@@ -393,30 +395,38 @@ static int vdi_open(BlockDriverState *bs, int flags)
     if (header.version != VDI_VERSION_1_1) {
         logout("unsupported version %u.%u\n",
                header.version >> 16, header.version & 0xffff);
+        ret = -ENOTSUP;
         goto fail;
     } else if (header.offset_bmap % SECTOR_SIZE != 0) {
         /* We only support block maps which start on a sector boundary. */
         logout("unsupported block map offset 0x%x B\n", header.offset_bmap);
+        ret = -ENOTSUP;
         goto fail;
     } else if (header.offset_data % SECTOR_SIZE != 0) {
         /* We only support data blocks which start on a sector boundary. */
         logout("unsupported data offset 0x%x B\n", header.offset_data);
+        ret = -ENOTSUP;
         goto fail;
     } else if (header.sector_size != SECTOR_SIZE) {
         logout("unsupported sector size %u B\n", header.sector_size);
+        ret = -ENOTSUP;
         goto fail;
     } else if (header.block_size != 1 * MiB) {
         logout("unsupported block size %u B\n", header.block_size);
+        ret = -ENOTSUP;
         goto fail;
     } else if (header.disk_size >
                (uint64_t)header.blocks_in_image * header.block_size) {
         logout("unsupported disk size %" PRIu64 " B\n", header.disk_size);
+        ret = -ENOTSUP;
         goto fail;
     } else if (!uuid_is_null(header.uuid_link)) {
         logout("link uuid != 0, unsupported\n");
+        ret = -ENOTSUP;
         goto fail;
     } else if (!uuid_is_null(header.uuid_parent)) {
         logout("parent uuid != 0, unsupported\n");
+        ret = -ENOTSUP;
         goto fail;
     }
 
@@ -432,7 +442,8 @@ static int vdi_open(BlockDriverState *bs, int flags)
     if (bmap_size > 0) {
         s->bmap = g_malloc(bmap_size * SECTOR_SIZE);
     }
-    if (bdrv_read(bs->file, s->bmap_sector, (uint8_t *)s->bmap, bmap_size) < 0) {
+    ret = bdrv_read(bs->file, s->bmap_sector, (uint8_t *)s->bmap, bmap_size);
+    if (ret < 0) {
         goto fail_free_bmap;
     }
 
@@ -448,7 +459,7 @@ static int vdi_open(BlockDriverState *bs, int flags)
     g_free(s->bmap);
 
  fail:
-    return -1;
+    return ret;
 }
 
 static int vdi_reopen_prepare(BDRVReopenState *state,
-- 
1.7.10.4

[Qemu-devel] [PATCH v2 5/5] block/vdi: Check for bad signature

[Stefan Weil]

vdi_open did not check for a bad signature.
This check was only in vdi_probe.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 block/vdi.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/block/vdi.c b/block/vdi.c
index 8b768bf..257a592 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -392,7 +392,11 @@ static int vdi_open(BlockDriverState *bs, int flags)
         header.disk_size &= ~(SECTOR_SIZE - 1);
     }
 
-    if (header.version != VDI_VERSION_1_1) {
+    if (header.signature != VDI_SIGNATURE) {
+        logout("bad vdi signature %08x\n", header.signature);
+        ret = -EMEDIUMTYPE;
+        goto fail;
+    } else if (header.version != VDI_VERSION_1_1) {
         logout("unsupported version %u.%u\n",
                header.version >> 16, header.version & 0xffff);
         ret = -ENOTSUP;
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH v2 0/5] block: Fix error report for wrong file format

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 818 bytes --]

On 01/17/2013 01:45 PM, Stefan Weil wrote:
> These patches improve the error report if the file format was
> specified explicitly (example: -drive file=myfile,format=qcow2)
> and the given format does not match the real format.
> 
> They fix those bugs:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=556482
> https://bugs.launchpad.net/qemu/+bug/1090600
> 
> Changes in v2:
> 
> * Use error code EMEDIUMTYPE instead of inventing a new one.

Alas, EMEDIUMTYPE is non-standard, and doesn't exist outside of Linux.
If you are providing fallbacks for half the platforms because you don't
want to use a standard errno value, then why not go all the way and use
a fallback for all platforms.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH v2 0/5] block: Fix error report for wrong file format

[Stefan Weil]

Am 17.01.2013 22:28, schrieb Eric Blake:
> On 01/17/2013 01:45 PM, Stefan Weil wrote:
>    
>> These patches improve the error report if the file format was
>> specified explicitly (example: -drive file=myfile,format=qcow2)
>> and the given format does not match the real format.
>>
>> They fix those bugs:
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=556482
>> https://bugs.launchpad.net/qemu/+bug/1090600
>>
>> Changes in v2:
>>
>> * Use error code EMEDIUMTYPE instead of inventing a new one.
>>      
> Alas, EMEDIUMTYPE is non-standard, and doesn't exist outside of Linux.
> If you are providing fallbacks for half the platforms because you don't
> want to use a standard errno value, then why not go all the way and use
> a fallback for all platforms.
>    

That's what I did in v1 of my patches: it used BDRV_WRONG_FORMAT.

I didn't introduce a EFILEFORMAT because it looks like
something used outside of the QEMU world (which is not true).

If EFILEFORMAT (or any other new name) is preferred,
I won't object. In this case, I can either send a new patch series,
or whoever commits my patches can do a simple replace operation.

Regards,
Stefan W.

Re: [Qemu-devel] [PATCH v2 0/5] block: Fix error report for wrong file format

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1063 bytes --]

On 01/17/2013 03:25 PM, Stefan Weil wrote:

>>> * Use error code EMEDIUMTYPE instead of inventing a new one.
>>>      
>> Alas, EMEDIUMTYPE is non-standard, and doesn't exist outside of Linux.
>> If you are providing fallbacks for half the platforms because you don't
>> want to use a standard errno value, then why not go all the way and use
>> a fallback for all platforms.
>>    
> 
> That's what I did in v1 of my patches: it used BDRV_WRONG_FORMAT.
> 
> I didn't introduce a EFILEFORMAT because it looks like
> something used outside of the QEMU world (which is not true).
> 
> If EFILEFORMAT (or any other new name) is preferred,
> I won't object. In this case, I can either send a new patch series,
> or whoever commits my patches can do a simple replace operation.

Fair enough; I won't hold up the patch series over a bikeshed choice of
error naming.  So with that:

Series:
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH v2 2/5] block: Use error code EMEDIUMTYPE for wrong format in some block drivers

[Markus Armbruster]

Stefan Weil <sw@weilnetz.de> writes:

> This improves error reports for bochs, cow, qcow, qcow2, qed and vmdk
> when a file with the wrong format is selected.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>  block/bochs.c |    2 +-
>  block/cow.c   |    2 +-
>  block/qcow.c  |    2 +-
>  block/qcow2.c |    2 +-
>  block/qed.c   |    2 +-
>  block/vmdk.c  |    4 ++--
>  6 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/block/bochs.c b/block/bochs.c
> index 1b1d9cd..3737583 100644
> --- a/block/bochs.c
> +++ b/block/bochs.c
> @@ -126,7 +126,7 @@ static int bochs_open(BlockDriverState *bs, int flags)
>          strcmp(bochs.subtype, GROWING_TYPE) ||
>  	((le32_to_cpu(bochs.version) != HEADER_VERSION) &&
>  	(le32_to_cpu(bochs.version) != HEADER_V1))) {
> -        goto fail;
> +        return -EMEDIUMTYPE;
>      }
>  
>      if (le32_to_cpu(bochs.version) == HEADER_V1) {

You make the function return either 0, -1 or -EMEDIUMTYPE.  Please make
it return either 0 or a negative errno code, like this (untested):

diff --git a/block/bochs.c b/block/bochs.c
index 1b1d9cd..a9eb338 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -111,14 +111,15 @@ static int bochs_probe(const uint8_t *buf, int buf_size, const char *filename)
 static int bochs_open(BlockDriverState *bs, int flags)
 {
     BDRVBochsState *s = bs->opaque;
-    int i;
+    int ret, i;
     struct bochs_header bochs;
     struct bochs_header_v1 header_v1;
 
     bs->read_only = 1; // no write support yet
 
-    if (bdrv_pread(bs->file, 0, &bochs, sizeof(bochs)) != sizeof(bochs)) {
-        goto fail;
+    ret = bdrv_pread(bs->file, 0, &bochs, sizeof(bochs));
+    if (ret < 0) {
+        return ret;
     }
 
     if (strcmp(bochs.magic, HEADER_MAGIC) ||
@@ -126,7 +127,7 @@ static int bochs_open(BlockDriverState *bs, int flags)
         strcmp(bochs.subtype, GROWING_TYPE) ||
 	((le32_to_cpu(bochs.version) != HEADER_VERSION) &&
 	(le32_to_cpu(bochs.version) != HEADER_V1))) {
-        goto fail;
+        return -EMEDIUMTYPE;
     }
 
     if (le32_to_cpu(bochs.version) == HEADER_V1) {
@@ -138,9 +139,11 @@ static int bochs_open(BlockDriverState *bs, int flags)
 
     s->catalog_size = le32_to_cpu(bochs.extra.redolog.catalog);
     s->catalog_bitmap = g_malloc(s->catalog_size * 4);
-    if (bdrv_pread(bs->file, le32_to_cpu(bochs.header), s->catalog_bitmap,
-                   s->catalog_size * 4) != s->catalog_size * 4)
-	goto fail;
+    ret = bdrv_pread(bs->file, le32_to_cpu(bochs.header), s->catalog_bitmap,
+                     s->catalog_size * 4);
+    if (ret < 0) {
+	return ret;
+    }
     for (i = 0; i < s->catalog_size; i++)
 	le32_to_cpus(&s->catalog_bitmap[i]);
 
@@ -153,8 +156,6 @@ static int bochs_open(BlockDriverState *bs, int flags)
 
     qemu_co_mutex_init(&s->lock);
     return 0;
- fail:
-    return -1;
 }
 
 static int64_t seek_to_sector(BlockDriverState *bs, int64_t sector_num)

Same for all the other bdrv_open() methods.

Re: [Qemu-devel] [PATCH v2 2/5] block: Use error code EMEDIUMTYPE for wrong format in some block drivers

[Stefan Weil]

Am 18.01.2013 09:53, schrieb Markus Armbruster:
> Stefan Weil <sw@weilnetz.de> writes:
>> This improves error reports for bochs, cow, qcow, qcow2, qed and vmdk
>> when a file with the wrong format is selected.
>>
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>> ---
>>  block/bochs.c |    2 +-
>>  block/cow.c   |    2 +-
>>  block/qcow.c  |    2 +-
>>  block/qcow2.c |    2 +-
>>  block/qed.c   |    2 +-
>>  block/vmdk.c  |    4 ++--
>>  6 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/block/bochs.c b/block/bochs.c
>> index 1b1d9cd..3737583 100644
>> --- a/block/bochs.c
>> +++ b/block/bochs.c
>> @@ -126,7 +126,7 @@ static int bochs_open(BlockDriverState *bs, int flags)
>>          strcmp(bochs.subtype, GROWING_TYPE) ||
>>  	((le32_to_cpu(bochs.version) != HEADER_VERSION) &&
>>  	(le32_to_cpu(bochs.version) != HEADER_V1))) {
>> -        goto fail;
>> +        return -EMEDIUMTYPE;
>>      }
>>  
>>      if (le32_to_cpu(bochs.version) == HEADER_V1) {
> You make the function return either 0, -1 or -EMEDIUMTYPE.  Please make
> it return either 0 or a negative errno code, like this (untested):

Hi Markus,

returning 0, -1 is like before, only returning -EMEDIUMTYPE is new.

You are right, a return value of -1 should be replaced by a negative
error value. I fixed this for block/vdi.c in a separate patch as
suggested by Kevin, see http://patchwork.ozlabs.org/patch/213375/.

The same kind of improvement should be done for other block
drivers which currently use -1, but that can be done after my
patch series was applied.

The primary purpose of my patch series was fixing open bugreports.
For vdi I did more because I feel responsible for that part of the
code.

Regards,
StefanW.

Re: [Qemu-devel] [PATCH v2 2/5] block: Use error code EMEDIUMTYPE for wrong format in some block drivers

[Markus Armbruster]

Stefan Weil <sw@weilnetz.de> writes:

> Am 18.01.2013 09:53, schrieb Markus Armbruster:
>> Stefan Weil <sw@weilnetz.de> writes:
>>> This improves error reports for bochs, cow, qcow, qcow2, qed and vmdk
>>> when a file with the wrong format is selected.
>>>
>>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>>> ---
>>>  block/bochs.c |    2 +-
>>>  block/cow.c   |    2 +-
>>>  block/qcow.c  |    2 +-
>>>  block/qcow2.c |    2 +-
>>>  block/qed.c   |    2 +-
>>>  block/vmdk.c  |    4 ++--
>>>  6 files changed, 7 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/block/bochs.c b/block/bochs.c
>>> index 1b1d9cd..3737583 100644
>>> --- a/block/bochs.c
>>> +++ b/block/bochs.c
>>> @@ -126,7 +126,7 @@ static int bochs_open(BlockDriverState *bs, int flags)
>>>          strcmp(bochs.subtype, GROWING_TYPE) ||
>>>  	((le32_to_cpu(bochs.version) != HEADER_VERSION) &&
>>>  	(le32_to_cpu(bochs.version) != HEADER_V1))) {
>>> -        goto fail;
>>> +        return -EMEDIUMTYPE;
>>>      }
>>>  
>>>      if (le32_to_cpu(bochs.version) == HEADER_V1) {
>> You make the function return either 0, -1 or -EMEDIUMTYPE.  Please make
>> it return either 0 or a negative errno code, like this (untested):
>
> Hi Markus,
>
> returning 0, -1 is like before, only returning -EMEDIUMTYPE is new.
>
> You are right, a return value of -1 should be replaced by a negative
> error value. I fixed this for block/vdi.c in a separate patch as
> suggested by Kevin, see http://patchwork.ozlabs.org/patch/213375/.
>
> The same kind of improvement should be done for other block
> drivers which currently use -1, but that can be done after my
> patch series was applied.
>
> The primary purpose of my patch series was fixing open bugreports.
> For vdi I did more because I feel responsible for that part of the
> code.

I had a closer look at the various bdrv_open() methods, and how they're
used.  Turns out that we already assume they return 0/-errno, yet the
following methods return -1 on some or all errors:

    bochs_open 
    cloop_open
    dmg_open
    parallels_open
    vdi_open
    vpc_open

They all need to be fixed.  I appreciate you fixing vdi_open().

However, you "improve" bochs_open() from completely and obviously broken
(return -1 on error always) to half-broken (return -1 on some errors,
and -errno on others).  I don't like that.

Fixing it up doesn't look hard to me (sketch appended).  Could you do
that for us?

If not, I'd prefer to leave bochs_open() completely and obviously
broken.




diff --git a/block/bochs.c b/block/bochs.c
index 1b1d9cd..57b2dc8 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -111,13 +111,14 @@ static int bochs_probe(const uint8_t *buf, int buf_size, const char *filename)
 static int bochs_open(BlockDriverState *bs, int flags)
 {
     BDRVBochsState *s = bs->opaque;
-    int i;
+    int ret, i;
     struct bochs_header bochs;
     struct bochs_header_v1 header_v1;
 
     bs->read_only = 1; // no write support yet
 
-    if (bdrv_pread(bs->file, 0, &bochs, sizeof(bochs)) != sizeof(bochs)) {
+    ret = bdrv_pread(bs->file, 0, &bochs, sizeof(bochs));
+    if (ret < 0) {
         goto fail;
     }
 
@@ -126,6 +127,7 @@ static int bochs_open(BlockDriverState *bs, int flags)
         strcmp(bochs.subtype, GROWING_TYPE) ||
 	((le32_to_cpu(bochs.version) != HEADER_VERSION) &&
 	(le32_to_cpu(bochs.version) != HEADER_V1))) {
+        ret = -EMEDIUMTYPE;
         goto fail;
     }
 
@@ -138,8 +140,9 @@ static int bochs_open(BlockDriverState *bs, int flags)
 
     s->catalog_size = le32_to_cpu(bochs.extra.redolog.catalog);
     s->catalog_bitmap = g_malloc(s->catalog_size * 4);
-    if (bdrv_pread(bs->file, le32_to_cpu(bochs.header), s->catalog_bitmap,
-                   s->catalog_size * 4) != s->catalog_size * 4)
+    ret = bdrv_pread(bs->file, le32_to_cpu(bochs.header), s->catalog_bitmap,
+                     s->catalog_size * 4);
+    if (ret < 0)
 	goto fail;
     for (i = 0; i < s->catalog_size; i++)
 	le32_to_cpus(&s->catalog_bitmap[i]);
@@ -154,7 +157,7 @@ static int bochs_open(BlockDriverState *bs, int flags)
     qemu_co_mutex_init(&s->lock);
     return 0;
  fail:
-    return -1;
+    return ret;
 }
 
 static int64_t seek_to_sector(BlockDriverState *bs, int64_t sector_num)