[Qemu-devel] [PATCH] usb-ccid: remove needless migration state code

Marc-André Lureau posted 1 patch 6 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20171012112157.15647-1-marcandre.lureau@redhat.com
Test checkpatch passed
Test docker passed
Test s390x passed
There is a newer version of this series
hw/usb/dev-smartcard-reader.c | 22 ----------------------
1 file changed, 22 deletions(-)
[Qemu-devel] [PATCH] usb-ccid: remove needless migration state code
Posted by Marc-André Lureau 6 years, 6 months ago
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(-)

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()
     }
-- 
2.15.0.rc0.40.gaefcc5f6f


Re: [Qemu-devel] [PATCH] usb-ccid: remove needless migration state code
Posted by Gerd Hoffmann 6 years, 6 months ago
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


Re: [Qemu-devel] [PATCH] usb-ccid: remove needless migration state code
Posted by Marc-André Lureau 6 years, 6 months ago
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


-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH] usb-ccid: remove needless migration state code
Posted by Paolo Bonzini 6 years, 6 months ago
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