Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: vnic hot un\plug - avoid race #419

Merged
merged 2 commits into from
Jul 21, 2022
Merged

Conversation

erav
Copy link
Member

@erav erav commented Jun 7, 2022

Lack of a lock in ActivateDeactivateVmNicCommand allows several
instances of hot plug and unplug requests to run in parallel causing an
inconsistent in vNIC status as well as libvirt failing the requests due
to non-unique aliases. This patch adds a lock to the command so that
each instance of the command has to finish before the next one starts.

Bug-Url: https://bugzilla.redhat.com/2084530
Change-Id: I93ac9f1f12f34526ad10b9a49790c3a2689e8c4d
Signed-off-by: Eitan Raviv eraviv@redhat.com

@erav erav requested a review from almusil as a code owner June 7, 2022 11:12
@erav erav requested a review from ahadas June 7, 2022 11:12
@almusil
Copy link
Member

almusil commented Jun 7, 2022

The change looks good, do we have some reproducer to test it with? I guess oVirt/ovirt-system-tests#175 right?

@erav
Copy link
Member Author

erav commented Jun 7, 2022

The change looks good, do we have some reproducer to test it with? I guess oVirt/ovirt-system-tests#175 right?

Yes, that is the intention of that PR, but the test is not stable and proofable yet

Copy link
Member

@mwperina mwperina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@erav
Copy link
Member Author

erav commented Jun 9, 2022

/ost network-suite-master

Copy link
Member

@ahadas ahadas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, we did the same for hot-(un)plug disks and it works well

@ahadas
Copy link
Member

ahadas commented Jun 13, 2022

/ost

@erav
Copy link
Member Author

erav commented Jun 14, 2022

/ost

1 similar comment
@erav
Copy link
Member Author

erav commented Jun 14, 2022

/ost

@erav
Copy link
Member Author

erav commented Jun 14, 2022

/ost

1 similar comment
@erav
Copy link
Member Author

erav commented Jun 14, 2022

/ost

@erav
Copy link
Member Author

erav commented Jun 14, 2022

/ost network-suite-master

1 similar comment
@erav
Copy link
Member Author

erav commented Jun 14, 2022

/ost network-suite-master

@erav
Copy link
Member Author

erav commented Jun 14, 2022

/ost network-suite-master

passed in https://redir.apps.ovirt.org/dj/job/ds-ost-baremetal_manual/40310

@erav
Copy link
Member Author

erav commented Jun 14, 2022

/ost

@erav
Copy link
Member Author

erav commented Jun 14, 2022

/ost

passed in https://redir.apps.ovirt.org/dj/job/ds-ost-baremetal_manual/40322
still waiting for pass on QE automation and new OST test [1]
[1] oVirt/ovirt-system-tests#175

@erav erav force-pushed the vnic-lock-on-hotplug branch 3 times, most recently from 852f32a to f447e8b Compare June 26, 2022 14:54
@erav erav force-pushed the vnic-lock-on-hotplug branch 2 times, most recently from 8d15a2c to 68a2763 Compare June 29, 2022 06:17
@erav erav force-pushed the vnic-lock-on-hotplug branch 2 times, most recently from 62bcfae to 0e377e2 Compare July 14, 2022 12:16
@erav
Copy link
Member Author

erav commented Jul 14, 2022

/ost network-suite-master

Lack of a lock in ActivateDeactivateVmNicCommand allows several
instances of hot plug and unplug requests to run in parallel causing
inconsistency in vNIC status as well as libvirt failing the requests due
to non-unique aliases. This patch adds a lock to the command so that
each instance of the command has to finish before the next one starts.

Bug-Url: https://bugzilla.redhat.com/2084530
Change-Id: I93ac9f1f12f34526ad10b9a49790c3a2689e8c4d
Signed-off-by: Eitan Raviv <eraviv@redhat.com>
Now that there is a lock on the command preventing VmDevicesMonitoring
from running while the command is working, there should be no race
between the command and VmDevicesMonitoring.

Change-Id: I7837dc642c7c55634bc6a7ad69a79049cc291726
Signed-off-by: Eitan Raviv <eraviv@redhat.com>
@ahadas
Copy link
Member

ahadas commented Jul 18, 2022

/ost

@erav
Copy link
Member Author

erav commented Jul 18, 2022

/ost network-suite-master

2 similar comments
@erav
Copy link
Member Author

erav commented Jul 18, 2022

/ost network-suite-master

@erav
Copy link
Member Author

erav commented Jul 20, 2022

/ost network-suite-master

@ahadas ahadas merged commit 9336b4f into oVirt:master Jul 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants