Re: [Qemu-devel] [PATCH v2 8/8] virtio-crypto: use virtqueue_error for errors with queue context

[Cornelia Huck]

On Thu, 13 Jul 2017 17:31:59 +0200
Ladi Prosek <address@hidden> wrote:

> On Thu, Jul 13, 2017 at 5:20 PM, Cornelia Huck <address@hidden> wrote:
> > On Thu, 13 Jul 2017 13:02:37 +0200
> > Ladi Prosek <address@hidden> wrote:
> >  
> >> virtqueue_error includes queue index in the error output and is preferred
> >> for errors that pertain to a virtqueue rather than to the device as a 
> >> whole.
> >>
> >> Signed-off-by: Ladi Prosek <address@hidden>
> >> ---
> >>  hw/virtio/virtio-crypto.c | 56 
> >> ++++++++++++++++++++++++-----------------------
> >>  1 file changed, 29 insertions(+), 27 deletions(-)
> >>
> >> diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
> >> index 0353eb6..08e6c03 100644
> >> --- a/hw/virtio/virtio-crypto.c
> >> +++ b/hw/virtio/virtio-crypto.c
> >> @@ -35,6 +35,7 @@ static inline int virtio_crypto_vq2q(int queue_index)
> >>
> >>  static int
> >>  virtio_crypto_cipher_session_helper(VirtIODevice *vdev,
> >> +           VirtQueue *ctrl_vq,
> >>             CryptoDevBackendSymSessionInfo *info,
> >>             struct virtio_crypto_cipher_session_para *cipher_para,
> >>             struct iovec **iov, unsigned int *out_num)
> >> @@ -61,7 +62,7 @@ virtio_crypto_cipher_session_helper(VirtIODevice *vdev,
> >>          info->cipher_key = g_malloc(info->key_len);
> >>          s = iov_to_buf(*iov, num, 0, info->cipher_key, info->key_len);
> >>          if (unlikely(s != info->key_len)) {
> >> -            virtio_error(vdev, "virtio-crypto cipher key incorrect");
> >> +            virtqueue_error(ctrl_vq, "virtio-crypto cipher key 
> >> incorrect");  
> >
> > As virtio-crypto devices always have one control queue, I would just
> > use the pointer stored in the vcrypto structure. This avoids some of
> > the cascading function signature changes.  
> 
> Will do. I wanted to be explicit to make the change more future-proof,
> but maybe it is better to keep it simple. Thanks!

I don't see a case for multiple virtio-crypto control vqs, even in the
future, but let's see what the virtio-crypto maintainer thinks.

> 
> >>              return -EFAULT;
> >>          }
> >>          iov_discard_front(iov, &num, info->key_len);