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

virt-install --connect lxc:/ unnecessarily requires --memory #73

Closed
emojifreak opened this issue Jan 4, 2020 · 5 comments
Closed

virt-install --connect lxc:/ unnecessarily requires --memory #73

emojifreak opened this issue Jan 4, 2020 · 5 comments

Comments

@emojifreak
Copy link
Contributor

I can run virt-install --memory 2048 --connect lxc:/ --os-variant fedora31 --filesystem /var/lib/lxc/fedora31test/rootfs,/ --network none --transient --import --name fedora31test without any error on Fedora Rawhide, but if I remove --memory 2048 I get an error ERROR --memory amount in MiB is required and virt-install fails. As far as I understand LXC containers do not need memory amount.

Package versions:

# dnf list --installed
virt-install.noarch 2.2.1-2.fc31
libvirt-daemon-driver-lxc.x86_64 5.10.0-2.fc32 
libvirt-daemon-lxc.x86_64        5.10.0-2.fc32 
lxc.x86_64                       3.2.1-1.fc32   
lxc-libs.x86_64                  3.2.1-1.fc32  
lxc-templates.x86_64             3.2.1-1.fc32 
@crobinso
Copy link
Member

Indeed, the libvirt lxc:/// driver doesn't appear to do anything with the memory setting. The libvirt XML parser though will reject XML without in it. I pushed a commit to make virt-install just silently default to '--memory 64' if no memory setting was specified for lxc:/// . Thanks for the report!

@phrdina
Copy link
Member

phrdina commented Jan 15, 2020

That's not true, we need to revert that patch. Memory is used by cgroups so if we default to 64 you would not be able to do a lot of things inside the container by default.

@crobinso
Copy link
Member

@Antique hmm I checked src/lxc libvirt code but couldn't find anything that looked like actual usage of cur_balloon. Did I miss it?

@phrdina
Copy link
Member

phrdina commented Jan 15, 2020

Well, --memory translates to element which in code is stored as total_memory. Now in src/lxc/lxc_cgroup.c in function virLXCCgroupSetupMemTune we call virDomainDefGetMemoryInitial which gets the value from total_memory excluding empty memory modules and uses that to set cgroup memory max which limits how much of memory can container use.

crobinso added a commit that referenced this issue Jan 15, 2020
My previous patch was misguided as pointed out by Pavel:
#73 (comment)

And it was setting incorrect memory, which I missed because the tests
are busted here. Add a hack to work around that

Bump up the default to 1024, and print it, so the user can tell if
the default is not to their liking

Signed-off-by: Cole Robinson <crobinso@redhat.com>
@crobinso
Copy link
Member

My bad, thanks for catching that. I change things to default to --memory 1024, and print it so the user gets notified, like we do when default to memory based on --os-variant for qemu

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

No branches or pull requests

3 participants