Ekaterina Tumanova <tuman...@linux.vnet.ibm.com> writes: > On 11/27/2014 05:55 PM, Markus Armbruster wrote: >> I'm sorry for the delay. I got the flu and have difficulties thinking >> straight for longer than a few minutes. >> >> Ekaterina Tumanova <tuman...@linux.vnet.ibm.com> writes: >> >>> Add driver functions for geometry and blocksize detection >>> >>> Signed-off-by: Ekaterina Tumanova <tuman...@linux.vnet.ibm.com> >>> --- >>> block.c | 26 ++++++++++++++++++++++++++ >>> include/block/block.h | 20 ++++++++++++++++++++ >>> include/block/block_int.h | 3 +++ >>> 3 files changed, 49 insertions(+) >>> >>> diff --git a/block.c b/block.c >>> index a612594..5df35cf 100644 >>> --- a/block.c >>> +++ b/block.c >>> @@ -548,6 +548,32 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error >>> **errp) >>> } >>> } >>> >>> +struct ProbeBlockSize bdrv_probe_blocksizes(BlockDriverState *bs) >>> +{ >>> + BlockDriver *drv = bs->drv; >>> + struct ProbeBlockSize err_geo = { .rc = -1 }; >>> + >>> + assert(drv != NULL); >>> + if (drv->bdrv_probe_blocksizes) { >>> + return drv->bdrv_probe_blocksizes(bs); >>> + } >>> + >>> + return err_geo; >>> +} >> >> I'm not sure about "probe". Naming is hard. "get"? >> > There was already "bdrv_get_geometry", and I wanted the _geometry and > _blocksize functions to use the same naming convention.
Fair enough. bdrv_get_geometry() is a silly wrapper around bdrv_nb_sectors() that maps failure to zero sectors. I hope to kill it some time. Doesn't help you now. > I thought probe might be more suitable, since we do not "get" the value > for sure. maybe "detect"? Feel free to stick to "probe". >> Squashing status and actual payload into a single struct to use as >> return type isn't wrong, but unusual. When the payload can't represent >> failure conveniently, we usually return status, and write the payload to >> a buffer provided by the caller, like this: >> >> int bdrv_get_blocksizes(BlockDriverState *bs, >> uint16_t *physical_blk_sz, >> uint16_t *logical_blk_sz) >> >> Or, with a struct to hold both sizes: >> >> int bdrv_get_blocksizes(BlockDriverState *bs, BlockSizes *bsz) >> > Do you insist on changing that? Returning a struct via stack seemed > useful to me, since there was less probability of caller allocating > a buffer of incorrect size or smth like that. You'd have to do crazy stuff to defeat the static type checker. Please stick to the common technique. [...]