> -----Original Message-----
> From: Michael S. Tsirkin [mailto:m...@redhat.com]
> Sent: Wednesday, February 14, 2018 12:46 AM
> To: Zhoujian (jay) <jianjay.z...@huawei.com>
> Cc: qemu-devel@nongnu.org; pbonz...@redhat.com; Huangweidong (C)
> <weidong.hu...@huawei.com>; stefa...@redhat.com; pa...@linux.vnet.ibm.com;
> longpeng <longpe...@huawei.com>; xin.z...@intel.com; roy.fan.zh...@intel.com;
> Gonglei (Arei) <arei.gong...@huawei.com>; wangxin (U)
> <wangxinxin.w...@huawei.com>
> Subject: Re: [PATCH v6 1/4] cryptodev: add vhost-user as a new cryptodev
> backend
>
> On Sun, Jan 21, 2018 at 08:54:47PM +0800, Jay Zhou wrote:
> > diff --git a/backends/cryptodev-vhost-user.c
> > b/backends/cryptodev-vhost-user.c new file mode 100644 index
> > 0000000..4e63ece
> > --- /dev/null
> > +++ b/backends/cryptodev-vhost-user.c
> > @@ -0,0 +1,333 @@
> > +/*
> > + * QEMU Cryptodev backend for QEMU cipher APIs
> > + *
> > + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD.
> > + *
> > + * Authors:
> > + * Gonglei <arei.gong...@huawei.com>
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see
> <http://www.gnu.org/licenses/>.
> > + *
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/boards.h"
> > +#include "qapi/error.h"
> > +#include "qapi/qmp/qerror.h"
> > +#include "qemu/error-report.h"
> > +#include "standard-headers/linux/virtio_crypto.h"
> > +#include "sysemu/cryptodev-vhost.h"
> > +#include "chardev/char-fe.h"
> > +
> > +
> > +/**
> > + * @TYPE_CRYPTODEV_BACKEND_VHOST_USER:
> > + * name of backend that uses vhost user server */ #define
> > +TYPE_CRYPTODEV_BACKEND_VHOST_USER "cryptodev-vhost-user"
> > +
> > +#define CRYPTODEV_BACKEND_VHOST_USER(obj) \
> > + OBJECT_CHECK(CryptoDevBackendVhostUser, \
> > + (obj), TYPE_CRYPTODEV_BACKEND_VHOST_USER)
> > +
> > +
> > +typedef struct CryptoDevBackendVhostUser {
> > + CryptoDevBackend parent_obj;
> > +
> > + CharBackend chr;
> > + char *chr_name;
> > + bool opened;
> > + CryptoDevBackendVhost *vhost_crypto[MAX_CRYPTO_QUEUE_NUM];
> > +} CryptoDevBackendVhostUser;
> > +
> > +static int
> > +cryptodev_vhost_user_running(
> > + CryptoDevBackendVhost *crypto) {
> > + return crypto ? 1 : 0;
> > +}
> > +
> > +static void cryptodev_vhost_user_stop(int queues,
> > + CryptoDevBackendVhostUser *s) {
> > + size_t i;
> > +
> > + for (i = 0; i < queues; i++) {
> > + if (!cryptodev_vhost_user_running(s->vhost_crypto[i])) {
> > + continue;
> > + }
> > +
> > + if (s->vhost_crypto) {
> > + cryptodev_vhost_cleanup(s->vhost_crypto[i]);
> > + s->vhost_crypto[i] = NULL;
> > + }
> > + }
> > +}
>
> This test is problematic: clang build triggers an error:
> > /home/petmay01/linaro/qemu-for-merges/backends/cryptodev-vhost-user.c:86:16:
> > error: address of array 's->vhost_crypto' will always evaluate to
> > 'true' [-Werror,-Wpointer-bool-conversion]
> > if (s->vhost_crypto) {
> > ~~ ~~~^~~~~~~~~~~~
This line should be
if (s->vhost_crypto[i]) {
>
> I really don't see how this could do the right thing, which makes me suspect
> that either you did not test stop, or you always have all queues enabled.
>
> Pls test a config with some queues disabled.
>
> In particular this machinery needs some unit tests to catch errors like this.
Okay, will do more tests, sorry about that.
Regards,
Jay
>
>
> --
> MST
