From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34134) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XNAJq-0001fW-Ua for qemu-devel@nongnu.org; Thu, 28 Aug 2014 20:45:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XNAJj-00025D-Ju for qemu-devel@nongnu.org; Thu, 28 Aug 2014 20:45:34 -0400 Received: from mx1.redhat.com ([209.132.183.28]:1736) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XNAJj-000251-BV for qemu-devel@nongnu.org; Thu, 28 Aug 2014 20:45:27 -0400 Date: Fri, 29 Aug 2014 08:45:37 +0800 From: Fam Zheng Message-ID: <20140829004537.GC4196@T430.redhat.com> References: <1409205191-11406-1-git-send-email-famz@redhat.com> <20140828152224.GA4970@irqsave.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20140828152224.GA4970@irqsave.net> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3] block: Introduce "null" driver List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?iso-8859-1?Q?Beno=EEt?= Canet Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi , armbru@redhat.com On Thu, 08/28 17:22, Beno=EEt Canet wrote: > The Thursday 28 Aug 2014 =E0 13:53:11 (+0800), Fam Zheng wrote : > > This is an analogue to Linux null_blk. It can be used for testing blo= ck > > device emulation and general block layer functionalities such as > > coroutines and throttling, where disk IO is not necessary or wanted. > >=20 > > Use null:// for AIO version, and null-co:// for coroutine version. > >=20 > > Signed-off-by: Fam Zheng > >=20 > > --- > > V3: Drop copy&paste blkdebug structure. (Beniot) > >=20 > > V2: Don't #ifdef code, add two drivers. (Benoit) > > Add to QAPI BlockdevOptions. (Eric) > > Add "file.size" option to override backend size. (What is a bette= r > > way to associate /dev/vd{a,b,c} with command line devices, if siz= es > > are the same?) > > --- > > block/Makefile.objs | 1 + > > block/null.c | 179 +++++++++++++++++++++++++++++++++++++++++= ++++++++++ > > qapi/block-core.json | 19 +++++- > > 3 files changed, 197 insertions(+), 2 deletions(-) > > create mode 100644 block/null.c > >=20 > > diff --git a/block/Makefile.objs b/block/Makefile.objs > > index 858d2b3..087e281 100644 > > --- a/block/Makefile.objs > > +++ b/block/Makefile.objs > > @@ -9,6 +9,7 @@ block-obj-y +=3D snapshot.o qapi.o > > block-obj-$(CONFIG_WIN32) +=3D raw-win32.o win32-aio.o > > block-obj-$(CONFIG_POSIX) +=3D raw-posix.o > > block-obj-$(CONFIG_LINUX_AIO) +=3D linux-aio.o > > +block-obj-y +=3D null.o > > =20 > > ifeq ($(CONFIG_POSIX),y) > > block-obj-y +=3D nbd.o nbd-client.o sheepdog.o > > diff --git a/block/null.c b/block/null.c > > new file mode 100644 > > index 0000000..171fd19 > > --- /dev/null > > +++ b/block/null.c > > @@ -0,0 +1,179 @@ > > +/* > > + * Null block driver > > + * > > + * Authors: > > + * Fam Zheng > > + * > > + * Copyright (C) 2014 Red Hat, Inc. > > + * > > + * This work is licensed under the terms of the GNU GPL, version 2 o= r later. > > + * See the COPYING file in the top-level directory. > > + */ > > + > > +#include "block/block_int.h" > > + > > +typedef struct { > > + int64_t length; > > +} BDRVNullState; > > + > > +static QemuOptsList runtime_opts =3D { > > + .name =3D "null", > > + .head =3D QTAILQ_HEAD_INITIALIZER(runtime_opts.head), > > + .desc =3D { > > + { > > + .name =3D "filename", > > + .type =3D QEMU_OPT_STRING, > > + .help =3D "", > > + }, >=20 > You seems to define filename both here and in QMP but null_file_open do= n't use it neither > any part of the code. >=20 > Do you declare it only to fool some other part of the block layer ? Yeah, it has to absorb the "filename" option in null_file_open. The value= would be "null://" or "null-co://", and is parsed in generic open code. >=20 > > + { > > + .name =3D BLOCK_OPT_SIZE, > > + .type =3D QEMU_OPT_SIZE, > > + .help =3D "size of the null block", > > + }, > > + { /* end of list */ } > > + }, > > +}; > > + > > +static int null_file_open(BlockDriverState *bs, QDict *options, int = flags, > > + Error **errp) > > +{ > > + QemuOpts *opts; > > + BDRVNullState *s =3D bs->opaque; > > + > > + opts =3D qemu_opts_create(&runtime_opts, NULL, 0, &error_abort); > > + qemu_opts_absorb_qdict(opts, options, &error_abort); > > + s->length =3D > > + qemu_opt_get_size(opts, BLOCK_OPT_SIZE, 1 << 30); > > + qemu_opts_del(opts); > > + return 0; > > +} > > + > > +static void null_close(BlockDriverState *bs) > > +{ > > +} > > + > > +static int64_t null_getlength(BlockDriverState *bs) > > +{ > > + BDRVNullState *s =3D bs->opaque; > > + return s->length; > > +} > > + > > +static coroutine_fn int null_co_read(BlockDriverState *bs, int64_t s= ector_num, > > + uint8_t *buf, int nb_sectors) > > +{ > > + return 0; > > +} > > + > > +static coroutine_fn int null_co_write(BlockDriverState *bs, int64_t = sector_num, > > + const uint8_t *buf, int nb_sec= tors) > > +{ > > + return 0; > > +} > > + > > +static coroutine_fn int null_co_flush(BlockDriverState *bs) > > +{ > > + return 0; > > +} > > + > > +typedef struct { > > + BlockDriverAIOCB common; > > + QEMUBH *bh; > > +} NullAIOCB; > > + > > +static void null_aio_cancel(BlockDriverAIOCB *blockacb); > > + > > +static const AIOCBInfo null_aiocb_info =3D { > > + .aiocb_size =3D sizeof(NullAIOCB), > > + .cancel =3D null_aio_cancel, > > +}; > > + > > +static void null_bh_cb(void *opaque) > > +{ > > + NullAIOCB *acb =3D opaque; > > + acb->common.cb(acb->common.opaque, 0); > > + qemu_bh_delete(acb->bh); > > + qemu_aio_release(acb); > > +} > > + > > +static BlockDriverAIOCB *null_aio_readv(BlockDriverState *bs, > > + int64_t sector_num, QEMUIOVe= ctor *qiov, > > + int nb_sectors, > > + BlockDriverCompletionFunc *c= b, > > + void *opaque) > > +{ > > + NullAIOCB *acb; > > + > > + acb =3D qemu_aio_get(&null_aiocb_info, bs, cb, opaque); > > + acb->bh =3D aio_bh_new(bdrv_get_aio_context(bs), null_bh_cb, acb= ); > > + qemu_bh_schedule(acb->bh); > > + return &acb->common; > > +} > > + > > +static BlockDriverAIOCB *null_aio_writev(BlockDriverState *bs, > > + int64_t sector_num, QEMUIOV= ector *qiov, > > + int nb_sectors, > > + BlockDriverCompletionFunc *= cb, > > + void *opaque) > > +{ > > + NullAIOCB *acb; > > + > > + acb =3D qemu_aio_get(&null_aiocb_info, bs, cb, opaque); > > + acb->bh =3D aio_bh_new(bdrv_get_aio_context(bs), null_bh_cb, acb= ); > > + qemu_bh_schedule(acb->bh); > > + return &acb->common; > > +} > > + > > +static BlockDriverAIOCB *null_aio_flush(BlockDriverState *bs, > > + BlockDriverCompletionFunc *c= b, > > + void *opaque) > > +{ > > + NullAIOCB *acb; > > + > > + acb =3D qemu_aio_get(&null_aiocb_info, bs, cb, opaque); > > + acb->bh =3D aio_bh_new(bdrv_get_aio_context(bs), null_bh_cb, acb= ); > > + qemu_bh_schedule(acb->bh); > > + return &acb->common; > > +} >=20 > The former three function body are strictly identical maybe you could f= actorize them. > They would become thin wrapper around one common function. Good idea! >=20 > > + > > +static void null_aio_cancel(BlockDriverAIOCB *blockacb) > > +{ > > + NullAIOCB *acb =3D container_of(blockacb, NullAIOCB, common); > > + qemu_bh_delete(acb->bh); > > + qemu_aio_release(acb); > > +} > > + > > +static BlockDriver bdrv_null =3D { > > + .format_name =3D "null", > > + .protocol_name =3D "null", > > + .instance_size =3D sizeof(BDRVNullState), > > + > > + .bdrv_file_open =3D null_file_open, > > + .bdrv_close =3D null_close, > > + .bdrv_getlength =3D null_getlength, > > + > > + .bdrv_aio_readv =3D null_aio_readv, > > + .bdrv_aio_writev =3D null_aio_writev, > > + .bdrv_aio_flush =3D null_aio_flush, > > +}; > > + > > +static BlockDriver bdrv_null_co =3D { > > + .format_name =3D "null-co", > > + .protocol_name =3D "null-co", > > + .instance_size =3D sizeof(BDRVNullState), > > + > > + .bdrv_file_open =3D null_file_open, > > + .bdrv_close =3D null_close, > > + .bdrv_getlength =3D null_getlength, > > + > > + .bdrv_read =3D null_co_read, > > + .bdrv_write =3D null_co_write, > > + .bdrv_co_flush_to_disk =3D null_co_flush, > > +}; > > + > > +static void bdrv_null_init(void) > > +{ > > + bdrv_register(&bdrv_null); > > + bdrv_register(&bdrv_null_co); > > +} > > + > > +block_init(bdrv_null_init); > > diff --git a/qapi/block-core.json b/qapi/block-core.json > > index fb74c56..9b2c9c6 100644 > > --- a/qapi/block-core.json > > +++ b/qapi/block-core.json > > @@ -1150,7 +1150,8 @@ > > 'data': [ 'archipelago', 'file', 'host_device', 'host_cdrom', 'hos= t_floppy', > > 'http', 'https', 'ftp', 'ftps', 'tftp', 'vvfat', 'blkdeb= ug', > > 'blkverify', 'bochs', 'cloop', 'cow', 'dmg', 'parallels'= , 'qcow', > > - 'qcow2', 'qed', 'raw', 'vdi', 'vhdx', 'vmdk', 'vpc', 'qu= orum' ] } > > + 'qcow2', 'qed', 'raw', 'vdi', 'vhdx', 'vmdk', 'vpc', 'qu= orum', > > + 'null' ] } >=20 > Why not also adding null-co to QMP ? Will do. Fam