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.

Sign up for GitHub

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

Jump to bottom

core: VDS Maintenance will wait for all disk transfers to happen and … #632

Merged
merged 1 commit into from
Sep 22, 2022
Merged

core: VDS Maintenance will wait for all disk transfers to happen and … #632

merged 1 commit into from
Sep 22, 2022

Conversation

ArtiomDivak
Copy link
Member

@ArtiomDivak ArtiomDivak commented Sep 4, 2022 •
edited
Loading

core: VDS Maintenance will wait for all disk transfers to happen and block new ones

if a user wants to set a host to maintenance and at least one of
the disk is in transfer the command will fail and will send an error.
now it will not send an error it will wait for the disk transfer to finish
(the host will be in preparingForMaintenance mode) and new transfers will be blocked from starting.

Bug-Url: https://bugzilla.redhat.com/1990231
Signed-off-by: Artiom Divak adivak@redhat.com

@ahadas ahadas added the storage label Sep 7, 2022
@ArtiomDivak ArtiomDivak force-pushed the bug-1990231 branch 2 times, most recently from 4b425b4 to 8f49ff0 Compare September 11, 2022 13:40
mkemel
mkemel reviewed Sep 12, 2022
View reviewed changes
Copy link
Member

@mkemel mkemel left a comment

Choose a reason for hiding this comment

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

Currently, looking at the PR it's not quite clear from your explanation what you are aiming to do here. The PR message is not up-to-date, as well as the commit message, and it needs formatting.

...src/test/java/org/ovirt/engine/core/bll/storage/disk/image/TransferDiskImageCommandTest.java Outdated Show resolved Hide resolved
...src/test/java/org/ovirt/engine/core/bll/storage/disk/image/TransferDiskImageCommandTest.java Outdated Show resolved Hide resolved
@barpavel barpavel self-requested a review September 14, 2022 10:56
mkemel
mkemel reviewed Sep 15, 2022
View reviewed changes
Copy link
Member

@mkemel mkemel left a comment

Choose a reason for hiding this comment

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

I think you still need to tidy-up the commit message.
At least to format it properly.

@ArtiomDivak ArtiomDivak force-pushed the bug-1990231 branch 2 times, most recently from 26e1687 to 5946a43 Compare September 20, 2022 06:38
@ArtiomDivak ArtiomDivak force-pushed the bug-1990231 branch 2 times, most recently from f59e1e7 to de77745 Compare September 21, 2022 07:20
@ArtiomDivak @ahadas
core: VDS Maintenance will wait for all disk transfers to happen and …
f718d07 
…block new ones

if a user want to set a host to maintenance and at least one of
the disk is in transfer the command will fail and will send an error.
now it will not send an error it will wait for the disk transfer to finnish
(the host will be in preparingForMaintenance mode) and new transfers will be blocked from starting.

Bug-Url: https://bugzilla.redhat.com/2120228
Signed-off-by: Artiom Divak <adivak@redhat.com>
@ahadas ahadas self-requested a review September 21, 2022 20:29
@ahadas
Copy link
Member

ahadas commented Sep 21, 2022

/ost

barpavel
barpavel approved these changes Sep 22, 2022
View reviewed changes
Copy link
Member

@barpavel barpavel left a comment

Choose a reason for hiding this comment

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

LGTM, the testVirtCanMoveToMaintenance() test can be fixed.
The new part already fixed by Arik, the existing also has an issue.

...ker/src/test/java/org/ovirt/engine/core/vdsbroker/monitoring/VirtMonitoringStrategyTest.java Show resolved Hide resolved
...ker/src/test/java/org/ovirt/engine/core/vdsbroker/monitoring/VirtMonitoringStrategyTest.java Show resolved Hide resolved
@ahadas ahadas merged commit 2de7297 into oVirt:master Sep 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahadas ahadas ahadas approved these changes

@mkemel mkemel mkemel left review comments

@barpavel barpavel barpavel approved these changes

@sgratch sgratch Awaiting requested review from sgratch sgratch is a code owner

@ljelinkova ljelinkova Awaiting requested review from ljelinkova

@bennyz bennyz Awaiting requested review from bennyz bennyz is a code owner

@emesika emesika Awaiting requested review from emesika emesika is a code owner

@mwperina mwperina Awaiting requested review from mwperina mwperina is a code owner

@michalskrivanek michalskrivanek Awaiting requested review from michalskrivanek michalskrivanek is a code owner

@oliel oliel Awaiting requested review from oliel

@smelamud smelamud Awaiting requested review from smelamud smelamud is a code owner

@didib didib Awaiting requested review from didib didib is a code owner

Assignees
No one assigned
Labels
storage
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ArtiomDivak @ahadas @barpavel @mkemel