Re: [Qemu-devel] Block driver and constant sector size
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
Re: [Qemu-devel] Block driver and constant sector size
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
[Qemu-devel] Block driver and constant sector size
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
