> > On 05/17/2017 11:12 AM, Gonglei (Arei) wrote: > >>>> By the way, I'm having a hard time understing how is the requirement > form > >>>> > >> > http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-260 > >>>> 004 > >>>> (2.4.4 Message Framing) satisfied by this code. Could you explain this > >>>> to me please? > >>>> > >>> Sorry for my bad English, > >>> I don't know which normative formulation the code violates? > >> I'm not sure it's actually violated, but I'm concerned about the following > >> normative statement: "The device MUST NOT make assumptions about the > >> particular > >> arrangement of descriptors. The device MAY have a reasonable limit of > >> descriptors it will allow in a chain." > >> > >> Please also read the explanatory part I've linked, because the normative > >> statement is pretty abstract. > >> > >> In my understanding, the spec says, that e.g. the virti-crypto device > >> should not fail if a single request is split up into let's say two chunks > >> and transmitted by the means of two top level descriptors. > >> > >> Do you agree with my reading of the spec? > >> > > Yes, I agree. But what's the relationship about the request here? > > We don't assume the request have to use one desc entity, it can > > use scatter-gather list for one request header. > > The device can cover the situation in the QEMU. > > > >> What does the virtio-crypto device do if it encounters such a situation? > >> > > This isn't a problem. Pls see blow code segment: > > > > virtio_crypto_handle_request() > > {... > > if (unlikely(iov_to_buf(out_iov, out_num, 0, &req, sizeof(req)) > > != sizeof(req))) { > > virtio_error(vdev, "virtio-crypto request outhdr too short"); > > return -1; > > } > > iov_discard_front(&out_iov, &out_num, sizeof(req)); > > ... > > > > Thats exactly what worries me. I see a call to virtio_error there... > > > void GCC_FMT_ATTR(2, 3) virtio_error(VirtIODevice *vdev, const char *fmt, ...) > { > va_list ap; > > va_start(ap, fmt); > error_vreport(fmt, ap); > va_end(ap); > > vdev->broken = true; > > if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) { > virtio_set_status(vdev, vdev->status | > VIRTIO_CONFIG_S_NEEDS_RESET); > virtio_notify_config(vdev); > } > } > > This however seems to make the device 'broken'. Or am I missing something? > Yes, if the parse process failed, the device will broke. But This is a exception scenario IMHO, which is the same situation with other virtio devices.
Stefan introduced the virtio_error(). Thanks, -Gonglei
