Re: [PATCH 16/25] squash into 'network: convert local pointers to g_auto*'

2020-06-24 Thread Laine Stump
OOPS!! I meant to squash this into patch 10 before posting. If you want to just review it separately I can squash it in before push. Or if you want to be pedantic I can squash it in and resend :-) On 6/24/20 11:34 PM, Laine Stump wrote: Signed-off-by: Laine Stump ---

[PATCH 14/25] network: replace VIR_ALLOC/REALLOC with g_new0/g_renew

2020-06-24 Thread Laine Stump
most of these are long-lived or attached to some other object, but a couple are automatics, and can take advantage of g_autoptr. Signed-off-by: Laine Stump --- src/network/bridge_driver.c | 44 +++-- 1 file changed, 13 insertions(+), 31 deletions(-) diff --git

[PATCH 18/25] nwfilter: define a typedef for struct ebtablesSubChainInst

2020-06-24 Thread Laine Stump
Signed-off-by: Laine Stump --- src/nwfilter/nwfilter_ebiptables_driver.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 8b77578117..cc814235aa 100644 ---

[PATCH 24/25] nwfilter: use standard label names when reasonable

2020-06-24 Thread Laine Stump
Rather than having labels named exit, done, exit_snooprequnlock, skip_rename, etc, use the standard "cleanup" label. And instead of err_exit, malformed, tear_down_tmpebchains, use "error". Signed-off-by: Laine Stump --- src/nwfilter/nwfilter_dhcpsnoop.c | 36 +++

[PATCH 08/25] network: fix memory leak in networkBuildDhcpDaemonCommandLine()

2020-06-24 Thread Laine Stump
hostsfilestr was not being freed. This will be turned into g_autofree in an upcoming patch converting a lot more of the same file to using g_auto*, but I wanted to make a separate patch for this first so the other patch is simpler to review. Signed-off-by: Laine Stump ---

[PATCH 11/25] network: use g_free() in place of remaining VIR_FREE()

2020-06-24 Thread Laine Stump
Signed-off-by: Laine Stump --- src/network/bridge_driver.c | 59 + 1 file changed, 33 insertions(+), 26 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 668aa9ca88..a1b2f5b6c7 100644 --- a/src/network/bridge_driver.c

[PATCH 21/25] nwfilter: convert local pointers to use g_auto*

2020-06-24 Thread Laine Stump
Signed-off-by: Laine Stump --- src/nwfilter/nwfilter_dhcpsnoop.c | 91 src/nwfilter/nwfilter_ebiptables_driver.c | 170 +- src/nwfilter/nwfilter_gentech_driver.c| 19 +-- src/nwfilter/nwfilter_learnipaddr.c | 9 +- 4 files changed, 108

[PATCH 20/25] nwfilter: remove unnecessary code from ebtablesGetSubChainInsts()

