Hey folks, We have started work on issue 11 <https://gitlab.com/libvirt/libvirt/-/issues/11>, and we have some questions to ensure we tackle the issue properly. - What are the different use cases for g_autoptr vs g_autofree? We found that g_autofree is preferred for anything that uses g_malloc according to the Glib documentation <https://developer.gnome.org/glib/stable/glib-Miscellaneous-Macros.html#g-autofree>, and g_autoptr is for types with custom destructors. However, when using g_autoptr, we got compile errors when trying to pass the g_autoptr as an argument (the argument seems to be converted to an integer). When should we use each of these, and when should we not convert them at all? - We see that some work has been done to convert files to use the Glib API. In some cases, files contain code that uses both the old memory management API and the Glib API. Should we focus our attention on files where these conversions are not yet underway? Or should we expect that many of the files are only partially converted? - In some cases, we found that converting to the Glib API might require cascading changes to code structure (function parameter types, function return types). Is it advisable to pursue these cases as well, or should we limit changes to pointers which are declared and then freed within one method? - Do you know of a directory or set of files where the conversions to the Glib API are needed? We appreciate the correspondence, and we hope to use this information to make a good contribution to the project! Best regards, Dustan Helm <dustan.helm@yahoo.com> Barrett Schonefeld <bschoney@utexas.edu> Ryan Gahagan <ryangahagan18@gmail.com> On Wed, Sep 16, 2020 at 4:35 AM Michal Privoznik <mprivozn@redhat.com> wrote:

On 9/15/20 10:39 PM, Barrett J Schonefeld wrote:

...

Hey libvirt team,

We (Ryan Gahagan, Dustan Helm, and Barrett Schonefeld) are computer science students at the University of Texas at Austin. We are taking a course in virtualization, and we’d like to contribute to the libvirt repository as part of this course. Here are the issues we are most interested in:

https://gitlab.com/libvirt/libvirt/-/issues/11 <https://gitlab.com/libvirt/libvirt/-/issues/11>

https://gitlab.com/libvirt/libvirt/-/issues/16

