[PATCH 4/5] util: simplify virNetDevMacVLanCreateWithVPortProfile()

the device in the case that a specific name was given in the XML - all we need to do is log any requested name, and then call exactly the same code as we would if no name was given. Signed-off-by: Laine Stump --- src/util/virnetdevmacvlan.c | 64 ++--- 1 file changed, 10

[PATCH 1/5] util: fix tap device name auto-generation for FreeBSD

e is empty it generates a new name with the proper prefix, and if it's not empty, it leaves it alone. Signed-off-by: Laine Stump --- src/util/virnetdevtap.c | 35 ++- 1 file changed, 6 insertions(+), 29 deletions(-) diff --git a/src/util/virnetdevtap.c

Re: [RFC PATCH 1/6] compatibilities: added in flags for NFS support

(answering for Peter since he is on "vacation", so it may be awhile before he gets around to this) On 12/14/20 5:54 PM, Ryan Gahagan wrote: On Fri, Dec 11, 2020 at 3:35 AM Peter Krempa > wrote: In subject/summary. We don't have anything which we'd prefix with

Re: [RFC PATCH 3/6] docs: added rng schema and formatdomain for NFS

On 12/14/20 5:57 PM, Ryan Gahagan wrote: On Thu, Dec 10, 2020 at 10:37 PM Han Han > wrote: On Fri, Dec 11, 2020 at 4:00 AM Ryan Gahagan mailto:rgaha...@cs.utexas.edu>> wrote: diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index

Re: [PATCHv3 4/5] netdevveth: Simplify virNetDevVethCreate by using virNetDevGenerateName

On 12/13/20 8:50 PM, Shi Lei wrote: Simplify virNetDevVethCreate by using common GenerateName/ReserveName functions. Signed-off-by: Shi Lei --- src/lxc/lxc_process.c| 3 + src/util/virnetdevveth.c | 140 +-- 2 files changed, 36 insertions(+), 107

[PATCH] lxc: don't try to reserve macvtap name for LXC domains

nction in the case of the LXC driver, since it is pointless anyway. Signed-off-by: Laine Stump --- src/lxc/lxc_process.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 0f818e2ee0..79d84617d1 100644 --- a/src/lxc/lxc_process.c +++ b/s

Re: [PATCHv3 0/5] netdev: Extract GenerateName/ReserveName as common functions

On 12/14/20 12:49 PM, Daniel Henrique Barboza wrote: On 12/13/20 10:50 PM, Shi Lei wrote: V2 here: https://www.redhat.com/archives/libvir-list/2020-December/msg00563.html Since V2:   *  Fix libxl driver for missing changing virNetDevMacVLanReserveName V1 here:

Re: [PATCHv2 3/5] netdevmacvlan: Use helper function to create unique macvlan/macvtap name

On 12/13/20 8:20 PM, Shi Lei wrote: On 2020-12-14 at 08:58, Laine Stump wrote: On 12/9/20 10:00 PM, Shi Lei wrote: Simplify ReserveName/GenerateName for macvlan and macvtap by using common functions. Signed-off-by: Shi Lei ---    src/libvirt_private.syms    |   1 -    src/lxc/lxc_process.c

Re: [PATCHv2 1/5] netdev: Introduce several helper functions for generating unique netdev name

On 12/9/20 10:00 PM, Shi Lei wrote: Extract ReserveName/GenerateName from netdevtap and netdevmacvlan as common helper functions. Signed-off-by: Shi Lei --- src/bhyve/bhyve_command.c | 4 +- src/conf/domain_conf.c | 4 +-

Re: [PATCHv2 3/5] netdevmacvlan: Use helper function to create unique macvlan/macvtap name

On 12/9/20 10:00 PM, Shi Lei wrote: Simplify ReserveName/GenerateName for macvlan and macvtap by using common functions. Signed-off-by: Shi Lei --- src/libvirt_private.syms| 1 - src/lxc/lxc_process.c | 2 +- src/qemu/qemu_process.c | 2 +- src/util/virnetdevmacvlan.c

[libvirt PATCH v2 7/7] qemu: remove now-redundant check for file length when determining PCIe vs. PCI

Now that virPCIDeviceIsPCIExpress() checks the length of the file when the process lacks sufficient privilege to read the entire PCI config file in sysfs, we can remove the open-coding for that case from its consumer. Signed-off-by: Laine Stump --- src/qemu/qemu_domain_address.c | 38

