Re: [RFC PATCH] block/vpc: Support probing of fixed-size VHD images

[Max Reitz <mreitz@redhat.com>]

On 19.05.21 12:19, Thomas Huth wrote:
> On 29/03/2021 09.25, Thomas Huth wrote:
>> Fixed-size VHD images don't have a header, only a footer. To be able
>> to still detect them right, support probing via the file name, too.
>>
>> Without this change, images get detected as raw:
>>
>> $ qemu-img create -f vpc -o subformat=fixed test.vhd 2G
>> Formatting 'test.vhd', fmt=vpc size=2147483648 subformat=fixed
>> $ qemu-img info test.vhd
>> image: test.vhd
>> file format: raw
>> virtual size: 2 GiB (2147992064 bytes)
>> disk size: 8 KiB
>>
>> With this change:
>>
>> $ qemu-img info test.vhd
>> image: test.vhd
>> file format: vpc
>> virtual size: 2 GiB (2147991552 bytes)
>> disk size: 8 KiB
>>
>> Resolves: https://bugs.launchpad.net/qemu/+bug/1819182
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   I've marked the subject with RFC since I'm not quite sure whether this
>>   is really a good idea... please let me know what you think about it...
>>
>>   block/vpc.c | 12 +++++++++++-
>>   1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/block/vpc.c b/block/vpc.c
>> index 17a705b482..be561e4b39 100644
>> --- a/block/vpc.c
>> +++ b/block/vpc.c
>> @@ -191,8 +191,18 @@ static uint32_t vpc_checksum(void *p, size_t size)
>>     static int vpc_probe(const uint8_t *buf, int buf_size, const char 
>> *filename)
>>   {
>> -    if (buf_size >= 8 && !strncmp((char *)buf, "conectix", 8))
>> +    if (buf_size >= 8 && !strncmp((char *)buf, "conectix", 8)) {
>>           return 100;
>> +    }
>> +
>> +    /* It could be a fixed-size image without header -> check 
>> extension, too */
>> +    if (filename) {
>> +        int len = strlen(filename);
>> +        if (len > 4 && !strcasecmp(&filename[len - 4], ".vhd")) {
>> +            return 10;
>> +        }
>> +    }
>> +
>>       return 0;
>>   }
>
> Ping!
>
> Anybody any comments on this one?
>
>  Thomas

Sorry I’m replying so late, but honestly, it’s because I’m just a bit 
afraid to respond.  So, perhaps like others, I hoped someone else with a 
stronger opinion would do it in my stead.

I understand this addresses a real problem, but OTOH probing by the file 
extension intuitively seems like a bad solution to me. What’s the 
problem with simply requiring the user to spcify the format in such a case?

I mean, I can’t think of a concrete problem with probing by the filename 
extension.  The worst that can happen is that a raw image is called 
.vhd, we try to open it, and the vhd driver then says the image doesn’t 
work.  That could be a real problem, but, well, it would be kind of 
deserved.  (Unless the user really in good faith just thought .vhd would 
be a nice extension for their raw virtual HDD image.  Which, then again, 
I couldn’t blame them for.)

Perhaps we could print a message if the extension matches that advises 
the user to explicitly specify the format for such images?

Max