On Wed, Jul 18, 2012 at 8:51 AM, Wenchao Xia <xiaw...@linux.vnet.ibm.com> wrote: > Hi, following is API draft, prototypes were taken from qemu/block.h, > and the API prefix is changed frpm bdrv to qbdrvs, to declare related > object is BlockDriverState, not BlockDriver. One issue here is it may > require include block_int.h, which is not LGPL2 licensed yet. > API format is kept mostly the same with qemu generic block layer, to > make it easier for implement, and easier to make qemu migrate on it if > possible. > > > /* structure init and uninit */ > BlockDriverState *qbdrvs_new(const char *device_name); > void qbdrvs_delete(BlockDriverState *bs); > > > /* file open and close */ > int qbdrvs_open(BlockDriverState *bs, const char *filename, int flags, > BlockDriver *drv);
How are the errors passed? Alternative version with file descriptor or struct FILE instead of filename might become useful but can be added later. > void qbdrvs_close(BlockDriverState *bs); > int qbdrvs_img_create(const char *filename, const char *fmt, > const char *base_filename, const char *base_fmt, > char *options, uint64_t img_size, int flags); 'const char *options' > > > /* sync access */ > int qbdrvs_read(BlockDriverState *bs, int64_t sector_num, > uint8_t *buf, int nb_sectors); > int qbdrvs_write(BlockDriverState *bs, int64_t sector_num, > const uint8_t *buf, int nb_sectors); Do we want to use sectors here? How about just raw byte offsets and number of bytes? I'd leave any hardware details out and just provide file semantics (open/read/write/close). Future QEMU refactorings could make supporting HW info inconvenient. If HW details (geometry etc., VM state) are needed, there should be a separate API. Vectored I/O might be useful too. > > > /* info retrieve */ > //sector, size and geometry info > int qbdrvs_get_info(BlockDriverState *bs, BlockDriverInfo *bdi); Currently BlockDriverInfo does not look very useful for outside users, with the exception of dirty state. How about accessors instead: bool qbdrvs_is_dirty(BlockDriverState *bs); Also the format (QCOW) is needed so that the user can use backing file functions: const char *qbdrvs_get_format(BlockDriverState *bs); > int64_t qbdrvs_getlength(BlockDriverState *bs); > int64_t qbdrvs_get_allocated_file_size(BlockDriverState *bs); > void qbdrvs_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr); Also this function shouldn't be needed if we don't give HW details with this API. > //image type > const char *qbdrvs_get_format_name(BlockDriverState *bs); > //backing file info > void qbdrvs_get_backing_filename(BlockDriverState *bs, > char *filename, int filename_size); > void qbdrvs_get_full_backing_filename(BlockDriverState *bs, > char *dest, size_t sz); These are specific to QCOW etc., so bool qbdrvs_has_backing_files(BlockDriverState *bs)? > > > /* advanced image content access */ > int qbdrvs_is_allocated(BlockDriverState *bs, int64_t sector_num, int > nb_sectors, > int *pnum); > int qbdrvs_discard(BlockDriverState *bs, int64_t sector_num, int > nb_sectors); Again, some files do not have a concept of allocation. > int qbdrvs_has_zero_init(BlockDriverState *bs); > > >> Il 16/07/2012 10:16, Wenchao Xia ha scritto: >>>> >>>> >>> Really thanks for the investigation, I paid quite sometime to dig out >>> which license is compatible to LGPL, this have sorted it out. >>> The coroutine and structure inside is quite a challenge. >> >> >> Coroutines are really just a small complication in the program flow if >> all you support is synchronous access to files (i.e. no HTTP etc.). >> Their usage should be completely transparent. >> >>> What about >>> provide the library first in nbd + sync access, and waiting for the >>> library employer response? If it is good to use, then replace implement >>> code to native qemu block layer code, change code's license, while keep >>> API unchanged. >> >> >> You can start by proposing the API. >> >> Paolo >> > > > -- > Best Regards > > Wenchao Xia > > >
