From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:38776) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Stwfm-00084s-8N for qemu-devel@nongnu.org; Wed, 25 Jul 2012 04:10:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Stwfh-00062r-Pb for qemu-devel@nongnu.org; Wed, 25 Jul 2012 04:10:22 -0400 Received: from e28smtp01.in.ibm.com ([122.248.162.1]:51343) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Stwfg-0005mh-Of for qemu-devel@nongnu.org; Wed, 25 Jul 2012 04:10:17 -0400 Received: from /spool/local by e28smtp01.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 25 Jul 2012 13:39:57 +0530 Received: from d28av05.in.ibm.com (d28av05.in.ibm.com [9.184.220.67]) by d28relay01.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q6P89rHk28901434 for ; Wed, 25 Jul 2012 13:39:54 +0530 Received: from d28av05.in.ibm.com (loopback [127.0.0.1]) by d28av05.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q6P89qQl027344 for ; Wed, 25 Jul 2012 18:09:52 +1000 Message-ID: <500FA974.6090209@linux.vnet.ibm.com> Date: Wed, 25 Jul 2012 16:08:20 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <4FFA9C30.2070201@linux.vnet.ibm.com> <4FFAA0C3.3080703@redhat.com> <4FFBB7FB.3070303@linux.vnet.ibm.com> <4FFBD6F1.90403@redhat.com> <20120713091611.GC15503@stefanha-thinkpad.localdomain> <4FFFEF8E.5080705@redhat.com> <50000793.2020401@redhat.com> <5003CDC6.2040103@linux.vnet.ibm.com> <5003CE8B.20804@redhat.com> <500678F7.1030705@linux.vnet.ibm.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [RFC] introduce a dynamic library to expose qemu block API List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: Anthony Liguori , Stefan Hajnoczi , Michael Tokarev , =?UTF-8?B?TGx1w61z?= , qemu-devel@nongnu.org, Stefan Weil , Hannes Reinecke , Paolo Bonzini 于 2012-7-24 2:15, Blue Swirl 写道: > On Wed, Jul 18, 2012 at 8:51 AM, Wenchao Xia 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? > Returning negative value should indicate errors. I plan to insert a qblock-err.h to define all number there, and then a function of const char *qberr2str(int err) to translate error. > Alternative version with file descriptor or struct FILE instead of > filename might become useful but can be added later. > Agree. >> 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. > not sure why original API used sector instead of raw address, if no special requirement for sector I think raw address is better. > 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); > I hope to package the info into a structure, such as #define QBLOCK_INFO_SIZE (128) struct QblockInfo { int dirty; char *format; int reserved[RESERVE_SPACE]; }; assert sizeof(QblockInfo) == QBLOCK_INFO_SIZE in every version. This will make caller's code more clean. >> 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. > OK. >> //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)? > How about using return value to indicate it? int qbdrvs_get_backing_filename(BlockDriverState *bs, char *filename, int filename_size); >> >> >> /* 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. > Maybe return value could indicate it. >> 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 >> >> >> > -- Best Regards Wenchao Xia