[Qemu-devel] Block driver and constant sector size

[Qemu-devel] Block driver and constant sector size

[Jonathan Phenix]

Hi,

currently the block driver in qemu only handles blocks (or sectors) 
which are 512 bytes long, this is ideal if the device you are emulating 
have a block size which is a multiple of 512. Unfortunately, this is not 
always the case, for example an audio CD (CD-DA) or a CD in mode 2 have 
a sector size of 2352, which is not a multiple of 512. Typical data CD 
are encoded in mode 1 with an ISO9660 file system, and this mode uses a 
sector size of 2048, which is a multiple of 512, this is the reason why 
the problem probably didn't come up yet. You can dump about any CDs in 
mode 2 under Linux with the "cdrdao read-cd --read-raw [...]" command 
for example.

In order to let the block driver read these raw images, I would like to 
change:

BlockDriverState *bdrv_new(const char *device_name);

to:

BlockDriverState *bdrv_new(const char *device_name, int sector_size);

and change:

int (*bdrv_probe)(const uint8_t *buf, int buf_size, const char *filename);

of the BlockDriver structure to:

int (*bdrv_probe)(const uint8_t *buf, int buf_size, const char 
*filename, int sector_size);

Then, each probe functions should be modified to reject sector size 
which is not 512 bytes, except for the raw block driver, which will be 
modified to accept any block sizes. This change would probably solve the 
whole problem without having a negative impact on the rest of the code.

Is it the right way to solve the problem? If this solution is accepted, 
I will code it and submit a patch.

Regards,

Jonathan Phénix

Re: [Qemu-devel] Block driver and constant sector size

[Paul Brook]

On Saturday 12 May 2007, Jonathan Phenix wrote:
> Hi,
>
> currently the block driver in qemu only handles blocks (or sectors)
> which are 512 bytes long, 
>...
> Then, each probe functions should be modified to reject sector size
> which is not 512 bytes, except for the raw block driver, which will be
> modified to accept any block sizes. This change would probably solve the
> whole problem without having a negative impact on the rest of the code.
>
> Is it the right way to solve the problem? If this solution is accepted,
> I will code it and submit a patch.

Seems like it might just be simpler to have the qemu block ABI use bytes 
rather than blocks. Maybe with some common helper functions for doing R/M/W 
on hard sectored devices.

By adding variable sized sectors you're just shifting complexity from the 
block backends to the device emulation.

Paul

Re: [Qemu-devel] Block driver and constant sector size

[Jonathan Phenix]

Paul Brook wrote:
> On Saturday 12 May 2007, Jonathan Phenix wrote:
>   
>> Hi,
>>
>> currently the block driver in qemu only handles blocks (or sectors)
>> which are 512 bytes long, 
>> ...
>> Then, each probe functions should be modified to reject sector size
>> which is not 512 bytes, except for the raw block driver, which will be
>> modified to accept any block sizes. This change would probably solve the
>> whole problem without having a negative impact on the rest of the code.
>>
>> Is it the right way to solve the problem? If this solution is accepted,
>> I will code it and submit a patch.
>>     
>
> Seems like it might just be simpler to have the qemu block ABI use bytes 
> rather than blocks. Maybe with some common helper functions for doing R/M/W 
> on hard sectored devices.
>
> By adding variable sized sectors you're just shifting complexity from the 
> block backends to the device emulation.
>   
No, it will not add complexity at all to the device emulation code, 
perhaps I was not clear enough on what I was trying to describe. 
Basically, the only change that would be needed to existing users of the 
block driver is to change "bdrv_new(...)" to "bdrv_new(..., 512)" (it 
could be also done with "bdrv_open" instead) and that's it. The only 
other difference that the user code will see is that the buffer required 
by bdrv_read, bdrv_write and friends must now be a multiple of the size 
requested before, but that's to be expected. The block driver will 
continue to deal with sectors as a base unit.

As for internal changes in the block driver, a new variable, let's call 
it "requested_sector_size" will be added to the probe function of each 
driver as well as the BlockDriver structure. We can simply do "if 
(requested_sector_size != 512) return 0;" for all existing drivers for 
now in their probe functions and implement it in the future if required. 
I will add support for a sector_size that is different than 512 in the 
raw driver, the image file produced by cdrdao is in raw format. Other 
popular CD ripping tools for Windows also creates raw images, with very 
slight variations, as far as I know.

Sounds good?

Regards,

Jonathan Phénix
> Paul
>