On 14/06/2021 10:01, Philippe Mathieu-Daudé wrote:
On 6/14/21 9:44 AM, Mark Cave-Ayland wrote:
On 14/06/2021 06:42, Philippe Mathieu-Daudé wrote:
On 6/13/21 12:26 PM, Mark Cave-Ayland wrote:
Commit 4e78f3bf35 "esp: defer command completion interrupt on
incoming data
transfers" added a version check for use with VMSTATE_*_TEST macros
to allow
migration from older QEMU versions. Unfortunately the version check
fails to
work in its current form since if the VMStateDescription version_id is
incremented, the test returns false and so the fields are not
included in the
outgoing migration stream.
Change the version check to use >= rather == to ensure that migration
works
correctly when the ESPState VMStateDescription has version_id > 5.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Fixes: 4e78f3bf35 ("esp: defer command completion interrupt on
incoming data transfers")
---
Well, it is not buggy yet :)
:)
hw/scsi/esp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index bfdb94292b..39756ddd99 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -1120,7 +1120,7 @@ static bool esp_is_version_5(void *opaque, int
version_id)
Can you rename esp_is_at_least_version_5()?
Sure, I can rename it if you like but it will of course make the diff
noisier. esp_is_at_least_version_5() seems quite a mouthful though, what
about esp_min_version_5() instead?
I was looking at esp_is_before_version_5(). Following that logic it
should be named esp_is_after_version_4()? Or esp_min_version_5() and
rename esp_is_before_version_5() -> esp_max_version_4(). All options
seem confuse...
Maybe _V macros suggested by Paolo make all clearer?
Unfortunately the _V macros don't work correctly here (see my previous
reply to Paolo) which is why these functions exist in the first place.
If all the proposed options seem equally confusing, is it worth just
sticking with what was in the original patch? Otherwise we end up with a
whole series renaming functions in a way we're still not happy with,
compared with the original patch which is effectively a diff of 1
character.