Re: [Qemu-devel] [PATCH RFC] ps2: set the keybord output buffer size as the same as kernel

[Juan Quintela]

Gerd Hoffmann <address@hidden> wrote:
> On Di, 2014-04-22 at 08:16 +0000, Gonglei (Arei) wrote:
>> > 
>> > > diff --git a/hw/input/ps2.c b/hw/input/ps2.c
>> > > index 3412079..a754fef 100644
>> > > --- a/hw/input/ps2.c
>> > > +++ b/hw/input/ps2.c
>> > > @@ -71,7 +71,7 @@
>> > >  #define MOUSE_STATUS_ENABLED    0x20
>> > >  #define MOUSE_STATUS_SCALE21    0x10
>> > >
>> > > -#define PS2_QUEUE_SIZE 256
>> > > +#define PS2_QUEUE_SIZE 16     /* Keyboard output buffer size */
>> > >
>> > >  typedef struct {
>> > >      uint8_t data[PS2_QUEUE_SIZE];
>> > 
>> > This changes ps2 vmstate and breaks live migration.
>> > 
>> Good catch, Gerd.
>> I got the information in the destination of live migration:
>> Unknown savevm section type 24
>> load of migration failed
>> 
>> I'm not familiar with the situation of cross-version live migration,
>> could you give me
>> some guide ? Thanks.
>
> Keep the data array 256 bytes long, best with a comment that
> compatibility with older qemu versions requires this.
>
> Also the post_load function must handle the case that rptr, wptr & count
> variables have values which used to be valid for the older qemu versions
> but are not valid any more with the smaller queue.  In the (unlikely)
> case that count is larger than 16 the best you can do is probably simply
> throw away the queue.  16 and less queue elements you can move to the
> start of the data array (so they are within the 16 bytes still used
> after your patch is merged) and adjust rptr+wptr accordingly.
>
> Cc'ing Juan for additional insights.
>
> HTH,
>   Gerd


static int ps2_common_post_load(void *opaque, int version_id)
{
    PS2State *s = opaque;

    /* Here goes the code that resets rptr/wptr/count if it is bigger
       than p16
       Gerd said that dropping the queue is a good idea.
     */

    return 0;
}

static const VMStateDescription vmstate_ps2_common = {
    .name = "PS2 Common State",
    .version_id = 3,
    .minimum_version_id = 2,
    .minimum_version_id_old = 2,
    .post_load = ps2_common_post_load,
    .fields = (VMStateField[]) {
        VMSTATE_INT32(write_cmd, PS2State),
        VMSTATE_INT32(queue.rptr, PS2State),
        VMSTATE_INT32(queue.wptr, PS2State),
        VMSTATE_INT32(queue.count, PS2State),
        VMSTATE_BUFFER(queue.data, PS2State),
        /* A coment here explaining why we changed the queue from 256 to
          16 could be a good idea */
        VMSTATE_UNUSED_BUFFER(256-16 );
        VMSTATE_END_OF_LIST()
    }
};


Hope it helps.

Later, Juan.