virttest.env_process: Keep vm.instance throughout the workflow #2020
Conversation
We use the vm.instance as a key to our screendump status dict, but the instance number might change during our evaluation. Let's get the instance at the beginning of our workflow and keep it everywhere to avoid assigning to non-existing keys. Signed-off-by: Lukáš Doktor <[email protected]>
| counter[vm.instance] += 1 | ||
| filename = "%04d.jpg" % counter[vm.instance] | ||
| counter[instance] += 1 | ||
| filename = "%04d.jpg" % counter[instance] |
There was a problem hiding this comment.
if vm.instance changes, counter[instance] will starts from 1, so all previous image files will be overwritten. I believe it is better to relate vm.instance with screendump_dir.
|
I'm sorry, I don't really have time to properly look at that other PR. The purpose of this PR is not to change the way screendump works, it only fixes a bug where processing of a single screendump might result in crash/traceback. It happens when we check whether It is not critical, I'm seeing this issue for many years, but never had time to send a patch. Anyway this patch does not affect anything apart of the single case where VM re-creates during processing of the single screendump (we get the new Btw at first glance I think the other PR suffers the same here: b62f707#diff-4ad1e2a008c3f4477e993d5e26652b0fR567 |
|
Yes, patch #2016 suffers from the same issue, but is the overwritten of previous screendump files after migration( |
|
Yes, I've been seeing it in my logs from time to time for years, just never cared enough to send it. Finally now I did. Would you please incorporate it in your screendump rework? Or should I send it after that one is merged? |
|
You could keep this patch, after its merge, I will re-evaluate this and modify my rework then. |
We use the vm.instance as a key to our screendump status dict, but the
instance number might change during our evaluation. Let's get the
instance at the beginning of our workflow and keep it everywhere to
avoid assigning to non-existing keys.
Signed-off-by: Lukáš Doktor [email protected]