Google Summer of Code 2016/Abstracting device address allocation

From Libvirt Wiki
Jump to: navigation, search

During Google Summer of Code, I was working on device address allocation in libvirt.

The biggest challenge in this project was the fact that it involved understanding and modifying a vast amount of already existing code. Because that code was constantly being edited by other libvirt contributors, there were a lot of rebase conflicts that I had to solve while working on my patches. Every new iteration of a patch, apart from regular changes, had also to be carefully examined if nothing broke during the rebase, and fixed if something wrong happened.

Data structure design

There are a lot of ways that addresses might be assigned in QEMU. Especially PCI addresses come to mind, because they are the most complex ones, but they are not the only ones: there are also ccw and virtio address sets in libvirt. More types of addresses were planned to be introduced, which was later proven to be true when an usb address set was introduced during this summer. Another nice thing to have would be a possibility of handling other types of addresses too, even though they aren't already kept as an address set in libvirt, like device names (sda, sdb, ...), in order to verify that they are correct and protect against duplicates. The main point is, there are a lot of address types that behave differently and there are more to come. What is more, when next hypervisors add support of explicitly stating device addresses, the number of functions aimed at just handling the address types would skyrocket, because the current ones worked with QEMU only.

I designed an abstract data structure that was capable of handling all the existing address types. Can't say for sure about the future ones, but I suppose that it would be enough.
The main idea is that the addresses that are usable would be kept as a set of pools, each pool being some range of addresses. For example, if someone decided to add a PCI bus, a new pool would be added to the allocator.
When allocating addresses, the interface allowed to pick any address, any address from some range, or some specific address. To handle reserving a whole slot of a PCI bus, a function that reserved a range of addresses was also to be provided.
The internal representation was a thing to be discussed, but I had two candidates: either an array of bits, or a sorted list of ranges in the form [from, to].

After a long discussion, it was decided that a structure to handle all the possible cases would be too complicated, especially because not all of the addresses are (at least initially) as complex as the PCI ones. Now, in hindsight, I am sure that implementing that structure would be a lot of fun, but there were better approaches to the problem.

Here's a link to the design and discussion about it on the mailing list, where you can find a detailed explanation on how it would work:


[PATCH] qemu_hotplug: Use a helper variable consistently

A small change to check how sending patches to the mailing list works.

Link to the patch series:

[PATCH 0/2] Abstracting device address allocation

I migrated the virtio serial address handling functions to a hypervisor-agnostic place and moved the virtio address set cache to domain definition instead of being cached in qemu private data.

Link to the patch series:

[PATCH 0/4] Move ccwaddrs and pciaddrs to domainDef

Instead of caching ccw and pci address sets in QEMU's private data, cache it in the domain's definition. Then move ccw and pci-handling functions to domain_addr.

By moving the address handling functions to a qemu-agnostic place, there is a greater chance that when a new hypervisors introduces explicitly stating device addresses, functions get reused instead of reimplemented. To achieve this goal, the cache of address sets had to be moved from qemu's private data to the domain definition.

Link to the patch series:

Then it was decided that after all, caching the address sets isn't really needed for anything anymore and won't be required in any planned features, so it can be removed instead.

[PATCH 0/3] Clean up qemuhotplugtest xml files

Modifying existing code meant that something might suddenly stop working, so I first modified the place where test xml files are kept.

Link to the patch series:

[PATCH 0/2] Add qemu hotplug tests with ccw devices

Hotplugging ccw devices was previously not tested, so I decided to add some rather complex testcases for them.

Link to the patch series:

[PATCH 00/10] Test persistent device attachment

I separated some code out of qemuDomainAttachDeviceLive and qemuDomainDetachDeviceLive, then modified qemuhogplugtest, and then added the first test for attaching a device to the persistent domain (up to this point, only attaching to the live domain was tested).

This patch made it easier to test attaching/detaching more types of devices, because a more general function was now being used.

Link to the patch series:

[PATCH v2] qemuhotplugtest: Add tests for ccw devices

A second version of the ccw tests, with some very slight changes.

Link to the patch series:

[PATCH v3] qemuhotplugtest: Add tests for ccw devices

I fixed the ccw tests.

My mentor noticed that in my test xmls, there are a few things that aren't supposed to work. These tests were successfully passing and the errors in xmls weren't discovered by libvirt. It proved that improving the testing suite for hotplugging/coldplugging and writing new tests is really worth it.
(this is v3, but is accidentally tagged it as v2 on the mailing list).

Link to the patch series:

[PATCH v2 00/10] Test persistent device attachment

I was asked to rebase and fix the conflicts in my patch series that introduced testing persistent device attachment.

Link to the patch series:

[PATCH 00/10] Remove caching of address sets

I deleted the caching of pci, virtioserial and ccw address sets.

The credit for the first three commits from this patch series goes to Cole Robinson.

Link to the patch series:

[PATCH 00/13] Move functions from qemu_domain_address.c to domain_addr.c

I moved the functions from qemu_domain_address to domain_addr. That required making some functions qemu-agnostic.

This is the second try of modifying address handling functions, so that they can be used for other hypervisors too.

Link to the patch series:

[PATCH] Remove code duplication in domain_addr.c

A small patch to remove a blatant code duplication.

Link to the patch series:

[PATCH v2 00/10] Remove caching of address sets

A new address set (usb) was added before my changes that removed the caching of address sets was accepted, so it had to be updated. Also, fixed two small bugs from the first version of this patch.

Link to the patch series:

[PATCH v3 0/6] Test persistent device attachment

About a half of v2 of my "testing persistent device attachment" series was accepted. In this patch, I implemented my mentor's suggestions to improve my changes. This time, I also added a split of qemuDomainUpdateDeviceFlags, which allowed to finally run a testcase that has been commented out for three years.

Link to the patch series:

[PATCH v3 0/5] Remove caching of address sets

My mentor made some suggestions on how to handle pci address set differently than in my previous patch. I implemented it and also expanded upon that.

Link to the patch series:

[PATCH v4 0/5] Remove caching of address sets

A new version of a patch series that removes pci address set caching. This time, a function displays a warning when doing something on the line instead of failing.

Link to the patch series:

[PATCH RFC 0/5] Remove usb address set caching

Since while I was working on removing the caching of pci, virtio and pci address sets, a new address set was introduced, I removed its caching. Here, I took the approach (using the fact that usb address sets are a new thing without many caveats) of using the same function to both calculate and recalculate the address set using the same function. I also suggested unifying assigning usb addresses in a new domain and hotplugging a usb address.

Link to the patch series:

[Bug] make check fails when LIBVIRT_DEBUG is defined

I also discovered a malfunction with debugging prints when executing tests, so I decided to write about it on the mailing list:

Summary of patches

Commits that are already merged: (Credit goes to Cole Robinson, I just rebased it) (Credit goes to Cole Robinson, I just rebased it) (Credit goes to Cole Robinson, I just rebased it)

The latest versions of commits that are still applicable, but haven't been reviewed yet: