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

Bug fixes #367

Merged
merged 7 commits into from
Feb 9, 2024
Merged

Bug fixes #367

merged 7 commits into from
Feb 9, 2024

Conversation

bchedotel
Copy link
Contributor

Few bug fixes:

  • add a default option for OVS configuration
  • add disk configuration in the vm template XML
  • add bridge configuration

@bchedotel bchedotel added bug Something isn't working Yocto labels Feb 7, 2024
eroussy
eroussy reviewed Feb 7, 2024
View reviewed changes
Copy link
Member

@eroussy eroussy left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I think the variable for bridge should be named create_br0_bridge.
Also, add the variable in examples/inventories/advanced_inventory_example.yaml and explain what it does.

eroussy
eroussy reviewed Feb 7, 2024
View reviewed changes
Copy link
Member

@eroussy eroussy left a comment

Choose a reason for hiding this comment

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

in the commit "templates/ovs_configuration.json.j2: fix default option for OVS configuration"
Remove the word "templates/" in the name. It will be shorter and more readable.

eroussy
eroussy reviewed Feb 7, 2024
View reviewed changes
Copy link
Member

@eroussy eroussy left a comment

Choose a reason for hiding this comment

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

You should not harcode the disk configuration.
You have to add a new variable in the ansible inventory and use it in the xml file with jinja templating.

dupremathieu
dupremathieu requested changes Feb 7, 2024
View reviewed changes
Copy link
Member

@dupremathieu dupremathieu left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The last commit title (templates/ovs_configuration.json.j2: fix default option for OVS configuration) is too long.

templates/vm/guest.xml.j2 Outdated Show resolved Hide resolved
templates/vm/guest.xml.j2 Outdated Show resolved Hide resolved
eroussy
eroussy requested changes Feb 8, 2024
View reviewed changes
templates/vm/guest.xml.j2 Outdated Show resolved Hide resolved
eroussy
eroussy requested changes Feb 8, 2024
View reviewed changes
vars/network_vars.yml Outdated Show resolved Hide resolved
dupremathieu
dupremathieu requested changes Feb 8, 2024
View reviewed changes
templates/vm/guest.xml.j2 Outdated Show resolved Hide resolved
@bchedotel bchedotel force-pushed the bug_fix branch 2 times, most recently from 090791e to 240428d Compare February 8, 2024 10:21
@dupremathieu dupremathieu self-requested a review February 8, 2024 10:21
dupremathieu
dupremathieu previously approved these changes Feb 8, 2024
View reviewed changes
@bchedotel bchedotel requested a review from eroussy February 8, 2024 10:24
@bchedotel
ovs_configuration.json.j2: fix default option for OVS configuration
f3af2db 
Refactor OVS configuration setup by providing default values for 'ovs_bridges' and 'ignored_bridges'.

Signed-off-by: Benjamin Chedotel <[email protected]>
@bchedotel
guest.xml.j2: add disk configuration
2871f6c 
Update 'guest.xml.j2' template to include disk configuration for the virtual machine.

Signed-off-by: Benjamin Chedotel <[email protected]>
@bchedotel
networks_vars.yml: add bridge configuration
3af18cf 
Update 'network_vars.yml' to incorporate bridge configuration options.

Signed-off-by: Benjamin Chedotel <[email protected]>
@bchedotel
deploy_vms_standalone.yaml: don't hardcode xml location
4da75b0 
The VM's XML configuration file for debugging was hardcoded, so it
is now possible to define its location properly in the VM's inventory.

Signed-off-by: Benjamin Chedotel <[email protected]>
@bchedotel
advanced_inventory_cluster_example.yaml: add create_br0_bridge
235cc57 
Added a variable to create a Linux bridge. This not only facilitates
communication between the hypervisor and the vms, but also allows
everything to be configured with a single interface.

Signed-off-by: Benjamin Chedotel <[email protected]>
eroussy
eroussy previously approved these changes Feb 9, 2024
View reviewed changes
@bchedotel
vms_inventory_example.yaml: add local_disk variable
e1b6047 
Added local_disk variable to indicate the VM disk list,
when you're on the standalone version and it's not managed by
VM manager.

Signed-off-by: Benjamin Chedotel <[email protected]>
@bchedotel
advanced_inventory_standalone_example.yaml: add create_br0_bridge
de5a6e6 
Added a variable to create a Linux bridge. This not only facilitates
communication between the hypervisor and the vms, but also allows
everything to be configured with a single interface.

Signed-off-by: Benjamin Chedotel <[email protected]>
@bchedotel bchedotel merged commit 31d33c4 into main Feb 9, 2024
3 checks passed
@bchedotel bchedotel deleted the bug_fix branch February 9, 2024 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eroussy eroussy eroussy approved these changes

@dupremathieu dupremathieu Awaiting requested review from dupremathieu

Assignees
No one assigned
Labels
bug Something isn't working Yocto
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bchedotel @dupremathieu @eroussy