BiteSizedTasks
This page tracks introductory tasks for new libvirt contributors. Code cleanups, bugs, and small features are listed with difficulty ranging from trivial to intermediate.
Contents
Beginner tasks
These tasks are good first introductions to libvirt code. You will be required to get things building and make sure the test suite passes, but not necessarily need to understand the details of what libvirt is doing. Mostly these are following existing example changes that are already in the code. Contact the listed mentor with any questions
Move validation checks out of domain XML parsing
Mentor: Cole Robinson <crobinso@redhat.com>
The code that handles domain/VM XML parsing (virDomainDefParseXML
in src/domain/domain_conf.c
) has various validation checks sprinkled within it. Many of these checks should be moved out of the parse step and into virDomainDefPostParse
, so they can be shared by various places in the code that build a domain definition without XML, such as converting from native vmware/virtualbox/xen formats.
Not all validation checks can or should be moved. You'll need to determine if the check is specific to the actual XML parsing, or is it general for all VM configurations. So anything that throws an error if it hits unexpected XML should not be moved.
Just a few of the candidates for moving to virDomainDefPostParse
:
- All the checks against dom->os.type and dom->virtType in
virDomainInputDefParseXML
should go into avirDomainInputDefValidate
function - The error "tray is only valid for cdrom and floppy" in
virDomainDiskDefParseXML
- 'disk vendor' and 'disk product' errors in
virDomainDiskDefParseXML
host->socket
checks invirDomainStorageNetworkParseHost
- all the option collision 'total and...' error messages in
virDomainDiskDefIotuneParse
- 'pci-root and pcie-root controllers should not' in
virDomainControllerDefParseXML
- PCI
chassisNr
,chassis
,port
,busNr
,numaNode
range checks invirDomainControllerDefParseXML
- "Controllers must use the 'ccid' address type" in
virDomainSmartcardDefParseXML
- any 'not available' or 'valid only' errors in
virDomainGraphicsListenDefParseXML
- All the 'only supported' errors in
virDomainVideoDefParseXML
- size validation in
virDomainMemoryTargetDefParseXML
- 'must be in range' errors in
virDomainDefParseBootXML
- 'You must map the root user of container' in
virDomainIdmapDefParseXML
- 'duplicate blkio device path' error
- 'Only one primary video device is supported' error
You can see an example commit here: https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=265e6924f569431d431981c4c0d8d68baf0fb2a2
More usage of virStringParseYesNo
Mentor: Cole Robinson <crobinso@redhat.com>
There's a common pattern in our XML parsing to convert string 'yes' to true and 'no' to false, and error if we receive anything other than those values. To find instances, try git grep "(STREQ(.*, \"yes\")" and git grep "(STREQ(.*, \"no\")". A function virStringParseYesNo
was added to do this conversion, and several conversions were already done:
https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=612c0d4bb80bf40f82d64057838dd610b78ad306
For most of the remaining usages in domain_conf.c
, only "yes" is explicitly checked for. This means all other values are implictly handled as 'false'. In this case, use virStringParseYesNo
, but if it returns -1, don't raise an error but set the bool value to false.
Clean up variables in tools/libvirt-guests.sh.in
Mentor: Martin Kletzander <mkletzan@redhat.com>
The script has many functions that just redeclare variables all over the place instead of making them local to the function using the local
keyword. There might be some errors due to the rewriting of the variables and it's also prone to error, so all such variables should be marked local
. There is at least one function that returns an information in a variable. Such occurrences might be fixed up or kept that way.
Slightly-less-beginner tasks
These tasks are slightly more involved than the beginner tasks, but still don't require extensive libvirt knowledge. If any of the beginner tasks seem too simple, maybe try one of these. Talk with the listed mentor if you have questions.
Move virDomainVideoDefaultType
logic to individual drivers
Mentor: Cole Robinson <crobinso@redhat.com>
The virDomainVideoDefaultType
function in src/conf/domain_conf.c
shouldn't be there :) The logic setting a device default should be in the post parse function of individual driver code.
See this example commit that moves logic out of that function and into the src/qemu: https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=ef08a545388f388b7c76b99a3f3d2584daf05145
Similar changes need to be made for the following drivers:
- src/bhyve: move the BHYVE logic out of
virDomainVideoDefaultType
and intobhyveDomainDeviceDefPostParse
- src/libxl: move the XEN logic out of
virDomainVideoDefaultType
and intolibxlDomainDeviceDefPostParse
- src/vz: move the VZ and PARALLELS logic out of
virDomainVideoDefaultType
and intovzDomainDeviceDefPostParse
- src/vmx and src/vmware: move the VMWARE logic out of
virDomainVideoDefaultType
and into
virVMXDomainDevicesDefPostParse
andvmwareDomainDeviceDefPostParse
The following drivers need the postparse plumbing added entirely. Your mentor could help with that:
src/test
src/vbox
Move default Input bus logic to PostParse handling
Mentor: Cole Robinson <crobinso@redhat.com>
Similar to what was started with default video model logic in these commits:
- https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=a2ca7ca52eb624883ac5a7e1d5deebb43981a410
- https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=29a90f071dd7c1764f82c7fcbfdded35d252b174
- https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=ef08a545388f388b7c76b99a3f3d2584daf05145
Break out the default bus setting logic from virDomainInputDefParseXML
and move it to PostParse handling. Look for anything that sets a def->bus value in that function, it's grounds for removal
Add additional virsh command line completion functions
Mentor: Michal Privoznik <mprivozn@redhat.com>
Completers are small functions that are called from the virsh command line tool whenever the user started typing something and hit TAB TAB. Their purpose is to offer list of strings that suit entered input. If there is only one item on the list then it's entered in automatically. For instance:
virsh # dom<TAB><TAB> domblkerror domblkstat domcontrol domfsinfo .. virsh # start --domain <TAB><TAB> fedora fedora.i686 freebsd gentoo ...
Additional functions are needed to improve other commands.
An example patch: https://www.redhat.com/archives/libvir-list/2018-June/msg01672.html A series of example patches: https://www.redhat.com/archives/libvir-list/2018-January/msg00436.html
Mentor: Cole Robinson <crobinso@redhat.com>
Libvirt ships several daemons (libvirtd
, virtlogd
, virtlockd
) that all have similar code around general daemon startup. Right now the code is duplicated for each daemon, but it could be shared, like in a new file src/util/virdaemon.c
. The main files to look in are src/logging/log_daemon.c
, src/locking/lock_daemon.c
, and daemon/libvirtd.c
. Some example functions to look at:
- the *ForkIntoBackground functions
- the *SetupLogging functions
- the *SetupSignals functions
- the *DaemonClientNew functions
- the *UnixSocketPaths functions
Bugs and feature requests from bugzilla (LibvirtFirstBug)
Some bugs in the libvirt bugzilla tracker are marked as good introductory bug reports or features for new contributors. These bugs have the 'LibvirtFirstBug' tag. Each bug report contains info and sample commits to point you in the right direction. You can open the list in your browser bugzilla here.
Failed to load RSS feed from https://bugzilla.redhat.com/buglist.cgi?bug_status=NEW&bug_status=ASSIGNED&bug_status=POST&bug_status=MODIFIED&classification=Community&f1=status_whiteboard&f2=component&list_id=4999130&o1=substring&o2=anywordssubstr&order=bug_id&product=Virtualization%20Tools&query_based_on=&query_format=advanced&v1=LibvirtFirstBug&v2=libvirt&ctype=atom: Error parsing XML for RSS
Ongoing code transitions
GLib replacements
In 2019 libvirt began using GLib internally. We can use GLib APIs to drop some libvirt-specific code.
GKeyFile
Use the g_key_file_* APIs to replace libvirt's virkeyfile.c
code
Path functions
Functions in virfile.c
related to path handling can be replaced with g_path_* APIs. The callers should be made to invoke the GLib APIS directly.
XAttr / ACLs
Replace virfile.c
XAttr and ACL handling with GFileAttribute APIs
Threads
The callers of the various virMutex
, virThread
, virCond
, virOnce
, etc APIs should be switched to use the GLib threading APIs.
g_autofree and g_autoptr usage
g_autofree
is a feature provided by GLib and the C compiler which will free a variable when it goes out of scope.
g_autoptr
works similarly, but it will call a custom registred free
function instead.
We are slowly converting the code to use these new patterns. Any variable that is allocated and free'd within a single function can likely be converted to use g_autofree
or g_autoptr
. Similarly, any usage of VIR_FREE
under a cleanup:
can likely be converted.