Re: [PATCH v3 1/2] virtio-video: Add virtio video device specification

[Keiichi Watanabe <keiichiw@chromium.org>]

Hi Dmitry,

On Thu, Apr 9, 2020 at 10:23 PM Dmitry Sepp <dmitry.sepp@opensynergy.com> wrote:
>
> Hi Keiichi,
>
> On Donnerstag, 9. April 2020 12:46:56 CEST Keiichi Watanabe wrote:
> > Hi,
> >
> > On Tue, Apr 7, 2020 at 11:49 PM Dmitry Sepp <dmitry.sepp@opensynergy.com>
> wrote:
> > > Hi,
> > >
> > > > +\item[VIRTIO_VIDEO_CMD_STREAM_DESTROY] Destroy a video stream
> > > > +  (context) within the device.
> > > > +
> > > > +\begin{lstlisting}
> > > > +struct virtio_video_stream_destroy {
> > > > +        struct virtio_video_cmd_hdr hdr;
> > > > +};
> > > > +\end{lstlisting}
> > >
> > > Let's add more strict description to stream_destroy, like as follows:
> > > Device MUST NOT generate any events for the stream in question after
> > > receiving the command. Before completing the command, Device MUST ensure
> > > that all asynchronous commands that are related to the stream have been
> > > completed and all memory objects are unreferenced.
> >
> > Sounds good. But, the device should probably be able to generate
> > VIRTIO_VIDEO_EVENT_ERROR for a device-wide error?
> > Or, should VIRTIO_VIDEO_EVENT_ERROR always be a per-stream error? (I
> > haven't documented it in v3)
> >
>
> In the current version of the driver  I have we interpret it a stream error. I
> think it makes sense as several stream formats might be backed by different
> hardware devices on the host side. So it would be an overkill to mark the
> whole virtio device as broken on the guest side.

It makes sense. I'll explicitly document that it's a per-stream error.

>
> BTW, I think we should add some hard limit to the max_cap_length and
> max_resp_length in the spec, so buggy device does not make us allocate all
> memory for a response on the host side by providing a garbage value. I think
> 4k might be a good value.

Sounds good. Thank you for the suggestion.

Best regards,
Keiichi


>
> Best regards,
> Dmitry.
>
> > Best regards,
> > Keiichi
> >
> > > Best regards,
> > > Dmitry.
>
>