[libvirt PATCH v2 6/7] util: make virPCIDeviceIsPCIExpress() more intelligent

CIDeviceIsPCIExpress(), and do that check any time we fail while reading the config file (not only when the process is non-root). Fixes: https://bugzilla.redhat.com/1901685 Signed-off-by: Laine Stump --- src/util/virpci.c | 66 --- 1 file changed, 62 insert

[libvirt PATCH v2 5/7] util: change call sequence for virPCIDeviceFindCapabilityOffset()

pabilities, and so easily recognizeable). An upcoming patch will take advantage of the change made here. Signed-off-by: Laine Stump --- src/util/virpci.c | 77 --- 1 file changed, 66 insertions(+), 11 deletions(-) diff --git a/src/util/virpci.c b/src/uti

[libvirt PATCH v2 4/7] util: make read error of PCI config file more detailed

The new message is more verbose/useful, but only logged at debug level instead of as a warning (since it could easily happen in a non-error situation). Signed-off-by: Laine Stump --- src/util/virpci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virpci.c b

[libvirt PATCH v2 2/7] util: simplify calling of virPCIDeviceDetectFunctionLevelReset()

This function returned an int, and that int was being checked for < 0 in its solitary caller, but within the function it would only ever return 0 or 1. Change the function itself to return a bool, and the caller to just directly set the flag in the virPCIDevice. Signed-off-by: Laine St

[libvirt PATCH v2 3/7] util: simplify call to virPCIDeviceDetectPowerManagementReset()

This function returned an int, but would only return 0 or 1, and the one place it was called would just use !! to convert that value to a bool. Change the function to directly return bool instead. Signed-off-by: Laine Stump --- src/util/virpci.c | 8 1 file changed, 4 insertions(+), 4

[libvirt PATCH v2 1/7] qemu: use g_autoptr for a virPCIDevice

. Signed-off-by: Laine Stump --- src/qemu/qemu_domain_address.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index d872f75b38..b07672e2f4 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu

[libvirt PATCH v2 0/7] fix PCI vs PCIe detection when running libvirtd as root, but unprivileged

danpb had asked for some documentation on the changes to the APIs of internal functions in V1. I've added some standard function header docs in patches 5 & 6. Everything else is completely unchanged (and already reviewed by mprivozn). Laine Stump (7): qemu: use g_autoptr for a virPCIDe

Re: [PATCH 5/7] util: change call sequence for virPCIDeviceFindCapabilityOffset()

On 12/9/20 4:18 AM, Daniel P. Berrangé wrote: On Tue, Dec 08, 2020 at 08:37:43PM -0500, Laine Stump wrote: Previously there was no way to differentiate between this function 1) encountering an error while reading the pci config, and 2) determining that the device in question is a conventional

[PATCH 6/7] util: make virPCIDeviceIsPCIExpress() more intelligent

CIDeviceIsPCIExpress(), and do that check any time we fail while reading the config file (not only when the process is non-root). Fixes: https://bugzilla.redhat.com/1901685 Signed-off-by: Laine Stump --- src/util/virpci.c | 42 ++ 1 file changed, 38 insert

[PATCH 5/7] util: change call sequence for virPCIDeviceFindCapabilityOffset()

or Express Capabilities, and so easily recognizeable). An upcoming patch will take advantage of the change made here. Signed-off-by: Laine Stump --- src/util/virpci.c | 42 +++--- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/src/util/virpci.c b/src/uti

[PATCH 4/7] util: make read error of PCI config file more detailed

The new message is more verbose/useful, but only logged at debug level instead of as a warning (since it could easily happen in a non-error situation). Signed-off-by: Laine Stump --- src/util/virpci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virpci.c b

[PATCH 7/7] qemu: remove now-redundant check for file length when determining PCIe vs. PCI

Now that virPCIDeviceIsPCIExpress() checks the length of the file when the process lacks sufficient privilege to read the entire PCI config file in sysfs, we can remove the open-coding for that case from its consumer. Signed-off-by: Laine Stump --- src/qemu/qemu_domain_address.c | 38

[PATCH 2/7] util: simplify calling of virPCIDeviceDetectFunctionLevelReset()

This function returned an int, and that int was being checked for < 0 in its solitary caller, but within the function it would only ever return 0 or 1. Change the function itself to return a bool, and the caller to just directly set the flag in the virPCIDevice. Signed-off-by: Laine St

[PATCH 1/7] qemu: use g_autoptr for a virPCIDevice

