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

CA-391880: Update related field 'groups' of VM when destroying VM group. #5573

Conversation

BengangY
Copy link
Contributor

@BengangY BengangY commented Apr 19, 2024

In VM anti-affinity datamodel PR, there is many-many relation between VMs and VM groups. When changing the VM.groups manually, the VM_group.VMs will be updated automatically. But when removing a VM group, the VM.groups will not be updated automatically. This commit aims to update VM.groups for all VMs in a VM group when it is being removed.

@BengangY BengangY marked this pull request as ready for review April 19, 2024 14:34
@minglumlu
Copy link
Member

there is many-many relation between VM and VM group.
-> there is many-many relation between VMs and VM groups.

When removing a VM, or updating VM.groups, the VM group will update VM_group.VMs automatically.
-> When changing the VM.groups manually, the VM_group.VMs will be updated automatically.

But if removing a VM group, VM.groups will not update automatically.
-> But when removing a VM group, the VM.groups will not be updated automatically.

I didn't find a method to update it automatically. So I add some codes to update VM.groups when removing a group.
-> This commit aims to update VM.groups for all VMs in a VM group when it is being removed.

Copy link
Contributor

@gangj gangj left a comment

Choose a reason for hiding this comment

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

In VM anti-affinity https://github.com/xapi-project/xen-api/pull/5546, there is many-many relation between VMs and VM groups. When changing the VM.groups manually, the VM_group.VMs will be updated automatically. But when removing a VM group, the VM.groups will not be updated automatically. This commit aims to update VM.groups for all VMs in a VM group when it is being removed.
Would you please add this as the description of the code commit? It will make the code commit more clear.

I didn't find a method to update it automatically. So I add some codes to update VM.groups when removing a group.
I think this is the required thing to do, as we can find the same for the other relationship:
((_pci, "attached_VMs"), (_vm, "attached_PCIs"))
vm is removed from pci.attached_VMs: Db.PCI.remove_attached_VMs when needed, for example, when vm is destroyed.

In VM anti-affinity datamodel PR (xapi-project#5573),
there is many-many relation between VMs and VM groups.
 When changing the VM.groups manually, the VM_group.VMs will be updated automatically.
But when removing a VM group, the VM.groups will not be updated automatically.
This commit aims to update VM.groups for all VMs in a VM group when it is being removed.

Signed-off-by: Bengang Yuan <bengang.yuan@cloud.com>
@robhoes robhoes merged commit 81a05a0 into xapi-project:feature/vm-anti-affinity Apr 22, 2024
13 checks passed
Copy link

pytype_reporter extracted 50 problem reports from pytype output

.

You can check the results of the job here

@robhoes
Copy link
Member

robhoes commented Apr 22, 2024

The attached_PCIs is the only other example if a many/many relation in the datamodel that I can think of. And I recall problems with this then-new type of relation, which we didn't understand at the time. Ideally, the database should be updated automatically, and I think that setting one side of the relation does indeed correctly update the other side. But there may be a problem that the fields are not updated properly when objects are being deleted.

@BengangY BengangY deleted the private/bengangy/CA-391880 branch April 28, 2024 02:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants