From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47934) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1brMBB-0002LQ-PK for qemu-devel@nongnu.org; Tue, 04 Oct 2016 05:38:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1brMB6-0008F2-LO for qemu-devel@nongnu.org; Tue, 04 Oct 2016 05:38:28 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37974) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1brMB6-0008Eg-B6 for qemu-devel@nongnu.org; Tue, 04 Oct 2016 05:38:24 -0400 Date: Tue, 4 Oct 2016 10:38:20 +0100 From: Stefan Hajnoczi Message-ID: <20161004093820.GE4587@stefanha-x1.localdomain> References: <1475051152-400276-1-git-send-email-arei.gonglei@huawei.com> <1475051152-400276-6-git-send-email-arei.gonglei@huawei.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="7CZp05NP8/gJM8Cl" Content-Disposition: inline In-Reply-To: <1475051152-400276-6-git-send-email-arei.gonglei@huawei.com> Subject: Re: [Qemu-devel] [PATCH v4 05/13] virtio-crypto: add virtio crypto device emulation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gonglei Cc: qemu-devel@nongnu.org, virtio-dev@lists.oasis-open.org, luonengjun@huawei.com, mst@redhat.com, pbonzini@redhat.com, berrange@redhat.com, weidong.huang@huawei.com, wu.wubin@huawei.com, mike.caraman@nxp.com, agraf@suse.de, xin.zeng@intel.com, claudio.fontana@huawei.com, nmorey@kalray.eu, vincent.jardin@6wind.com, jianjay.zhou@huawei.com, hanweidong@huawei.com, peter.huangpeng@huawei.com --7CZp05NP8/gJM8Cl Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Sep 28, 2016 at 04:25:44PM +0800, Gonglei wrote: > Introduce the virtio crypto realization, I'll > finish the core code in the following patches. The > thoughts came from virtio net realization. >=20 > For more information see: > http://qemu-project.org/Features/VirtioCrypto This patch contains functions and struct fields that are unused. It would make code review easier if you add them when they are needed throughout the patch series. > +static void virtio_crypto_device_realize(DeviceState *dev, Error **errp) > +{ > + VirtIODevice *vdev =3D VIRTIO_DEVICE(dev); > + VirtIOCrypto *vcrypto =3D VIRTIO_CRYPTO(dev); > + int i; > + > + vcrypto->cryptodev =3D vcrypto->conf.cryptodev; > + if (vcrypto->cryptodev =3D=3D NULL) { > + error_setg(errp, "'cryptodev' parameter expects a valid object"); > + return; > + } > + > + vcrypto->max_queues =3D MAX(vcrypto->cryptodev->conf.peers.queues, 1= ); > + if (vcrypto->max_queues + 1 > VIRTIO_QUEUE_MAX) { > + error_setg(errp, "Invalid number of queues (=3D %" PRIu16 "), " > + "must be a postive integer less than %d.", > + vcrypto->max_queues, VIRTIO_QUEUE_MAX - 1); The error message is off by 1: must be a positive integer less than VIRTIO_QUEUE_MAX > + return; > + } > + > + virtio_init(vdev, "virtio-crypto", VIRTIO_ID_CRYPTO, vcrypto->config= _size); > + vcrypto->curr_queues =3D 1; > + > + for (i =3D 0; i < vcrypto->max_queues; i++) { > + virtio_add_queue(vdev, 1024, virtio_crypto_handle_dataq); > + } > + > + vcrypto->ctrl_vq =3D virtio_add_queue(vdev, 64, virtio_crypto_handle= _ctrl); > + if (!vcrypto->cryptodev->ready) { > + vcrypto->status &=3D ~VIRTIO_CRYPTO_S_HW_READY; > + } else { > + vcrypto->status |=3D VIRTIO_CRYPTO_S_HW_READY; > + } > + register_savevm(dev, "virtio-crypto", -1, 1, virtio_crypto_save, > + virtio_crypto_load, vcrypto); Please use VMSTATE_VIRTIO_DEVICE() instead of calling register_savevm(). > +#ifdef DEBUG_VIRTIO_CRYPTO > +#define DPRINTF(fmt, ...) \ > +do { printf("virtio_crypto: " fmt , ## __VA_ARGS__); } while (0) > +#else > +#define DPRINTF(fmt, ...) do { } while (0) > +#endif Please use tracing (see docs/tracing.txt) or if you really want to use debug printfs then define DPRINTF() in a way that prevents bitrot: #define DEBUG_VIRTIO_CRYPTO 0 #define DPRINTF(fmt, ...) \ do { \ if (DEBUG_VIRTIO_CRYPTO) { \ fprintf(stderr, "virtio_crypto: " fmt, ##__VA_ARGS__); \ } \ } while (0) --7CZp05NP8/gJM8Cl Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEcBAEBAgAGBQJX83iMAAoJEJykq7OBq3PILj8IAMB1KAZDDR2ZYdJPvWpZ+bDw uC7+TuZJj/sbKPHkBasT0Q9c5eOcf51APrgwdK+k55jmaS5jujj6ln7XrakTbt9h tEwI2OQMTSx5knGv7LW7FgE9Uv91I/tQh9XxcAQh7U5TXUktHiunvfNByw+lNISn 8u0cd+SsAmfYThzWQOxE53NAscqguAxuI12WSQtnwXxHdcrgtRRpW1S/B0FnPV/V aBRYFHIyUb8m60fFGzeH2FEKi3VvFp5YWi4W2gm5ZuS9vcCBfjFiayoag3O4X0HB dacS8o1NBW1NT+Cjc+W0pdLK2eW2D0wXlS4RRiKGSjNna2AB4boE9o64F8xMbUY= =rEaS -----END PGP SIGNATURE----- --7CZp05NP8/gJM8Cl--