Additionally, we would like to take a look at issue 4 (https://gitlab.com/libvirt/libvirt/-/issues/4 <https://gitlab.com/libvirt/libvirt/-/issues/4>), the UDP slowdown for QEMU. We expect issue 4 to be more time-intensive, and we would like to communicate with you to ensure we’re solving the problem effectively.

Our course only runs until the end of the fall semester, so our time to contribute to this project is somewhat limited. If you think any of the issues we picked would be too difficult to accomplish during that time frame, we would appreciate alternative suggestions. We really hope to contribute to this project and help make improvements where we can.

Hey,

it's always nice to see people interested in libvirt.

Another area that I can offer (not listed on the issues page) is writing virsh completers. These are callback functions which are run when a user hits <TAB><TAB>, for instance:

virsh start --domain<TAB><TAB>

brings up a list of shut off guests. It's really regular autocompletion like we're used to from bash and/or other projects.

I'd say writing a completer is more beneficial if one wants to learn how to use libvirt public APIs because that's basically what a completer callback does. I've reviewed some completer patches recently:

https://www.redhat.com/archives/libvir-list/2020-September/msg00592.html

I'm happy to help,

Michal

On 11/3/20 12:47 AM, Barrett J Schonefeld wrote:

Hey folks,

We have started work on issue 11 <https://gitlab.com/libvirt/libvirt/-/issues/11>, and we have some questions to ensure we tackle the issue properly.

- What are the different use cases for g_autoptr vs g_autofree? We found that g_autofree is preferred for anything that uses g_malloc according to the Glib documentation <https://developer.gnome.org/glib/stable/glib-Miscellaneous-Macros.html#g-autofree>, and g_autoptr is for types with custom destructors. However, when using g_autoptr, we got compile errors when trying to pass the g_autoptr as an argument (the argument seems to be converted to an integer). When should we use each of these, and when should we not convert them at all?

Right. g_autofree instructs compiler to call plain free() over the variable when it goes out of scope (mind you, not only at the end of a function, but also inside a loop, if-else bodies, etc.) while g_autoptr() does two things: it declares variable as a pointer to given type, and calls previously registered destructor when the variable goes out of scope (instead of plain free()). For instance: g_autofree char *tmp = g_strdup("my awesome string"); g_autoptr(virBitmap) b = virBitmapNew(..); And since virBitmap module lives under src/util/virbitmap.* that's also where you can find the destructor registration: G_DEFINE_AUTOPTR_CLEANUP_FUNC(virBitmap, virBitmapFree); We are currently in transition state of adopting glib so you can find a mixture of our old approach (VIR_FREE() and/or explicit destructor calls like virBitmapFree()) and the new approach (where glib is used). Not ideal, but for any new code we try to use glib if possible. Sometimes we just rewrite a module/file into glib, for instance: https://www.redhat.com/archives/libvir-list/2020-October/msg00210.html Hence, any rewrite to glib is welcome!

- We see that some work has been done to convert files to use the Glib API. In some cases, files contain code that uses both the old memory management API and the Glib API. Should we focus our attention on files where these conversions are not yet underway? Or should we expect that many of the files are only partially converted?

You are free to chose whatever you want. I'd start with small files (e.g. src/util/*) because they are usually self contained and have small functions in them (i.e. no complicated branching is happening inside a function).

- In some cases, we found that converting to the Glib API might require cascading changes to code structure (function parameter types, function return types). Is it advisable to pursue these cases as well, or should we limit changes to pointers which are declared and then freed within one method?

This is a tougher question. Libvirt has a promise of stable API which includes public C API and ABI. Therefore an user facing function can't change its parameters or their order. These are declared in include/libvirt/*.h and implemented in src/libvirt-. Note, the body of these functions can be changed, it's signature that can never change. Having said that, our internal APIs are free to change as we please.

- Do you know of a directory or set of files where the conversions to the Glib API are needed?

IMO, it's best to start with something smaller (like src/util/, tools/) send that for review to make sure you're on the right path and then continue with something bigger. Michal

On 11/5/20 10:04 PM, Barrett J Schonefeld wrote:

Hey folks,

We appreciate the feedback, and we've used this to complete some initial work on issue 11.

We started with small changes in src/util, and we submitted them via email (following these guidelines on submitting patches <https://libvirt.org/submitting-patches.html>) to ensure we are on the right path.

We do not see the patch we submitted via email showing up in the threads, and we are wondering if we are submitting incorrectly or if the patch goes through some approval process before appearing in the threads.

Best regards, Barrett Schonefeld

So I checked the moderator's queue and there are no patches stuck, so probably it is something else. Did you see any error when 'git send-email'-ing the patches? When I was setting up my machine I used to send patches just to me to verify send-email is working and only then I tried to send patches to the list. Also, are you sure you (the sender) are subscribed to the list? Generally, if you aren't then list won't forward your e-mails/patches. Michal

We've also been having some troubles actually getting ninja and meson to run properly. Our team has one member on MacOS, one on Ubuntu 18.04, and one working on a remote server (Ubuntu again) without sudo privileges. We want to be able to run ninja test to properly test and clean our code as we make pull requests, but it's been very difficult to get set up. The Ubuntu aptitude store has an outdated version of meson that doesn't actually run properly, and the pip3 version is up to date but then the build dependencies are left unresolved. These dependencies are also by default not actually in the aptitude store either, nor can they easily be mass installed via homebrew (to our knowledge). Even after manually configuring aptitude to find the links to all the dependencies of the project, manually installing meson and ninja, and installing the dependencies, we are still left with an error that says "XDR is required for remote driver". Most of our team cannot even reach this point, as any of the earlier steps is not reproducible due to either the environment not having the correct tooling or lacking sufficient administrator privilege to execute them. All of our code we've written thus far has relied entirely upon the CI to build the project for us, which is not a particularly efficient workflow due to the time it takes for CI to finish. How can we get ninja test (and other build tools) to actually run on our machines? Do you have any additional instructions that we may be able to use outside of the CONTRIBUTING.rst file? -Ryan Gahagan On Sat, Nov 7, 2020 at 12:04 PM Barrett J Schonefeld <bschoney@utexas.edu> wrote:

Michal,

We've struggled with the email system so far. You may have seen when we submitted a chain of 65 patches when we meant to send only the last commit. We tested this by sending it to ourselves first but must have made a mistake when submitting to libvir-list.

We submitted an initial patch for issue-11, which was rejected because Ryan emailed the patch, but I was CCed. I am working on getting my computer set up to submit this patch myself.

Additionally, we submitted a patch for issue-16 (from Ryan Gahagan), and we feel confident we did this correctly (even received a confirmation email). However, we have not seen the patch posted on the forum nor have we received any email that the patch was rejected. Is it possible it has not been posted because we left outstanding questions in the patch?

Best regards, Barrett Schonefeld

On Fri, Nov 6, 2020 at 2:03 AM Michal Prívozník <mprivozn@redhat.com> wrote:

...

...

Hey folks,

We appreciate the feedback, and we've used this to complete some initial work on issue 11.

We started with small changes in src/util, and we submitted them via email (following these guidelines on submitting patches <https://libvirt.org/submitting-patches.html>) to ensure we are on the right path.

We do not see the patch we submitted via email showing up in the threads, and we are wondering if we are submitting incorrectly or if

...

patch goes through some approval process before appearing in the

On 11/5/20 10:04 PM, Barrett J Schonefeld wrote: the threads.

...

Best regards, Barrett Schonefeld

So I checked the moderator's queue and there are no patches stuck, so probably it is something else. Did you see any error when 'git send-email'-ing the patches? When I was setting up my machine I used to send patches just to me to verify send-email is working and only then I tried to send patches to the list.

Also, are you sure you (the sender) are subscribed to the list? Generally, if you aren't then list won't forward your e-mails/patches.

Michal

On Mon, Nov 09, 2020 at 09:47:16AM +0000, Daniel P. Berrangé wrote:

On Sun, Nov 08, 2020 at 11:57:15AM -0600, Ryan Gahagan wrote:

...

We've also been having some troubles actually getting ninja and meson to run properly. Our team has one member on MacOS, one on Ubuntu 18.04, and one working on a remote server (Ubuntu again) without sudo privileges. We want to be able to run ninja test to properly test and clean our code as we make pull requests, but it's been very difficult to get set up.

The Ubuntu aptitude store has an outdated version of meson that doesn't actually run properly, and the pip3 version is up to date but then the build dependencies are left unresolved. These dependencies are also by default not actually in the aptitude store either, nor can they easily be mass installed via homebrew (to our knowledge). Even after manually configuring aptitude to find the links to all the dependencies of the project, manually installing meson and ninja, and installing the dependencies, we are still left with an error that says "XDR is required for remote driver". Most of our team cannot even reach this point, as any of the earlier steps is not reproducible due to either the environment not having the correct tooling or lacking sufficient administrator privilege to execute them.

All of our code we've written thus far has relied entirely upon the CI to build the project for us, which is not a particularly efficient workflow due to the time it takes for CI to finish. How can we get ninja test (and other build tools) to actually run on our machines? Do you have any additional instructions that we may be able to use outside of the CONTRIBUTING.rst file?

Almost all our CI systems are using containers for the build, and the container recipes are in the directory ci/containers. So if you want to see the set of packages to install on Ubuntu1804, then look at ci/containers/libvirt-ubuntu-1804.Dockerfile.

macOS CI is using a VM, and homebrew, but again we have a record of the build deps in ci/cirrus/libvirt-macos-1015.vars

NB, the macOS builds generally have many fewer features enabled, since Linux is the primary target of most developer's attention.

As an alternative to installing packages on your local machine, you can intead just install docker (or podman) container runtime tools. Then you can directly use the libvirt containers from our CI system on your local machine. In the "ci" sub-directory you can run a build on your local machine, using container images pulled down from GitLab CI. eg

cd ci make ci-build@fedora-32

will run a build using the Fedora 32 container image. This lets you build for any Linux distro, regardless of your host OS distro. ie you can test Fedora builds on an Ubuntu and vica-verca. "make ci-shell@distro" will give you have interactive shell in the container.

This is probably the easier way to get a local build running. Do all your work locally most of the time then you only need worry about pushing to GitLab to double check build success across the other distros just before submitting your contribution for review.

Yes it is, except it won't work in its current state, we haven't updated the build recipes to meson yet. You can find a patch in the attachment to get you going - I just quickly added/removed the necessary bits, so I need to polish it further for upstream, but it will do in the meantime. Erik