| Message ID | 20171012112157.15647-1-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | usb-ccid: remove needless migration state code | expand |
On Thu, 2017-10-12 at 13:21 +0200, Marc-André Lureau wrote: > This code appears to be unused since its introduction. It should be > safe to remove from VMState. Well, almost: > @@ -1452,7 +1431,6 @@ static VMStateDescription ccid_vmstate = { > VMSTATE_STRUCT_ARRAY(pending_answers, USBCCIDState, > PENDING_ANSWERS_NUM, 1, answer_vmstate, > Answer), > VMSTATE_UINT32(pending_answers_num, USBCCIDState), > - VMSTATE_UINT8(migration_state, USBCCIDState), > VMSTATE_UINT32(state_vmstate, USBCCIDState), > VMSTATE_END_OF_LIST() > } We have to keep this (or some placeholder) so the vmstate format doesn't change. cheers, Gerd
Hi On Thu, Oct 12, 2017 at 3:29 PM, Gerd Hoffmann <kraxel@redhat.com> wrote: > On Thu, 2017-10-12 at 13:21 +0200, Marc-André Lureau wrote: >> This code appears to be unused since its introduction. It should be >> safe to remove from VMState. > > Well, almost: > >> @@ -1452,7 +1431,6 @@ static VMStateDescription ccid_vmstate = { >> VMSTATE_STRUCT_ARRAY(pending_answers, USBCCIDState, >> PENDING_ANSWERS_NUM, 1, answer_vmstate, >> Answer), >> VMSTATE_UINT32(pending_answers_num, USBCCIDState), >> - VMSTATE_UINT8(migration_state, USBCCIDState), >> VMSTATE_UINT32(state_vmstate, USBCCIDState), >> VMSTATE_END_OF_LIST() >> } > > We have to keep this (or some placeholder) so the vmstate format > doesn't change. I did a simple migration test from a state file with the field to a state file without, and the other way around and it seemed to work. I probably screwed up the test. Looking at vmstate code, it obviously changes the wire format. So let's keep the field. (I was wondering how/if the vmdesc was able to match field names/version, probably not) I was looking for some documentation regarding VMState versionning, it doesn't seem to be documented when you need to bump version, and how to do it (struct vs version fields, compatibility with older version etc). I'll send v2 thanks
On 12/10/2017 16:02, Marc-André Lureau wrote: > I did a simple migration test from a state file with the field to a > state file without, and the other way around and it seemed to work. I > probably screwed up the test. > > Looking at vmstate code, it obviously changes the wire format. So > let's keep the field. (I was wondering how/if the vmdesc was able to > match field names/version, probably not) You can use VMSTATE_UNUSED(1) to drop the field from the struct. Paolo
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c index 0c77d2a41d..d23a4d7f7d 100644 --- a/hw/usb/dev-smartcard-reader.c +++ b/hw/usb/dev-smartcard-reader.c @@ -270,11 +270,6 @@ typedef struct BulkIn { uint32_t pos; } BulkIn; -enum { - MIGRATION_NONE, - MIGRATION_MIGRATED, -}; - typedef struct CCIDBus { BusState qbus; } CCIDBus; @@ -306,9 +301,6 @@ typedef struct USBCCIDState { CCID_ProtocolDataStructure abProtocolDataStructure; uint32_t ulProtocolDataStructureSize; uint32_t state_vmstate; - uint32_t migration_target_ip; - uint16_t migration_target_port; - uint8_t migration_state; uint8_t bmSlotICCState; uint8_t powered; uint8_t notify_slot_change; @@ -1243,9 +1235,6 @@ int ccid_card_ccid_attach(CCIDCardState *card) USBCCIDState *s = USB_CCID_DEV(dev); DPRINTF(s, 1, "CCID Attach\n"); - if (s->migration_state == MIGRATION_MIGRATED) { - s->migration_state = MIGRATION_NONE; - } return 0; } @@ -1341,9 +1330,6 @@ static void ccid_realize(USBDevice *dev, Error **errp) s->intr = usb_ep_get(dev, USB_TOKEN_IN, CCID_INT_IN_EP); s->bulk = usb_ep_get(dev, USB_TOKEN_IN, CCID_BULK_IN_EP); s->card = NULL; - s->migration_state = MIGRATION_NONE; - s->migration_target_ip = 0; - s->migration_target_port = 0; s->dev.speed = USB_SPEED_FULL; s->dev.speedmask = USB_SPEED_MASK_FULL; s->notify_slot_change = false; @@ -1379,13 +1365,6 @@ static int ccid_pre_save(void *opaque) USBCCIDState *s = opaque; s->state_vmstate = s->dev.state; - if (s->dev.attached) { - /* - * Migrating an open device, ignore reconnection CHR_EVENT to avoid an - * erroneous detach. - */ - s->migration_state = MIGRATION_MIGRATED; - } return 0; } @@ -1452,7 +1431,6 @@ static VMStateDescription ccid_vmstate = { VMSTATE_STRUCT_ARRAY(pending_answers, USBCCIDState, PENDING_ANSWERS_NUM, 1, answer_vmstate, Answer), VMSTATE_UINT32(pending_answers_num, USBCCIDState), - VMSTATE_UINT8(migration_state, USBCCIDState), VMSTATE_UINT32(state_vmstate, USBCCIDState), VMSTATE_END_OF_LIST() }
This code appears to be unused since its introduction. It should be safe to remove from VMState. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- hw/usb/dev-smartcard-reader.c | 22 ---------------------- 1 file changed, 22 deletions(-)