2020-06-24 Thread Laine Stump
On failure, this function would clear out and free the list of subchains it had been called with. This is unnecessary, because the *only* caller of this function will also clear out and free the list of subchains if it gets a failure from ebtablesGetSubChainInsts(). (It also makes more logical

[PATCH 10/25] network: convert local pointers to g_auto*

2020-06-24 Thread Laine Stump
This includes those that use plain VIR_FREE() as well as those that have a cleanup function defined for use via g_auto/g_autoptr (virCommand, virFirewall, virBuffer, virJSONValue etc). Signed-off-by: Laine Stump --- src/network/bridge_driver.c | 477 +++---

[PATCH 12/25] network: make networkDnsmasqXmlNsDef private to bridge_driver.c

2020-06-24 Thread Laine Stump
This struct isn't used anywhere else. Signed-off-by: Laine Stump --- src/network/bridge_driver.c | 10 ++ src/network/bridge_driver.h | 9 - 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index

[PATCH 16/25] squash into 'network: convert local pointers to g_auto*'

2020-06-24 Thread Laine Stump
Signed-off-by: Laine Stump --- src/network/bridge_driver_linux.c | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c index 0d0ac730f2..7f765bcf99 100644 --- a/src/network/bridge_driver_linux.c +++

[PATCH 04/25] util: validate return from xmlNodeGetContent before use

2020-06-24 Thread Laine Stump
There were a few uses of xmlNodeGetContent() that didn't check for NULL before using the result. A NULL return from xmlNodeGetContent() *could* (probably does) mean that there was an Out of Memory condition, but it is unclear from the documentation if that is always the case, or if it could just

[PATCH 22/25] nwfilter: convert remaining VIR_FREE() to g_free()

2020-06-24 Thread Laine Stump
Signed-off-by: Laine Stump --- src/nwfilter/nwfilter_dhcpsnoop.c | 16 src/nwfilter/nwfilter_driver.c| 10 +- src/nwfilter/nwfilter_ebiptables_driver.c | 2 +- src/nwfilter/nwfilter_gentech_driver.c| 6 +++--- src/nwfilter/nwfilter_learnipaddr.c

[PATCH 00/25] Several g_auto* conversion and related small bugfixes

2020-06-24 Thread Laine Stump
This started out with me noticing a memory leak in a patch that led to the realization that domain_conf.c hadn't been converted to use g_autofree yet, and each step of the way uncovered some other annoyance that I wanted to get rid of. Most of the changes are related to converting code to use

[PATCH 13/25] define g_autoptr cleanup function for virNetworkDHCPLease

2020-06-24 Thread Laine Stump
virNetworkDHCPLease and virNetworkDHCPLeaseFree() are declared in the public API file libvirt-network.h, and we can't pollute that with glib macro invocations, so put this in src/datatypes.h next to the other virNetwork items. Signed-off-by: Laine Stump --- src/datatypes.h | 2 ++ 1 file

[PATCH 02/25] use g_autoptr for all xmlBuffers

2020-06-24 Thread Laine Stump
AUTOPTR_CLEANUP_FUNC is set to xmlBufferFree() in util/virxml.h (This is actually new - added accidentally (but fortunately harmlessly!) in commit 257aba2dafe. I had added it along with the hunks in this patch, then decided to remove it and submit separately, but missed taking out the hunk in

[PATCH 09/25] util: add g_autoptr cleanup function for virFirewall objects

2020-06-24 Thread Laine Stump
Put in a separate patch so that two future patches can be re-ordered / selectively backported independent of each other. Signed-off-by: Laine Stump --- src/util/virfirewall.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/virfirewall.h b/src/util/virfirewall.h index

[PATCH 01/25] conf, vmx: check for OOM after calling xmlBufferCreate()

2020-06-24 Thread Laine Stump
Although libvirt itself uses g_malloc0() and friends, which exit when there isn't enouogh memory, libxml2 uses standard malloc(), which just returns NULL on OOM - this means we must check for NULL on return from any libxml2 functions that allocate memory. xmlBufferCreate(), for example, might

[PATCH 06/25] conf: eliminate useless error label in virDomainFeaturesDefParse()

2020-06-24 Thread Laine Stump
The error: label in this function just does "return -1", so replace all the "goto error" in the function with "return -1". Signed-off-by: Laine Stump --- src/conf/domain_conf.c | 91 -- 1 file changed, 44 insertions(+), 47 deletions(-) diff --git

[PATCH 25/25] replace g_new() with g_new0() for consistency

2020-06-24 Thread Laine Stump
g_new() is used in only 3 places. Switching them to g_new0() will do no harm, reduces confusion, and helps me sleep better at night knowing that all allocated memory is initialized to 0 :-) (Yes, I *know* that in all three cases the associated memory is immediately assigned some other value.

[PATCH 05/25] util: remove OOM error log from virGetHostnameImpl()

2020-06-24 Thread Laine Stump
The strings allocated in virGetHostnameImpl() are all allocated via g_strdup(), which will exit on OOM anyway, so the call to virReportOOMError() is redundant, and removing it allows slight modification to the code, in particular the cleanup label can be eliminated. Signed-off-by: Laine Stump

[PATCH 19/25] nwfilter replace VIR_ALLOC with g_new0

2020-06-24 Thread Laine Stump
Signed-off-by: Laine Stump --- src/nwfilter/nwfilter_dhcpsnoop.c | 9 +++-- src/nwfilter/nwfilter_driver.c| 3 +-- src/nwfilter/nwfilter_ebiptables_driver.c | 3 +-- src/nwfilter/nwfilter_gentech_driver.c| 3 +-- src/nwfilter/nwfilter_learnipaddr.c | 6 ++ 5

[PATCH 17/25] use g_autoptr() for all usages of virFirewallNew/Free

2020-06-24 Thread Laine Stump
Signed-off-by: Laine Stump --- src/nwfilter/nwfilter_ebiptables_driver.c | 63 +++ src/util/virebtables.c| 24 ++--- src/util/viriptables.c| 14 ++--- tests/virfirewalltest.c | 50 -- 4 files

[PATCH 07/25] util: eliminate error label in virDomainDefFormatInternalSetRootName()

2020-06-24 Thread Laine Stump
The only reason for the error label in this function is to call virBufferFreeAndReset(). It's actually more common for a failed format function to just leave the virBuffer alone and let the caller free it when there is a failure, and in fact the only caller of this function that *wasn't* already

[PATCH 03/25] conf: refactor virDomainBlkioDeviceParseXML to remove possible NULL dereference

2020-06-24 Thread Laine Stump
virDomainBlkioDeviceParseXML() has multiple cases of sending the return from xmlNodeGetContent() directly to virStrToLong_xx() without checking for NULL. Although it is *very* rare for xmlNodeGetContent() to return NULL (possibly it only happens in an OOM condition? The documentation is unclear),

[PATCH 15/25] network: use proper arg type when calling virNetDevSetOnline()

2020-06-24 Thread Laine Stump
The 2nd arg to this function is a bool, not an int. Signed-off-by: Laine Stump --- src/network/bridge_driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 1dee2fac6e..cbf5f05f30 100644 ---

[PATCH 23/25] nwfilter: transform logic in virNWFilterRuleInstSort to eliminate label

2020-06-24 Thread Laine Stump
This rewrite of a nested conditional produces the same results, but eliminate a goto and corresponding label. Signed-off-by: Laine Stump --- src/nwfilter/nwfilter_ebiptables_driver.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git

Re: [libvirt PATCH] qemu: ramfb video device doesn't support PCI address

2020-06-24 Thread Laine Stump
On 6/24/20 6:06 PM, Jonathon Jongsma wrote: Although a ramfb video device is not a PCI device, we don't currently report an error for ramfb device definitions containing a PCI address. However, a guest configured with such a device will fail to start: # virsh start test1 error: Failed

Re: [PATCH] network: Fix a race condition when shutdown & start vm at the same time

2020-06-24 Thread Laine Stump
On 6/19/20 1:16 PM, Daniel P. Berrangé wrote: On Fri, Jun 19, 2020 at 01:06:33PM -0400, Laine Stump wrote: On 6/19/20 12:26 PM, Daniel P. Berrangé wrote: Maintain a global "int nextTapID" counter, and just iterate on this. NIC names can be upto 16 bytes, so we'll create billions of devices

Re: [libvirt PATCH] qemu: ramfb video device doesn't support PCI address

2020-06-24 Thread Daniel Henrique Barboza
On 6/24/20 7:06 PM, Jonathon Jongsma wrote: Although a ramfb video device is not a PCI device, we don't currently report an error for ramfb device definitions containing a PCI address. However, a guest configured with such a device will fail to start: # virsh start test1 error:

Re: [PATCH 13/13] news: Document HMAT addition

2020-06-24 Thread Daniel Henrique Barboza
On 6/24/20 10:49 AM, Michal Privoznik wrote: Signed-off-by: Michal Privoznik --- NEWS.rst | 5 + 1 file changed, 5 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 2fc43c31b9..8990cdaf61 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -47,6 +47,11 @@ v6.5.0 (unreleased)

Re: [PATCH 12/13] qemu: Build HMAT command line

2020-06-24 Thread Daniel Henrique Barboza
On 6/24/20 10:49 AM, Michal Privoznik wrote: Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1786303 Signed-off-by: Michal Privoznik --- src/conf/numa_conf.c | 7 + src/qemu/qemu_command.c | 183 ++

Re: [PATCH 11/13] qemu: Introduce QEMU_CAPS_NUMA_HMAT capability

2020-06-24 Thread Daniel Henrique Barboza
On 6/24/20 10:49 AM, Michal Privoznik wrote: This capability tracks whether QEMU is capable of defining HMAT ACPI table for the guest. Signed-off-by: Michal Privoznik --- Reviewed-by: Daniel Henrique Barboza

Re: [PATCH 10/13] numa: expose HMAT APIs

2020-06-24 Thread Daniel Henrique Barboza
On 6/24/20 10:49 AM, Michal Privoznik wrote: These APIs will be used by QEMU driver when building the command line. Signed-off-by: Michal Privoznik --- Reviewed-by: Daniel Henrique Barboza

Re: [PATCH 09/13] conf: Validate NUMA HMAT configuration

2020-06-24 Thread Daniel Henrique Barboza
On 6/24/20 10:49 AM, Michal Privoznik wrote: There are several restrictions, for instance @initiator and @target have to refer to existing NUMA nodes (daa), @cache has to refer to a defined cache level and so on. Signed-off-by: Michal Privoznik --- Reviewed-by: Daniel Henrique Barboza

[libvirt PATCH] qemu: ramfb video device doesn't support PCI address

2020-06-24 Thread Jonathon Jongsma
Although a ramfb video device is not a PCI device, we don't currently report an error for ramfb device definitions containing a PCI address. However, a guest configured with such a device will fail to start: # virsh start test1 error: Failed to start domain test1 error: internal

Re: [PATCH 08/13] conf: Parse and format HMAT

2020-06-24 Thread Daniel Henrique Barboza
On 6/24/20 10:49 AM, Michal Privoznik wrote: To cite ACPI specification: Heterogeneous Memory Attribute Table describes the memory attributes, such as memory side cache attributes and bandwidth and latency details, related to the System Physical Address (SPA) Memory Ranges. The

Re: [PATCH 07/13] Allow NUMA nodes without vCPUs

2020-06-24 Thread Daniel Henrique Barboza
On 6/24/20 10:49 AM, Michal Privoznik wrote: QEMU allows creating NUMA nodes that have memory only. These are somehow important for HMAT. With check done in qemuValidateDomainDef() for QEMU 2.7 or newer, You're mentioning "QEMU 2.7 or newer" but the code in qemu_validate is checking for

Re: [PATCH 06/13] numa_conf: Make virDomainNumaSetNodeCpumask() return void

2020-06-24 Thread Daniel Henrique Barboza
On 6/24/20 10:49 AM, Michal Privoznik wrote: There is only one caller of virDomainNumaSetNodeCpumask() which checks for the return value but because the function will return NULL iff the @cpumask was NULL in the first place. But in that place @cpumask can't be NULL because it was just

Re: [PATCH 01/13] qemuxml2xmltest: Add "numatune-distance" test case

2020-06-24 Thread Daniel Henrique Barboza
On 6/24/20 5:43 PM, Daniel Henrique Barboza wrote: On 6/24/20 10:48 AM, Michal Privoznik wrote: This test case tests that expanding of NUMA distances work. On This first sentence seems odd. Perhaps change it to "This test case checks that expanding NUMA distance works" Forgot to

Re: [PATCH 05/13] qemuBuildMachineCommandLine: Drop needless check

2020-06-24 Thread Daniel Henrique Barboza
On 6/24/20 10:49 AM, Michal Privoznik wrote: The machine can not be NULL at this point - qemuDomainDefPostParse() makes sure it isn't. Signed-off-by: Michal Privoznik --- Reviewed-by: Daniel Henrique Barboza

Re: [PATCH 04/13] qemu_command: Rename qemuBuildNumaArgStr()

2020-06-24 Thread Daniel Henrique Barboza
On 6/24/20 10:49 AM, Michal Privoznik wrote: The function doesn't just build the argument for -numa. Since the -numa can be repeated multiple times, it also puts -numa onto the cmd line. Also, the rest of the functions has 'Command' infix. Signed-off-by: Michal Privoznik --- Reviewed-by:

Re: [PATCH 03/13] numa_conf: Drop CPU from name of two functions

2020-06-24 Thread Daniel Henrique Barboza
On 6/24/20 10:49 AM, Michal Privoznik wrote: There are two functions virDomainNumaDefCPUFormatXML() and virDomainNumaDefCPUParseXML() which format and parse domain's . There is nothing CPU specific about them. Drop the infix. Signed-off-by: Michal Privoznik --- Reviewed-by: Daniel

Re: [PATCH 02/13] conf: Move and rename virDomainParseScaledValue()

2020-06-24 Thread Daniel Henrique Barboza
On 6/24/20 10:49 AM, Michal Privoznik wrote: There is nothing domain specific about the function, thus it should not have virDomain prefix. Also, the fact that it is a static function makes it impossible to use from other files. Move the function to virxml.c and drop the 'Domain' infix.

Re: [PATCH 01/13] qemuxml2xmltest: Add "numatune-distance" test case

2020-06-24 Thread Daniel Henrique Barboza
On 6/24/20 10:48 AM, Michal Privoznik wrote: This test case tests that expanding of NUMA distances work. On This first sentence seems odd. Perhaps change it to "This test case checks that expanding NUMA distance works" input we accept if only distance from A to B is specified. On the

Re: [libvirt-publican PATCH 3/3] gitlab: add CONTRIBUTING.rst file to indicate use of merge requests

2020-06-24 Thread Andrea Bolognani
On Wed, 2020-06-24 at 18:11 +0100, Daniel P. Berrangé wrote: > With the introduction of automated CI pipelines, we are now ready to switch > to using merge requests for the project. With this switch we longer wish > to have patches sent to the mailing list. > > Signed-off-by: Daniel P. Berrangé

Re: [libvirt-publican PATCH 2/3] gitlab: introduce CI jobs for building content

2020-06-24 Thread Andrea Bolognani
On Wed, 2020-06-24 at 18:11 +0100, Daniel P. Berrangé wrote: > +++ b/ci/containers/refresh > @@ -0,0 +1,22 @@ > +#!/bin/sh > + > +if test -z "$1" > +then > +echo "syntax: $0 PATH-TO-LCITOOL" > +exit 1 > +fi > + > +LCITOOL=$1 > + > +if ! test -x "$LCITOOL" > +then > +echo "$LCITOOL is

Re: [libvirt-publican PATCH 1/3] cfg: remove obsolete "release" config entry

2020-06-24 Thread Andrea Bolognani
On Wed, 2020-06-24 at 18:11 +0100, Daniel P. Berrangé wrote: > Signed-off-by: Daniel P. Berrangé > --- > publican.cfg | 1 - > 1 file changed, 1 deletion(-) Under the assumption that this preemptively fixes a build failure that would show up once GitLab CI is enabled Reviewed-by: Andrea

Re: [PATCH libvirt-cim 1/2] gitlab: introduce CI jobs testing git master & distro libvirt

2020-06-24 Thread Andrea Bolognani
On Wed, 2020-06-24 at 17:20 +0100, Daniel P. Berrangé wrote: > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml > new file mode 100644 > index 000..75d34c3 > --- /dev/null > +++ b/.gitlab-ci.yml Oh and this doesn't apply cleanly on top of master, because you're trying to create a new file while

Re: [PATCH libvirt v2 0/5] Fix zPCI address auto-generation on s390

2020-06-24 Thread Andrea Bolognani
On Wed, 2020-06-24 at 18:59 +0200, Shalini Chellathurai Saroja wrote: > Ping, in case you missed it. Please wait at least a week or so before pinging a series, and refrain from CCing individual developers when you do - we're all subscribed to libvir-list. Anyway I've seen the series and I'm

Re: [PATCH libvirt-cim 1/2] gitlab: introduce CI jobs testing git master & distro libvirt

2020-06-24 Thread Andrea Bolognani
On Wed, 2020-06-24 at 17:20 +0100, Daniel P. Berrangé wrote: > The sandbox build needs to validate two axis > > - A variety of distro versions > - A variety of libvirt versions > > We test a variety of libvirt versions by running a build against the > distro provided libvirt packages. All

Re: [PATCH libvirt-cim 2/2] gitlab: add CONTRIBUTING.rst file to indicate use of merge requests

2020-06-24 Thread Andrea Bolognani
On Wed, 2020-06-24 at 17:20 +0100, Daniel P. Berrangé wrote: > With the introduction of automated CI pipelines, we are now ready to switch > to using merge requests for the project. With this switch we longer wish > to have patches sent to the mailing list, and thus the git-publish > config is

Re: [PATCH libvirt-dbus v2 2/3] gitlab: introduce CI jobs testing git master & distro libvirt

2020-06-24 Thread Andrea Bolognani
On Wed, 2020-06-24 at 17:26 +0100, Daniel P. Berrangé wrote: > +++ b/ci/containers/libvirt-centos-7.Dockerfile > +dnf install -y \ > +python3-docutils \ Sorry, one more thing: if I refresh Dockerfiles by pointing the included script to lcitool coming from libvirt-ci master, this line

Re: [PATCH libvirt-dbus v2 2/3] gitlab: introduce CI jobs testing git master & distro libvirt

2020-06-24 Thread Andrea Bolognani
On Wed, 2020-06-24 at 17:26 +0100, Daniel P. Berrangé wrote: > The dbus build needs to validate one axis > > - A variety of libvirt versions > > We test a variety of libvirt versions by running a build against the > distro provided libvirt packages. All that is then missing is a build >

[libvirt-publican PATCH 1/3] cfg: remove obsolete "release" config entry

2020-06-24 Thread Daniel P . Berrangé
Signed-off-by: Daniel P. Berrangé --- publican.cfg | 1 - 1 file changed, 1 deletion(-) diff --git a/publican.cfg b/publican.cfg index bd5bb97..5832d17 100644 --- a/publican.cfg +++ b/publican.cfg @@ -3,7 +3,6 @@ version: 0.1 xml_lang: en-US -release: 0 type: brand brand: libvirt --

[libvirt-publican PATCH 3/3] gitlab: add CONTRIBUTING.rst file to indicate use of merge requests

2020-06-24 Thread Daniel P . Berrangé
With the introduction of automated CI pipelines, we are now ready to switch to using merge requests for the project. With this switch we longer wish to have patches sent to the mailing list. Signed-off-by: Daniel P. Berrangé --- .gitpublish | 4 CONTRIBUTING.rst | 28

[libvirt-publican PATCH 2/3] gitlab: introduce CI jobs for building content

2020-06-24 Thread Daniel P . Berrangé
The docs build needs to validate one axis - A variety of publican versions We get coverage for this by running builds across various distros. Signed-off-by: Daniel P. Berrangé --- .gitlab-ci.yml| 118 ++ ci/containers/README.rst

[libvirt-publican PATCH 0/3] introduce GitLab CI and merge requests

2020-06-24 Thread Daniel P . Berrangé
The publican theme is used by the libvirt-appdev-guide-python, so we should test it. Daniel P. Berrangé (3): cfg: remove obsolete "release" config entry gitlab: introduce CI jobs for building content gitlab: add CONTRIBUTING.rst file to indicate use of merge requests .gitlab-ci.yml

Re: [PATCH libvirt v2 0/5] Fix zPCI address auto-generation on s390

2020-06-24 Thread Shalini Chellathurai Saroja
Ping, in case you missed it. On 6/18/20 10:25 AM, Shalini Chellathurai Saroja wrote: The zPCI address validation or autogeneration does not work as expected in the following scenarios 1. uid = 0 and fid = 0 2. uid = 0 and fid > 0 3. uid = 0 and fid not specified 4. uid not specified and fid > 0

[PATCH libvirt-dbus v2 2/3] gitlab: introduce CI jobs testing git master & distro libvirt

2020-06-24 Thread Daniel P . Berrangé
The dbus build needs to validate one axis - A variety of libvirt versions We test a variety of libvirt versions by running a build against the distro provided libvirt packages. All that is then missing is a build against the latest libvirt git master, which only needs to be run on a single

[PATCH libvirt-dbus v2 0/3] Introduce GitLab CI and merge requests

2020-06-24 Thread Daniel P . Berrangé
This introduces CI build coverage and then documents the switch to use merge requests. Daniel P. Berrangé (3): src: fix double free of virDomain object gitlab: introduce CI jobs testing git master & distro libvirt gitlab: add CONTRIBUTING.rst file to indicate use of merge requests

[PATCH libvirt-dbus v2 3/3] gitlab: add CONTRIBUTING.rst file to indicate use of merge requests

2020-06-24 Thread Daniel P . Berrangé
With the introduction of automated CI pipelines, we are now ready to switch to using merge requests for the project. With this switch we longer wish to have patches sent to the mailing list, and thus the git-publish config is removed. Reviewed-by: Andrea Bolognani Signed-off-by: Daniel P.

[PATCH libvirt-dbus v2 1/3] src: fix double free of virDomain object

2020-06-24 Thread Daniel P . Berrangé
The virDomainSnapshotGetDomain() method does NOT increment the refcount on the returned virDomain, so it must not be unrefed. This double free is responsible for random failures of the test_snapshot.py tests. Reviewed-by: Andrea Bolognani Signed-off-by: Daniel P. Berrangé ---

Re: [PATCH libvirt-dbus 3/3] gitlab: add CONTRIBUTING.rst file to indicate use of merge requests

2020-06-24 Thread Andrea Bolognani
On Wed, 2020-06-24 at 16:17 +0100, Daniel P. Berrangé wrote: > With the introduction of automated CI pipelines, we are now ready to switch > to using merge requests for the project. With this switch we longer wish > to have patches sent to the mailing list, and thus the git-publish > config is

[PATCH libvirt-cim 1/2] gitlab: introduce CI jobs testing git master & distro libvirt

2020-06-24 Thread Daniel P . Berrangé
The sandbox build needs to validate two axis - A variety of distro versions - A variety of libvirt versions We test a variety of libvirt versions by running a build against the distro provided libvirt packages. All that is then missing is a build against the latest libvirt git master, which

[PATCH libvirt-cim 2/2] gitlab: add CONTRIBUTING.rst file to indicate use of merge requests

2020-06-24 Thread Daniel P . Berrangé
With the introduction of automated CI pipelines, we are now ready to switch to using merge requests for the project. With this switch we longer wish to have patches sent to the mailing list, and thus the git-publish config is removed. Signed-off-by: Daniel P. Berrangé --- .gitpublish | 4

Re: [PATCH libvirt-dbus 2/3] gitlab: introduce CI jobs testing git master & distro libvirt

2020-06-24 Thread Andrea Bolognani
On Wed, 2020-06-24 at 16:17 +0100, Daniel P. Berrangé wrote: > The dbus build needs to validate one axis > > - A variety of libvirt versions > > We test a variety of libvirt versions by running a build against the > distro provided libvirt packages. All that is then missing is a build >

[PATCH libvirt-cim 0/2] Introduce GitLab CI and merge requests

2020-06-24 Thread Daniel P . Berrangé
Daniel P. Berrangé (2): gitlab: introduce CI jobs testing git master & distro libvirt gitlab: add CONTRIBUTING.rst file to indicate use of merge requests .gitlab-ci.yml| 130 ++ .gitpublish | 4 -

Re: [PATCH libvirt-dbus 1/3] src: fix double free of virDomain object

2020-06-24 Thread Andrea Bolognani
On Wed, 2020-06-24 at 16:17 +0100, Daniel P. Berrangé wrote: > The virDomainSnapshotGetDomain() method does NOT increment the > refcount on the returned virDomain, so it must not be unrefed. > > This double free is responsible for random failures of the > test_snapshot.py tests. > >

Re: [GSoC][PATCH 0/2] Extracting qemu domain jobs into a separate file

2020-06-24 Thread Michal Privoznik
On 6/24/20 12:45 PM, Prathamesh Chavan wrote: These patches aim towards extracting out the domain jobs into a separate file. Curretnly, we have named it as `qemu_domainjob`, keeping it similar to the already exisiting `qemu_blockjob` files. Prathamesh Chavan (2): qemu_domain: Avoid using

[PATCH libvirt-dbus 1/3] src: fix double free of virDomain object

2020-06-24 Thread Daniel P . Berrangé
The virDomainSnapshotGetDomain() method does NOT increment the refcount on the returned virDomain, so it must not be unrefed. This double free is responsible for random failures of the test_snapshot.py tests. Signed-off-by: Daniel P. Berrangé --- src/domainsnapshot.c | 4 ++-- 1 file changed,

[PATCH libvirt-dbus 2/3] gitlab: introduce CI jobs testing git master & distro libvirt

2020-06-24 Thread Daniel P . Berrangé
The dbus build needs to validate one axis - A variety of libvirt versions We test a variety of libvirt versions by running a build against the distro provided libvirt packages. All that is then missing is a build against the latest libvirt git master, which only needs to be run on a single

[PATCH libvirt-dbus 0/3] Introduce GitLab CI and merge requests

2020-06-24 Thread Daniel P . Berrangé
This introduces CI build coverage and then documents the switch to use merge requests. Daniel P. Berrangé (3): src: fix double free of virDomain object gitlab: introduce CI jobs testing git master & distro libvirt gitlab: add CONTRIBUTING.rst file to indicate use of merge requests

[PATCH libvirt-dbus 3/3] gitlab: add CONTRIBUTING.rst file to indicate use of merge requests

2020-06-24 Thread Daniel P . Berrangé
With the introduction of automated CI pipelines, we are now ready to switch to using merge requests for the project. With this switch we longer wish to have patches sent to the mailing list, and thus the git-publish config is removed. Signed-off-by: Daniel P. Berrangé --- .gitpublish | 4

Re: [PATCH] spec: Add the build dependency of make

2020-06-24 Thread Michal Privoznik
On 6/24/20 11:07 AM, Han Han wrote: For some minimal OS like fedora cloud image, the make is not installed by default. Signed-off-by: Han Han --- libvirt.spec.in | 1 + 1 file changed, 1 insertion(+) diff --git a/libvirt.spec.in b/libvirt.spec.in index cd7c33ab1a..9f24e06aa4 100644 ---

Re: [PATCH 6/6] kbase: incrementalbackupinternals: Describe 'block commit'

2020-06-24 Thread Eric Blake
On 6/24/20 9:07 AM, Peter Krempa wrote: oVirt does merge images out of libvirt in some cases. Add docs outlining how it's done from a high level. Signed-off-by: Peter Krempa --- docs/kbase/incrementalbackupinternals.rst | 37 +++ 1 file changed, 37 insertions(+) diff

Re: [PATCH 5/6] kbase: incrementalbackupinternals: Replace bash with pseudocode

2020-06-24 Thread Eric Blake
On 6/24/20 9:07 AM, Peter Krempa wrote: Simplify the docs and reduce maintenance burden by just describing the algorithm by a pseudo-language. Users are encouraged to use libvirt anyways and projects such as oVirt which do some management of storage themselves are unlikely to use bash anyways.

Re: [PATCH 4/6] kbase: incrementalbackupinternals: Add section on 'qemu-img bitmap' use

2020-06-24 Thread Eric Blake
On 6/24/20 9:07 AM, Peter Krempa wrote: Define what users should look for when wanting to manipulate bitmaps themselves. Later on a patch will turn the bash algorithms into pseudocode for simplicity. Signed-off-by: Peter Krempa --- docs/kbase/incrementalbackupinternals.rst | 14

Re: [PATCH 3/6] kbase: incrementalbackupinternals: Add secion on bitmap handling in shell

2020-06-24 Thread Eric Blake
On 6/24/20 9:07 AM, Peter Krempa wrote: Add a section that outlines usage of tools to handle bitmaps and introduce terms corresponding to the output of qemu-img to be used in further sections. With this we can simplify the section about checking bitmap health as we don't have to explain the

Re: [PATCH 2/6] kbase: incrementalbackupinternals: Clarify language in snapshots section

2020-06-24 Thread Eric Blake
On 6/24/20 9:07 AM, Peter Krempa wrote: Emphaisze what needs to happen and also that creating a snapshot doesn't Emphasize create the appropriate bitmaps. Also mention that granularity is kept. Signed-off-by: Peter Krempa --- docs/kbase/incrementalbackupinternals.rst | 23

Re: [PATCH 1/6] kbase: incrementalbackupinternals: Add snapshot terminology

2020-06-24 Thread Eric Blake
On 6/24/20 9:07 AM, Peter Krempa wrote: Make it obvious what's meant by 'overlay' and 'backing image' for sake of extension of the document. Signed-off-by: Peter Krempa --- docs/kbase/incrementalbackupinternals.rst | 15 +++ 1 file changed, 15 insertions(+) Reviewed-by: Eric

[PATCH 6/6] kbase: incrementalbackupinternals: Describe 'block commit'

2020-06-24 Thread Peter Krempa
oVirt does merge images out of libvirt in some cases. Add docs outlining how it's done from a high level. Signed-off-by: Peter Krempa --- docs/kbase/incrementalbackupinternals.rst | 37 +++ 1 file changed, 37 insertions(+) diff --git

[PATCH 5/6] kbase: incrementalbackupinternals: Replace bash with pseudocode

2020-06-24 Thread Peter Krempa
Simplify the docs and reduce maintenance burden by just describing the algorithm by a pseudo-language. Users are encouraged to use libvirt anyways and projects such as oVirt which do some management of storage themselves are unlikely to use bash anyways. Signed-off-by: Peter Krempa ---

[PATCH 0/6] kbase: Improve incremental backup docs

2020-06-24 Thread Peter Krempa
Refactor some sections, get rid of bash in favor of pseudo-code and describe block-commit. Peter Krempa (6): kbase: incrementalbackupinternals: Add snapshot terminology kbase: incrementalbackupinternals: Clarify language in snapshots section kbase: incrementalbackupinternals: Add secion

[PATCH 4/6] kbase: incrementalbackupinternals: Add section on 'qemu-img bitmap' use

2020-06-24 Thread Peter Krempa
Define what users should look for when wanting to manipulate bitmaps themselves. Later on a patch will turn the bash algorithms into pseudocode for simplicity. Signed-off-by: Peter Krempa --- docs/kbase/incrementalbackupinternals.rst | 14 ++ 1 file changed, 14 insertions(+) diff

[PATCH 3/6] kbase: incrementalbackupinternals: Add secion on bitmap handling in shell

2020-06-24 Thread Peter Krempa
Add a section that outlines usage of tools to handle bitmaps and introduce terms corresponding to the output of qemu-img to be used in further sections. With this we can simplify the section about checking bitmap health as we don't have to explain the qemu-img output but can refer to the newly

[PATCH 2/6] kbase: incrementalbackupinternals: Clarify language in snapshots section

2020-06-24 Thread Peter Krempa
Emphaisze what needs to happen and also that creating a snapshot doesn't create the appropriate bitmaps. Also mention that granularity is kept. Signed-off-by: Peter Krempa --- docs/kbase/incrementalbackupinternals.rst | 23 --- 1 file changed, 12 insertions(+), 11

[PATCH 1/6] kbase: incrementalbackupinternals: Add snapshot terminology

2020-06-24 Thread Peter Krempa
Make it obvious what's meant by 'overlay' and 'backing image' for sake of extension of the document. Signed-off-by: Peter Krempa --- docs/kbase/incrementalbackupinternals.rst | 15 +++ 1 file changed, 15 insertions(+) diff --git a/docs/kbase/incrementalbackupinternals.rst

Re: [PATCH 3/3] leaseshelper: Report more errors

2020-06-24 Thread Daniel P . Berrangé
On Mon, Jun 15, 2020 at 01:32:36PM +0200, Michal Privoznik wrote: > Some functions or code paths that may fail don't report error > (e.g. when acquiring PID file fails) leading to a silent quit > of the leaseshelper. This makes it super hard for us and users > to debug what is happening.

Re: [PATCH 2/3] leaseshelper: Wait to acquire PID file

2020-06-24 Thread Daniel P . Berrangé
On Mon, Jun 15, 2020 at 01:32:35PM +0200, Michal Privoznik wrote: > On a DHCP transaction, dnsmasq runs our leases helper which > updates corresponding JSON files. While one dnsmasq won't run the > leaseshelper in parallel, two dnsmasqs (from two distinct > networks) might. To avoid corrupting

Re: [PATCH 0/3] leaseshelper: Wait to acquire PID file

2020-06-24 Thread Michal Privoznik
On 6/15/20 1:32 PM, Michal Privoznik wrote: Patch 1/3 is for demonstration purposes only. The rest patches actual problem.I was also thinking of making the PID file per bridge, i.e. include bridge name in the name (e.g. /var/run/virbr0_leaseshelper.pid) to decrease pressure on the single PID

[PATCH 05/13] qemuBuildMachineCommandLine: Drop needless check

2020-06-24 Thread Michal Privoznik
The machine can not be NULL at this point - qemuDomainDefPostParse() makes sure it isn't. Signed-off-by: Michal Privoznik --- src/qemu/qemu_command.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 05e5c19118..c5b0ee231e 100644

[PATCH 13/13] news: Document HMAT addition

2020-06-24 Thread Michal Privoznik
Signed-off-by: Michal Privoznik --- NEWS.rst | 5 + 1 file changed, 5 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 2fc43c31b9..8990cdaf61 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -47,6 +47,11 @@ v6.5.0 (unreleased) alphabetical order. Hook script in old place will be executed

[PATCH 06/13] numa_conf: Make virDomainNumaSetNodeCpumask() return void

2020-06-24 Thread Michal Privoznik
There is only one caller of virDomainNumaSetNodeCpumask() which checks for the return value but because the function will return NULL iff the @cpumask was NULL in the first place. But in that place @cpumask can't be NULL because it was just allocated by virBitmapParse(). Signed-off-by: Michal

[PATCH 04/13] qemu_command: Rename qemuBuildNumaArgStr()

2020-06-24 Thread Michal Privoznik
The function doesn't just build the argument for -numa. Since the -numa can be repeated multiple times, it also puts -numa onto the cmd line. Also, the rest of the functions has 'Command' infix. Signed-off-by: Michal Privoznik --- src/qemu/qemu_command.c | 10 +- 1 file changed, 5

[PATCH 09/13] conf: Validate NUMA HMAT configuration

2020-06-24 Thread Michal Privoznik
There are several restrictions, for instance @initiator and @target have to refer to existing NUMA nodes (daa), @cache has to refer to a defined cache level and so on. Signed-off-by: Michal Privoznik --- src/conf/domain_conf.c | 3 ++ src/conf/numa_conf.c | 99

[PATCH 10/13] numa: expose HMAT APIs

2020-06-24 Thread Michal Privoznik
These APIs will be used by QEMU driver when building the command line. Signed-off-by: Michal Privoznik --- src/conf/numa_conf.c | 139 +++ src/conf/numa_conf.h | 28 src/libvirt_private.syms | 6 ++ 3 files changed, 173 insertions(+) diff

[PATCH 12/13] qemu: Build HMAT command line

2020-06-24 Thread Michal Privoznik
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1786303 Signed-off-by: Michal Privoznik --- src/conf/numa_conf.c | 7 + src/qemu/qemu_command.c | 183 ++ .../numatune-hmat.x86_64-latest.args | 53 +

[PATCH 08/13] conf: Parse and format HMAT

2020-06-24 Thread Michal Privoznik
To cite ACPI specification: Heterogeneous Memory Attribute Table describes the memory attributes, such as memory side cache attributes and bandwidth and latency details, related to the System Physical Address (SPA) Memory Ranges. The software is expected to use this information as hint

[PATCH 07/13] Allow NUMA nodes without vCPUs

2020-06-24 Thread Michal Privoznik
QEMU allows creating NUMA nodes that have memory only. These are somehow important for HMAT. With check done in qemuValidateDomainDef() for QEMU 2.7 or newer, we can be sure that the vCPUs are fully assigned to NUMA nodes in domain XML. Signed-off-by: Michal Privoznik ---

  1   2   >