Re: [PATCH v2 03/20] vhost-user-gpu: use an extandable state enum for commands

[Philippe Mathieu-Daudé]

On 2/4/21 12:26 PM, Philippe Mathieu-Daudé wrote:
> On 2/4/21 11:52 AM, marcandre.lureau@redhat.com wrote:
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> Introduce a pending state for commands which aren't finished yet, but
>> are being handled. See following patch.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>  contrib/vhost-user-gpu/vugpu.h          | 8 +++++++-
>>  contrib/vhost-user-gpu/vhost-user-gpu.c | 8 ++++----
>>  contrib/vhost-user-gpu/virgl.c          | 2 +-
>>  3 files changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/contrib/vhost-user-gpu/vugpu.h b/contrib/vhost-user-gpu/vugpu.h
>> index 86f3ac86aa..04d5615812 100644
>> --- a/contrib/vhost-user-gpu/vugpu.h
>> +++ b/contrib/vhost-user-gpu/vugpu.h
>> @@ -129,12 +129,18 @@ typedef struct VuGpu {
>>      QTAILQ_HEAD(, virtio_gpu_ctrl_command) fenceq;
>>  } VuGpu;
>>  
>> +enum {
>> +    VG_CMD_STATE_NEW,
> 
> Maybe VG_CMD_STATE_STARTING?
> 
>> +    VG_CMD_STATE_PENDING,
> 
> Maybe introduce VG_CMD_STATE_PENDING in the
> patch using it.
> 
>> +    VG_CMD_STATE_FINISHED,
>> +};
> 
> Can we use a typedef ...
> 
>> +
>>  struct virtio_gpu_ctrl_command {
>>      VuVirtqElement elem;
>>      VuVirtq *vq;
>>      struct virtio_gpu_ctrl_hdr cmd_hdr;
>>      uint32_t error;
>> -    bool finished;
>> +    int state;
> 
> ... and use it here?
> 
> Or directly declare in place:
> 
>        enum {
>            VG_CMD_STATE_STARTING,
>            VG_CMD_STATE_PENDING,
>            VG_CMD_STATE_FINISHED,
>        } state;
> 
>>      QTAILQ_ENTRY(virtio_gpu_ctrl_command) next;
>>  };
>>  
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Since Gerd was willing to queue v1, feel free to ignore my
comments at this point. R-b stands.