. Signed-off-by: Laine Stump --- src/qemu/qemu_domain_address.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index d872f75b38..b07672e2f4 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu

[PATCH 3/7] util: simplify call to virPCIDeviceDetectPowerManagementReset()

This function returned an int, but would only return 0 or 1, and the one place it was called would just use !! to convert that value to a bool. Change the function to directly return bool instead. Signed-off-by: Laine Stump --- src/util/virpci.c | 8 1 file changed, 4 insertions(+), 4

[PATCH 0/7] fix PCI vs PCIe detection when running libvirtd as root, but unprivileged

I think there's an adequate description in patch 6 (and maybe in the bugzilla record: https://bugzilla.redhat.com/1901685). I already typed a long description once, and git send-email failed due to network problems and threw away everything I typed. I don't feel like typing it again. Laine Stump

Re: [PATCHv2 2/2] util:veth: Create veth device pair by netlink

On 12/8/20 4:19 AM, Daniel P. Berrangé wrote: On Mon, Dec 07, 2020 at 05:54:51PM -0500, Laine Stump wrote: On 11/22/20 10:28 PM, Shi Lei wrote: When netlink is supported, use netlink to create veth device pair rather than 'ip link' command. Signed-off-by: Shi Lei --- src/util

Re: [PATCH 5/5] netdevveth: Simplify virNetDevVethCreate by using virNetDevGenerateName

On 12/4/20 2:01 AM, Shi Lei wrote: Simplify virNetDevVethCreate by using common GenerateName/ReserveName functions. Signed-off-by: Shi Lei --- src/lxc/lxc_process.c| 3 + src/util/virnetdevveth.c | 146 +++ 2 files changed, 43 insertions(+), 106

Re: [PATCH 4/5] netdev: Enable virNetDevGenerateName to support veth

On 12/4/20 2:01 AM, Shi Lei wrote: Signed-off-by: Shi Lei --- src/util/virnetdev.c | 2 ++ src/util/virnetdev.h | 3 +++ 2 files changed, 5 insertions(+) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 5ff8e35f..ff1b1fa0 100644 --- a/src/util/virnetdev.c +++

Re: [PATCH 3/5] netdevmacvlan: Use helper function to create unique macvlan/macvtap name

On 12/4/20 2:01 AM, Shi Lei wrote: Simplify ReserveName/GenerateName for macvlan and macvtap by using common functions. Signed-off-by: Shi Lei --- src/util/virnetdevmacvlan.c | 107 src/util/virnetdevmacvlan.h | 6 -- 2 files changed, 22

Re: [PATCH 2/5] netdevtap: Use common helper function to create unique tap name

On 12/4/20 2:01 AM, Shi Lei wrote: Simplify GenerateName/ReserveName for netdevtap by using common functions. Signed-off-by: Shi Lei --- src/util/virnetdevtap.c | 58 +++-- 1 file changed, 4 insertions(+), 54 deletions(-) diff --git

Re: [PATCH 2/5] netdevtap: Use common helper function to create unique tap name

On 12/4/20 2:01 AM, Shi Lei wrote: Simplify GenerateName/ReserveName for netdevtap by using common functions. Signed-off-by: Shi Lei --- src/util/virnetdevtap.c | 58 +++-- 1 file changed, 4 insertions(+), 54 deletions(-) diff --git

Re: [PATCH 1/5] netdev: Introduce several helper functions for generating unique netdev name

First - thanks for taking this on, and doing it the right way! (i.e. refactoring my two changes down into a common set of functions to be reused, rather than duplicating the code). I can't really say why I didn't go the extra bit when I made the changes to virnetdevtap.c and virnetdevmacvlan.c

Re: [PATCHv2 2/2] util:veth: Create veth device pair by netlink

On 11/22/20 10:28 PM, Shi Lei wrote: When netlink is supported, use netlink to create veth device pair rather than 'ip link' command. Signed-off-by: Shi Lei --- src/util/virnetdevveth.c | 85 ++-- 1 file changed, 56 insertions(+), 29 deletions(-) diff

Re: [PATCHv2 1/2] util:netlink: Enable virNetlinkNewLink to support veth

On 11/22/20 10:28 PM, Shi Lei wrote: Signed-off-by: Shi Lei Reviewed-by: Laine Stump

Re: [PATCH] spec: keep existing nwfilters uuid on update

On 12/3/20 12:31 PM, Andrea Bolognani wrote: On Thu, 2020-12-03 at 17:39 +0100, Michal Privoznik wrote: On 10/26/20 10:21 AM, Nikolay Shirokovskiy wrote: -cp %{_datadir}/libvirt/nwfilter/*.xml %{_sysconfdir}/libvirt/nwfilter/ +# keep existing filters uuid on update +for dfile in

RFC: deprecating/obsoleting netcf package and libvirt virInterface*() APIs

netcf (the backend of libvirt's virInterface*() APIs) hasn't been modified in over 2 years, and the last time there was a change significant enough for an upstream release was in 2015 (!). It has never been possible to reliably translate back and forth between native and netcf/libvirt XML

Re: [PATCH 8/8] util: call iptables directly rather than via firewalld

On 11/24/20 7:01 AM, Daniel Henrique Barboza wrote: On 11/24/20 12:30 AM, Laine Stump wrote: When libvirt added support for firewalld, we were unable to use firewalld's higher level rules, because they weren't detailed enough and could not be applied to the iptables FORWARD or OUTPUT chains

[PATCH 6/8] util: always check for ebtables/iptables binaries, even when using firewalld

-by: Laine Stump --- src/util/virfirewall.c | 56 -- 1 file changed, 26 insertions(+), 30 deletions(-) diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index 694bb32f62..0b022b14af 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c

[PATCH 4/8] tests: fix iptables test case commandline options in virfirewalltest.c

complained about the bad option, rather than complaining that I had insufficient privilege to run the command). Signed-off-by: Laine Stump --- tests/virfirewalltest.c | 168 1 file changed, 84 insertions(+), 84 deletions(-) diff --git a/tests/virfirewa

[PATCH 3/8] util/tests: enable locking on iptables/ebtables commandlines by default

by libvirt now uses new enough versions of both iptables and ebtables that they all have support for -w/--concurrent. That means we can finally remove the conditional code and simply always use them. Signed-off-by: Laine Stump --- src/libvirt_private.syms | 1 - src/uti

[PATCH 7/8] util: synchronize with firewalld before we start calling iptables directly

message, we ignore the result, for good measure). (This patch is being done *before* the patch that switches to calling iptables directly, so that everything will function properly with any fractional part of the series applied). Signed-off-by: Laine Stump --- src/libvirt_private.syms | 1 +

[PATCH 1/8] util: fix typo in VIR_MOCK_WRAP_RET_ARGS()

ipv4 rules, this has never made any difference in practice, but I'm making some other changes in this file and just couldn't let it stand :-) Signed-off-by: Laine Stump --- tests/virfirewalltest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/virfirewalltest.c b/tests/vir

[PATCH 8/8] util: call iptables directly rather than via firewalld

by capturing the generated commandline as it does for the DIRECT backend, rather than capturing dbus messages using a mocked dbus API). Signed-off-by: Laine Stump --- src/util/virfirewall.c | 13 +++-- tests/virfirewalltest.c | 30 -- 2 files changed, 31 ins

[PATCH 5/8] network: be more verbose about the reason for a firewall reload

Signed-off-by: Laine Stump --- src/network/bridge_driver.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 5d9b9eaa4f..fdad2191e6 100644 --- a/src/network/bridge_driver.c +++ b/src/network

[PATCH 0/8] run iptables directly rather than via firewalld

The reasoning for this is explained in Patch 8/8 Laine Stump (8): util: fix typo in VIR_MOCK_WRAP_RET_ARGS() util/tests: enable locking on iptables/ebtables commandlines in unit tests util/tests: enable locking on iptables/ebtables commandlines by default tests: fix iptables test

Re: [PATCH 1/2] util:netlink: Enable virNetlinkNewLink to support veth

On 11/17/20 1:48 AM, Shi Lei wrote: Signed-off-by: Shi Lei --- src/util/virnetlink.c | 25 + src/util/virnetlink.h | 1 + 2 files changed, 26 insertions(+) diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index fdd3a6a4..e191f63b 100644 ---

Re: [PATCH 2/2] util:veth: Create veth device pair by netlink

On 11/17/20 1:48 AM, Shi Lei wrote: When netlink is supported, use netlink to create veth device pair rather than 'ip link' command. Signed-off-by: Shi Lei --- src/util/virnetdevveth.c | 39 ++- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git

Re: Issue 90 Further Clarifications

On 11/20/20 1:07 PM, Dustan B Helm wrote: On Fri, Nov 20, 2020 at 5:33 AM Peter Krempa > wrote: On Fri, Nov 20, 2020 at 09:41:44 +, Daniel Berrange wrote: > On Thu, Nov 19, 2020 at 03:48:48PM -0600, Dustan B Helm wrote: > > [image: Dustan Helm]

Re: [PATCH] util: convert char pointers to use g_autofree

On 11/20/20 11:43 AM, Barrett J Schonefeld wrote: I appreciate the feedback on this patch! I will work on splitting this into multiple patches. I believe this will involve redoing much of the work because I will need to split this patch (a single commit) into many commits. One suggestion on

Re: [libvirt PATCH v2 0/3] network: Drop UUID handling for default network

On 11/20/20 5:53 AM, Andrea Bolognani wrote: On Thu, 2020-11-19 at 11:03 -0500, Laine Stump wrote: We should change the other drivers that have uuids so that they also save if the uuid was added during parse. If that sounds boring to you, then I can put it on a list somewhere. I can't

Re: [libvirt PATCH] qemu: fix setting of scsi-id for ESP SCSI controllers

picion, so it was only momentary :-) Switching the code to use an enum will force authors to pay attention to this difference when adding future SCSI controllers. Signed-off-by: Daniel P. Berrangé Reviewed-by: Laine Stump (with extra points for keeping even the thought of last-century hardw

Re: [PATCH 1/2] hyperv: XML parsing of storage volumes

On 11/19/20 8:38 AM, Daniel P. Berrangé wrote: On Wed, Nov 18, 2020 at 01:48:24PM -0500, Matt Coleman wrote: dumpxml can now serialize: * floppy drives * file-backed and device-backed disk drives * images mounted to virtual CD/DVD drives * IDE and SCSI controllers Co-authored-by: Sri Ramanujam

Re: [libvirt PATCH v2 0/3] network: Drop UUID handling for default network

files changed, 23 insertions(+), 30 deletions(-) Reviewed-by: Laine Stump for the series. We should change the other drivers that have uuids so that they also save if the uuid was added during parse. If that sounds boring to you, then I can put it on a list somewhere. I can't guarantee

Re: [PATCH] util: convert char pointers to use g_autofree

On 11/19/20 6:46 AM, Tim Wiederhake wrote: On Wed, 2020-11-18 at 16:47 -0600, Ryan Gahagan wrote: From: Barrett Schonefeld additional conversions to the GLib API in src/util per issue #11. [...] return ret; } I believe you can remove "cleanup" and "ret" as well. More instances

Re: [libvirt PATCH 1/2] network: Drop UUID handling for default network

On 11/17/20 12:45 PM, Andrea Bolognani wrote: On Tue, 2020-11-17 at 10:47 -0500, Laine Stump wrote: On 11/16/20 11:43 AM, Andrea Bolognani wrote: I don't see how we could possibly not be okay with writing back the updated configuration: filling in whatever blanks are present in the user

Re: [libvirt PATCH 2/4] nodedev: dont rely on ignoring errors on missing properties

, but that's not what happens in practice. Instead the property simply won't get set at all. Taking you at your word that, e.g. ID_CDROM will *always* be unset, and never set to 0 when the device isn't a CD, Reviewed-by: Laine Stump (It totally makes sense that this would never happen, and I

Re: [libvirt PATCH 0/4] nodedev: cleanup some historical baggage

integer properties src/node_device/node_device_udev.c | 70 +- 1 file changed, 30 insertions(+), 40 deletions(-) Series Reviewed-by: Laine Stump (I made one comment on 2/4, but it's nothing that needs any action - just pointing out the level of trust you have

Re: [libvirt PATCH 1/2] network: Drop UUID handling for default network

On 11/16/20 11:43 AM, Andrea Bolognani wrote: On Mon, 2020-11-16 at 10:00 -0500, Laine Stump wrote: On 11/16/20 5:33 AM, Andrea Bolognani wrote: On Sun, 2020-11-15 at 19:19 -0500, Laine Stump wrote: On 11/15/20 3:43 PM, Andrea Bolognani wrote: We are generating a fresh UUID and storing

Re: nwfilter issue with new ebtables

On 11/16/20 10:36 AM, Daniel P. Berrangé wrote: On Mon, Nov 16, 2020 at 10:23:32AM -0500, Laine Stump wrote: On 11/16/20 2:01 AM, Christian Ehrhardt wrote: Hi, I have last week discussed breakage in nwfilter usage on IRC virsh start error: Failed to start domain

Re: nwfilter issue with new ebtables

On 11/16/20 2:01 AM, Christian Ehrhardt wrote: Hi, I have last week discussed breakage in nwfilter usage on IRC virsh start error: Failed to start domain error: internal error: applyDHCPOnlyRules failed - spoofing not protect With debug in the logs enabled I got

Re: [libvirt PATCH 1/2] network: Drop UUID handling for default network

On 11/16/20 5:33 AM, Andrea Bolognani wrote: On Sun, 2020-11-15 at 19:19 -0500, Laine Stump wrote: On 11/15/20 3:43 PM, Andrea Bolognani wrote: We are generating a fresh UUID and storing it in the XML for the default network, but this is unnecessary because the network driver

Re: [libvirt PATCH 1/2] network: Drop UUID handling for default network

On 11/15/20 3:43 PM, Andrea Bolognani wrote: We are generating a fresh UUID and storing it in the XML for the default network, but this is unnecessary because the network driver will automatically generate one if it's missing from the XML; But that automatically generated uuid will not be

Re: [PATCH] util: remove ATTRIBUTE_NONNULL from virDirClose declaration

After short discussion on IRC, I pushed this as "trivial". On 11/13/20 12:42 PM, Laine Stump wrote: Before commit 24d8968c, virDirClose took a DIR**, and that was never NULL, so its declaration included ATTRIBUTE_NONNULL(1). Since that commit, virDirClose takes a DIR*, and it may be

[PATCH] util: remove ATTRIBUTE_NONNULL from virDirClose declaration

-by: Laine Stump --- src/util/virfile.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/util/virfile.h b/src/util/virfile.h index ba31a78e58..32505826ee 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -269,8 +269,7 @@ int virDirOpenQuiet(DIR **dirp, const char

Re: [PATCH] domain_conf: make virDomainDiskSetSource() void

On 11/5/20 11:26 AM, Michal Prívozník wrote: On 11/5/20 7:23 AM, Matt Coleman wrote: The function only returns zero or aborts, so it might as well be void. This has the added benefit of simplifying the code that calls it. Signed-off-by: Matt Coleman ---   src/conf/domain_conf.c   |  3 +--  

Re: [libvirt PATCH v6 1/2] news: mention vdpa support

APIs also now list and provide XML descriptions for vDPA devices." ? Reviewed by: Laine Stump Let me know if that change is okay with you, and I'll push it.

Re: [libvirt PATCH v6 2/2] nodedev: fix resource leak

Oh shoot! I wrote/posted a patch for this (because I'd just finished making a long patch series to use g_autoptr for all DIR*, and wanted to update it to take this into account) on Tuesday or Wednesday, it was reviewed, and I pulled it into master of my local tree; I *thought* I had pushed it

Re: [PATCH 00/12] use g_autoptr for all DIR*

On 10/28/20 7:39 AM, Daniel Henrique Barboza wrote: Hey, On 10/27/20 10:35 PM, Laine Stump wrote: I don't even remember why I started looking at this. I think that again I was considering changing some function, and making the DIR* in that function autoclose was a prerequisite for a reasonably

[PATCH] node_device: fix leak of DIR*

Commit 53aec799fa31 introduced the function udevGetVDPACharDev(), which scans a directory using virDirOpenIfExists() and virDirRead(). It unfortunately forgets to close the DIR* when it is finished with it. This patch fixes that omission. Signed-off-by: Laine Stump --- src/node_device

[PATCH 12/12] remove unnecessary cleanup labels and unused return variables

the return variable itself. Signed-off-by: Laine Stump --- src/conf/virnetworkobj.c | 32 +++--- src/qemu/qemu_interop_config.c | 11 +++- src/storage/storage_util.c | 50 +- src/util/vircgroup.c | 16 --- src/util/vi

[PATCH 11/12] util: refactor function to simplify and remove label

is now just "return 0;" becomes the "if", and the new "else" no longer needs to be inside the conditional. 3) the value of firstEntryName can be moved into *netname with g_steal_pointer() Once that is all done, ret is no lo

[PATCH 07/12] util: declare g_autoptr cleanup function to auto-close DIR*

Signed-off-by: Laine Stump --- src/util/virfile.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/virfile.h b/src/util/virfile.h index 6fde4f88ca..6973fbd54a 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -271,6 +271,7 @@ int virDirRead(DIR *dirp, struct dirent **ent

[PATCH 06/12] util: change virDirClose to take a DIR* instead of DIR**.

. (Note that virDirClose() will still check for NULL before attempting to close, so that it can always be safely called, as long as the DIR* was initialized to NULL (another prerequisite of becoming a g_autoptr cleanup function) Signed-off-by: Laine Stump --- src/util/virfile.c | 7 +++ src/util

[PATCH 10/12] util: remove unused VIR_DIR_CLOSE() macro

() previously had an extra & on the pointer so that it could be transparently passed as a DIR** to virDirClose(), but that was removed several commits back.) Signed-off-by: Laine Stump --- src/util/virfile.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/util/virfile.h b/src/util/virfile.h i

[PATCH 05/12] util: manually set dirp to NULL after closing in virCapabilitiesInitCache()

. Then in an upcoming patch we can safely remove the side effect from VIR_DIR_CLOSE(). This extra/ugly bit of code is only temporary: once we introduce the g_autoptr cleanup function for DIR*, we will remove this manual close/clear completely anyway. Signed-off-by: Laine Stump --- src/conf/capabilities.c | 1

[PATCH 08/12] change DIR* int g_autoptr(DIR) where appropriate

All of these conversions are trivial - VIR_DIR_CLOSE() (aka virDirClose()) is called only once on the DIR*, and it happens just before going out of scope. Signed-off-by: Laine Stump --- src/bhyve/bhyve_capabilities.c | 3 +-- src/conf/virdomainobjlist.c | 3 +-- src/conf

[PATCH 09/12] conf: convert final DIR* to g_autoptr

This use of DIR* was re-using the same function-scope DIR* each time through a for loop, and due to multiple error gotos in the loop, it needed to have the scope of the DIR* reduced to just the loop at the same time as switching to g_autoptr. That's what this patch does. Signed-off-by: Laine

[PATCH 03/12] storage: remove extraneous call to VIR_DIR_CLOSE()

the conversion to g_autoptr trivial/mechanical. Signed-off-by: Laine Stump --- src/storage/storage_util.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 21ad54ac54..7eaf899883 100644 --- a/src/storage/storage_util.c +++ b/src/storage

[PATCH 01/12] consistently use VIR_DIR_CLOSE() instead of virDirClose()

This will make it easier to review upcoming patches that use g_autoptr to auto-close all DIRs. Signed-off-by: Laine Stump --- src/qemu/qemu_interop_config.c | 2 +- src/security/security_selinux.c | 6 ++ src/util/virdevmapper.c | 2 +- src/util/virfile.c | 3

[PATCH 02/12] tools: reduce scope of a DIR* in virHostValidateIOMMU()

This will make the trivial nature of a conversion to g_autoptr (in a later patch) more obvious. Signed-off-by: Laine Stump --- tools/virt-host-validate-common.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate

[PATCH 04/12] util: reduce scope of a DIR * in virCgroupV1SetOwner()

in a subsequent patch) NB: VIR_DIR_CLOSE() is already called at the bottom of the for loop, so removing the VIR_DIR_CLOSE() at the end of the function is *not* creating a leak of a DIR*! Signed-off-by: Laine Stump --- src/util/vircgroupv1.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git

[PATCH 00/12] use g_autoptr for all DIR*

idea), but decided that the DIR*'s really did need to autoclose. In the end, all uses of DIR* could be easily converted to use g_autoptr. Laine Stump (12): consistently use VIR_DIR_CLOSE() instead of virDirClose() tools: reduce scope of a DIR* in virHostValidateIOMMU() storage: remove

[PATCH] re-add cscope db files to .gitignore

Commit f7114e61db removed these lines as being "old and obsolete". cscope may be old, but it is still very much useful and used (i.e. *not* obsolete), and having the files show up as "Untracked files:" every time git status is run is annoying. Signed-off-by: Laine Stump ---

Re: [libvirt PATCH v5 6/6] Include vdpa devices in node device list

On 10/26/20 6:53 AM, John Ferlan wrote: On 10/14/20 1:08 PM, Jonathon Jongsma wrote: The current udev node device driver ignores all events related to vdpa devices. Since libvirt now supports vDPA network devices, include these devices in the device list. Example output: virsh # nodedev-list

Re: [libvirt PATCH v5 6/6] Include vdpa devices in node device list

have a normal parent device. Signed-off-by: Jonathon Jongsma Reviewed-by: Laine Stump (I had left this patch in limbo in case anyone had issues with the particular element names, and then forgot about it for the last week. Seeing that nobody had an issue, I'm pushing it so it gets

Re: [libvirt PATCH 2/2] qemu: combine conditionals

rn -1; In this case, I've added braces rather than combining the lines, since each is a separate subexpression, rather than just pieces of a single simple expression, as was the case in the previous patch. Reviewed-by: Laine Stump and pushed. i

Re: [libvirt PATCH 1/2] qemu: fix memory leak reported by coverity

increases the line length to 86 (based on the sentiments expressed during a recent discussion on that topic on the mailing list), I'm going to do that instead of adding the curly braces. Reviewed-by: Laine Stump and pushed. if (qemuMonitorJSONCommandWithFd(mon, cmd, fd, ) < 0)

Re: [libvirt PATCH] qemu: fix potential resource leak

On 10/23/20 1:28 AM, Peter Krempa wrote: On Thu, Oct 22, 2020 at 23:20:20 -0400, Laine Stump wrote: On 10/22/20 3:01 AM, Peter Krempa wrote: On Wed, Oct 21, 2020 at 22:31:09 -0400, Laine Stump wrote: On 10/21/20 5:50 PM, Jonathon Jongsma wrote: Coverity reported a potential resource leak

Re: [libvirt PATCH] qemu: fix potential resource leak

On 10/22/20 3:01 AM, Peter Krempa wrote: On Wed, Oct 21, 2020 at 22:31:09 -0400, Laine Stump wrote: On 10/21/20 5:50 PM, Jonathon Jongsma wrote: Coverity reported a potential resource leak. While it's probably not a real-world scenario, the code could technically jump to cleanup between

Re: [libvirt PATCH v5 4/6] qemu: add monitor functions for handling file descriptors

On 10/21/20 4:54 PM, John Ferlan wrote: On 10/14/20 1:08 PM, Jonathon Jongsma wrote: add-fd, remove-fd, and query-fdsets provide functionality that can be used for passing fds to qemu and closing fdsets that are no longer necessary. Signed-off-by: Jonathon Jongsma ---

Re: [libvirt PATCH] qemu: fix potential resource leak

leClose() is a NOP if fd < 0, so this doesn't need the conditional. I *was* going to say "With that change, Reviewed-by: Laine Stump " but this got me looking at the code of the entire function rather than just the diffs in the patch, and I've got a question - is there any reas

Re: [libvirt PATCH] rpm: disable LTO in Fedora 34

cause and whether binutils or libvirt will need changing. https://bugzilla.redhat.com/show_bug.cgi?id=1889763 Signed-off-by: Daniel P. Berrangé post-partem Reviewed-by: Laine Stump (I had been waiting for my CI to complete too :-) --- libvirt.spec.in | 7 +++ 1 file changed, 7

[PATCH] util: fix very old bug/typo in virNetDevParseVfInfo()

(but are doing the actual retrieval of the attribute from the correct place), this bug has no real consequences other than confusing anyone trying to understand the code. Signed-off-by: Laine Stump --- src/util/virnetdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util

[PATCH 6/9] util: use more g_autofree in virpci.c

Signed-off-by: Laine Stump --- src/util/virpci.c | 27 +-- 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/src/util/virpci.c b/src/util/virpci.c index 3ca513702e..c7ee259e29 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -1245,7 +1245,7 @@ static

[PATCH 5/9] util: avoid manual VIR_FREE of a g_autofree pointer in virPCIGetName()

thisPhysPortID is only used inside a conditional, so reduce its scope to just the body of that conditional, which will eliminate the need for the undesirable manual VIR_FREE(). Signed-off-by: Laine Stump --- src/util/virpci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git

[PATCH 4/9] util: simplify virPCIProbeStubDriver()

that might possibly be executed twice, thus eliminating the ugly use of goto to construct a loop, and also takes advantage of the fact that virPCIDriverDir() was previously returning *exactly* the same string both times it was called to eliminate the manual VIR_FREE of drvpath. Signed-off-by: Laine Stump

[PATCH 3/9] util: simplify virPCIDriverDir() and its callers

There is no need for a temporary variable in this function, and since it can't return NULL, no need for callers to check for it. Signed-off-by: Laine Stump --- src/util/virpci.c | 13 + 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/util/virpci.c b/src/util

[PATCH 0/9] several small cleanups in virpci.c et. al.

These started with a couple small cleanups to functions I was going to change the API for, but then I decided not to change the API, and just didn't want to waste the effort spent cleaning them up. Laine Stump (9): util: simplify virHostdevPCISysfsPath() util: simplify virPCIFile() and its

<    2   3   4   5   6   7   8   9   10   11   >