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

Change conflict message to be more informative #383

Merged
merged 2 commits into from
May 23, 2022
Merged

Change conflict message to be more informative #383

merged 2 commits into from
May 23, 2022

Conversation

ArtiomDivak
Copy link
Member

@ArtiomDivak ArtiomDivak commented May 19, 2022 •
edited by ahadas

when trying to remove VM while its disks are being transferred(uploaded or downloaded) you get this error message:
'Cannot remove VM: VM is locked. Please try again in a few minutes.'
Now it will show more informative message: 'Cannot remove VM. Disk vm_Disk is being transferred.'

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

@ArtiomDivak ArtiomDivak force-pushed the bug-2066078 branch 3 times, most recently from 4af5bb5 to 3c9953b Compare May 19, 2022 11:45
@ArtiomDivak ArtiomDivak requested a review from bennyz May 19, 2022 11:45
@ArtiomDivak
Copy link
Member Author

Hi,
I successfully reproduced the error by this commands:
"python3 /usr/share/doc/python3-ovirt-engine-sdk4/examples/download_disk_snapshot.py NFS_SD1 3694be67-587d-40f2-9546-d468b1079ab1 /home/adivak/Desktop -c engine
python3 /usr/share/doc/python3-ovirt-engine-sdk4/examples/download_disk_snapshot.py NFS_SD1 0725d399-9315-4bd7-bcef-82c1a23595ce /home/adivak/Desktop -c engine
python3 /usr/share/doc/python3-ovirt-engine-sdk4/examples/download_disk_snapshot.py NFS_SD1 5a9bc2ac-34f4-4a30-a3bc-f8aaa3bde71c /home/adivak/Desktop -c engine
python3 /usr/share/doc/python3-ovirt-engine-sdk4/examples/download_disk_snapshot.py NFS_SD1 aa9a6224-f6aa-4353-9649-f555408200eb /home/adivak/Desktop -c engine"
and sent by postman delete VM command

@barpavel
Copy link
Member

Sorry, maybe I'm missing something, but isn't this PR improves the message that will be displayed when initiating an image transfer while another operation is WIP (with exclusive lock on VM ID, like VM deletion), but not the opposite direction? Shouldn't the change be somewhere in RemoveVmCommand or something?

ahadas
ahadas reviewed May 19, 2022
View reviewed changes
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.

minor comment, other than that - lgtm

.../manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/EngineMessage.java Outdated Show resolved Hide resolved
@ahadas
Copy link
Member

ahadas commented May 19, 2022 •
edited

Sorry, maybe I'm missing something, but isn't this PR improves the message that will be displayed when initiating an image transfer while another operation is WIP (with exclusive lock on VM ID, like VM deletion), but not the opposite direction? Shouldn't the change be somewhere in RemoveVmCommand or something?

the original bug is about a bit different issue - that one is unable to remove the vm and gets a message that the vm is locked while if you check the vm status it's not locked (because we're talking about an in-memory lock). so here we attach a better message when locking the vm as part of image transfer, so once you try to remove the vm you'll get a better message

ahadas
ahadas reviewed May 23, 2022
View reviewed changes
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.

this change looks great but while on it, let's also address an issue we've noticed recently

...bll/src/main/java/org/ovirt/engine/core/bll/storage/disk/image/TransferDiskImageCommand.java Outdated Show resolved Hide resolved
@ahadas ahadas self-requested a review May 23, 2022 09:40
@ahadas ahadas added the storage label May 23, 2022
@ArtiomDivak ArtiomDivak force-pushed the bug-2066078 branch 3 times, most recently from 1a44f58 to 5103216 Compare May 23, 2022 12:21
ArtiomDivak and others added 2 commits May 23, 2022 18:28
@ArtiomDivak @ahadas
core:change conflict message to be more informative
40e1db1 
when trying to remove VM while its disks are being transferred(uploaded or downloaded) you get this error message:
'Cannot remove VM: VM is locked. Please try again in a few minutes.'
Now it will show more informative message: 'Cannot remove VM. Disk vm_Disk is being downloaded or uploaded.'

Bug-Url: https://bugzilla.redhat.com/2066078
Signed-off-by: Artiom Divak adivak@redhat.com
@ahadas
core: handling of non-existence disk image in TransferDiskImage#getSh…
155a374 
…aredLocks

Signed-off-by: Arik Hadas <ahadas@redhat.com>
@ahadas ahadas changed the title core:change conflict message to be more informative Change conflict message to be more informative May 23, 2022
@ahadas
Copy link
Member

ahadas commented May 23, 2022

/ost

@ahadas ahadas merged commit 9979165 into oVirt:master May 23, 2022
@barpavel barpavel mentioned this pull request May 24, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@barpavel barpavel barpavel left review comments

@ahadas ahadas ahadas approved these changes

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

@ljelinkova ljelinkova Awaiting requested review from ljelinkova

@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

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

@aesteve-rh aesteve-rh Awaiting requested review from aesteve-rh

@mkemel mkemel Awaiting requested review from mkemel

@sleviim sleviim Awaiting requested review from sleviim

@bennyz bennyz Awaiting requested review from bennyz bennyz 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 @barpavel @ahadas @bennyz