On Fri, Apr 19, 2024 at 02:23 PM -0500, Jonathon Jongsma
wrote:
> On 4/19/24 9:49 AM, Marc Hartmayer wrote:
>> It's better practice for all functions called by the threads to pass the
>> driver
>> via parameter and not global variables. Easier to test and cleaner.
>>
[…snip…]
>>
>>
>>
On Tue, Apr 23, 2024 at 10:03:35AM +0200, Marc Hartmayer wrote:
> On Fri, Apr 19, 2024 at 02:23 PM -0500, Jonathon Jongsma
> wrote:
> > On 4/19/24 9:49 AM, Marc Hartmayer wrote:
> >> It's better practice for all functions called by the threads to pass the
> >> driver
> >> via parameter and not g
On Mon, Apr 22, 2024 at 05:43 PM +0200, Boris Fiuczynski
wrote:
> This could be quashed with patch 3 but I am also fine with this if you
> do not want to spend the effort.
>
> Reviewed-by: Boris Fiuczynski
Will squash it.
[…snip]
--
Kind regards / Beste Grüße
Marc Hartmayer
IBM Deutschl
On Tue, Apr 23, 2024 at 09:10 AM +0100, Daniel P. Berrangé
wrote:
> On Tue, Apr 23, 2024 at 10:03:35AM +0200, Marc Hartmayer wrote:
>> On Fri, Apr 19, 2024 at 02:23 PM -0500, Jonathon Jongsma
>> wrote:
>> > On 4/19/24 9:49 AM, Marc Hartmayer wrote:
>> >> It's better practice for all functions c
On Tue, Apr 23, 2024 at 10:46:14AM +0200, Marc Hartmayer wrote:
> On Tue, Apr 23, 2024 at 09:10 AM +0100, Daniel P. Berrangé
> wrote:
> > On Tue, Apr 23, 2024 at 10:03:35AM +0200, Marc Hartmayer wrote:
> >> On Fri, Apr 19, 2024 at 02:23 PM -0500, Jonathon Jongsma
> >> wrote:
> >> > On 4/19/24 9
On Tue, Apr 23, 2024 at 10:06 AM +0100, Daniel P. Berrangé
wrote:
> On Tue, Apr 23, 2024 at 10:46:14AM +0200, Marc Hartmayer wrote:
>> On Tue, Apr 23, 2024 at 09:10 AM +0100, Daniel P. Berrangé
>> wrote:
>> > On Tue, Apr 23, 2024 at 10:03:35AM +0200, Marc Hartmayer wrote:
>> >> On Fri, Apr 19,
On Sun, Apr 21, 2024 at 10:53:09PM -0400, Laine Stump wrote:
> These functions are only ever used by the network driver, and are so
> specific to the network driver's usage of iptables that they likely
> won't ever be used elsewhere. The files are renamed to
> network_iptables.[ch] to be more in li
On Sun, Apr 21, 2024 at 10:53:11PM -0400, Laine Stump wrote:
> Now that the toplevel iptables functions have been moved out of the
> linux bridge driver into network_iptables.c, all of the utility
> functions are used only within that same file, so simplify it.
>
> Signed-off-by: Laine Stump
> --
On Sun, Apr 21, 2024 at 10:53:10PM -0400, Laine Stump wrote:
> Although initially we will add exactly the same rules for the nftables
> backend, the two may (hopefully) soon diverge as we take advantage of
> nftables features that weren't available in iptables. When we do that,
> there will need to
On Sun, Apr 21, 2024 at 10:53:12PM -0400, Laine Stump wrote:
> Signed-off-by: Laine Stump
> ---
> src/network/network_iptables.c | 51 +++---
> 1 file changed, 29 insertions(+), 22 deletions(-)
Reviewed-by: Daniel P. Berrangé
With regards,
Daniel
--
|: https://ber
On Sun, Apr 21, 2024 at 10:53:14PM -0400, Laine Stump wrote:
> I had originally named these as VIR_NETFILTER_* because I assumed the
> same enum would eventually be used by our nftables backend as well as
> iptables. But it turns out that in most cases it's not possible to
> delete an nftables rule
On Sun, Apr 21, 2024 at 10:53:15PM -0400, Laine Stump wrote:
> In normal practice a virFirewallCmd should never have 0 args by the
> time it gets to the Apply stage, but at some time while debugging one
> of the other patches in this series, exactly that happened (due to a
> bug that was since squa
On Sun, Apr 21, 2024 at 10:53:17PM -0400, Laine Stump wrote:
> We know at the time a virFirewallCmd is created (with
> virFirewallAddCmd*()) whether or not we will later want to ignore
> errors encountered when attempting to apply that command - if
> ignoreErrors is set in the AddCmd or if the grou
On Sun, Apr 21, 2024 at 10:53:18PM -0400, Laine Stump wrote:
> (This paragraph is for historical reference only, described only to
> avoid confusion of past use of the name with its new use) In a past
> life, virFirewallBackend had been a private static in virfirewall.c
> that was set at daemon ini
On Sun, Apr 21, 2024 at 10:53:19PM -0400, Laine Stump wrote:
> Signed-off-by: Laine Stump
> ---
> libvirt.spec.in | 3 ++
> src/network/libvirtd_network.aug | 36
> src/network/meson.build | 11
> src/network/net
On Sun, Apr 21, 2024 at 10:53:20PM -0400, Laine Stump wrote:
> It still can have only one useful value ("iptables"), but once a 2nd
> value is supported, it will be selectable by setting
> "firewall_backend=nftables" in /etc/libvirt/network.conf.
>
> If firewall_backend isn't set in network.conf,
On Sun, Apr 21, 2024 at 10:53:21PM -0400, Laine Stump wrote:
> Modify networkSetupPrivateChains() in the network driver to accept a
> firewallBackend argument so it will know which backend to call. (right
> now it always calls the iptables version of the lower level function,
> but in the future it
On Sun, Apr 21, 2024 at 10:53:30PM -0400, Laine Stump wrote:
> This was the only reason we required the iptables and ebtables
> packages at build time, and many other external commands already have
> their binaries found at runtime by looking through $PATH (virCommand
> automatically does this), so
On Sun, Apr 21, 2024 at 10:53:31PM -0400, Laine Stump wrote:
> The only reason for requiring these was so that meson could search for
> the binary location, and the previous patch eliminated that, so we no
> longer need them at build time.
>
> Signed-off-by: Laine Stump
> ---
> libvirt.spec.in |
On Sun, Apr 21, 2024 at 10:53:34PM -0400, Laine Stump wrote:
> The initial patches to support nftables for virtual networks left
> iptables as the default backend.
>
> The only functional difference between the two backends is that the
> nftables backend doesn't add any rules to fix up the checksu
On Sun, Apr 21, 2024 at 10:53:35PM -0400, Laine Stump wrote:
> We really shouldn't be requiring ebtables and iptables any more, since
> they don't always need to be used. Likewise, we probably should at
> least Recommend nftables, even though it's pretty much always
> installed already anyway.
>
>
On Sun, Apr 21, 2024 at 10:53:13PM -0400, Laine Stump wrote:
> These objects aren't rules, they are commands that are executed that
> may create a firewall rule, delete a firewall rule, or simply list the
> existing firewall rules. It's confusing for the objects to be called
> "Rule" (especially in
On Sun, Apr 21, 2024 at 10:53:16PM -0400, Laine Stump wrote:
> We will already need a separate function for virFirewallApplyCmd for
> iptables vs. nftables, but the only reason for needing a separate
> function for virFirewallAddCmd* is that iptables/ebtables need to have
> an extra arg added for l
On Sun, Apr 21, 2024 at 10:53:23PM -0400, Laine Stump wrote:
> If the VIR_FIREWALL_TRANSACTION_AUTO_ROLLBACK flag is set, each time
> an iptables command is executed that is adding a rule or chain, a
> corresponding command that will *delete* the same rule/chain is
> constructed and added to the li
On Sun, Apr 21, 2024 at 10:53:24PM -0400, Laine Stump wrote:
> So far this will only affect what happens if there is some failure
> while applying the firewall rules; the rollback rules aren't yet
> persistent beyond that time. More work is needed to remember the
> rollback rules while the network
On Sun, Apr 21, 2024 at 10:53:26PM -0400, Laine Stump wrote:
> These functions convert a virFirewall object to/from XML so that it
> can be serialized to disk (in a virNetworkObj's status file) and
> restored later (e.g. after libvirtd/virtnetworkd is restarted).
>
> Signed-off-by: Laine Stump
>
On Sun, Apr 21, 2024 at 10:53:27PM -0400, Laine Stump wrote:
> This virFirewall object will store the list of actions required to
> remove the firewall that was added for the currently active instance
> of the network, so it has been named "fwRemoval".
>
> There are no uses of the fwRemoval object
On Sun, Apr 21, 2024 at 10:53:28PM -0400, Laine Stump wrote:
> When destroying a network, the network driver has always assumed that
> it knew what firewall rules had been added as the network was
> started. This was usually correct - I only recall one time in the past
> that the firewall rules add
On Sun, Apr 21, 2024 at 10:53:32PM -0400, Laine Stump wrote:
> Support using nftables to setup the firewall for each virtual network,
> rather than iptables. The initial implementation of the nftables
> backend creates (almost) exactly the same ruleset as the iptables
> backend, determined by runni
On 4/19/24 8:38 AM, Boris Fiuczynski wrote:
> On 4/17/24 17:17, Cole Robinson wrote:
>> Commit v10.0.0-265-ge67bca23e4 added a `active_config` and
>> `defined_config` to nodedev mdev internal XML handling.
>> `defined_config` can be filled at XML parse time, but `active_config`
>> must be filled in
This was the implied default before nodedevs gained a notion of
being inactive and transient. It also matches the implied default
when parsing other object types
Signed-off-by: Cole Robinson
---
src/test/test_driver.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/test/test_driver.c b
- Error if INACTIVE requested for transient object
- Force dumping INACTIVE XML when object is inactive
Signed-off-by: Cole Robinson
---
src/test/test_driver.c | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
in
See last patch for explanation. First two patches are related
bugfixes/improvements
v5:
- Changed impl to match Boris' suggestion
Cole Robinson (3):
test: make parsed nodedevs active and persistent
test: Sync GetXML INACTIVE behavior with live driver
test: nodedev: fill active_config at d
Commit v10.0.0-265-ge67bca23e4 added a `active_config` and
`defined_config` to nodedev mdev internal XML handling.
`defined_config` can be filled at XML parse time, but `active_config`
must be filled in by nodedev driver. This wasn't implemented for the
test driver however, which caused virt-manage
Some sysfs files contain either string representation of a bitmap
or just a newline character. An example of such file is:
/sys/devices/system/cpu/isolated. Our current implementation of
virFileReadValueBitmap() fails in the latter case, unfortunately.
Introduce a slightly modified version that acc
Some sysfs files contain either string representation of a bitmap
or just a newline character. An example of such file is:
/sys/devices/system/cpu/isolated. Our current implementation of
virBitmapParseUnlimited() fails in the latter case,
unfortunately. Introduce a slightly modified version that ac
This is a helper that parses /sys/devices/system/cpu/isolated
into a virBitmap. It's going to be needed soon.
Signed-off-by: Michal Privoznik
---
src/libvirt_private.syms | 1 +
src/util/virhostcpu.c| 31 +++
src/util/virhostcpu.h| 1 +
3 files changed, 33 i
v2 of:
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/4V7OI5AEGYRN4GFQMQPIN4MYPJNK3NYJ/
diff to v1:
- Don't error out on systems where /sys/devices/system/cpu/isolated is
unavailable.
- Don't error out on systems where /sys/devices/system/cpu/isolated is
empty.
Both o
When starting a domain and there's no vCPU/emulator pinning set,
we query the list of all online physical CPUs and set affinity of
the child process (which eventually becomes QEMU) to that list.
We can't assume libvirtd itself had affinity to all online CPUs
and since affinity of the child process
On 4/23/24 6:10 AM, Daniel P. Berrangé wrote:
On Sun, Apr 21, 2024 at 10:53:18PM -0400, Laine Stump wrote:
(This paragraph is for historical reference only, described only to
avoid confusion of past use of the name with its new use) In a past
life, virFirewallBackend had been a private static in
On 4/23/24 6:17 AM, Daniel P. Berrangé wrote:
On Sun, Apr 21, 2024 at 10:53:20PM -0400, Laine Stump wrote:
It still can have only one useful value ("iptables"), but once a 2nd
value is supported, it will be selectable by setting
"firewall_backend=nftables" in /etc/libvirt/network.conf.
If firew
On 4/23/24 6:21 AM, Daniel P. Berrangé wrote:
On Sun, Apr 21, 2024 at 10:53:21PM -0400, Laine Stump wrote:
>> [...]
+static int
+networkFirewallSetupPrivateChains(virFirewallBackend backend,
+ virFirewallLayer layer)
+{
+switch (backend) {
+case VIR_FIRE
On 4/23/24 6:40 AM, Daniel P. Berrangé wrote:
I wonder if we shouldn't make the default firewall backend be
a meson_options.txt parameter.
Good idea!
If a distro rebases libvirt in their existing release, they
probably don't want the firewall backend silently changing
as a side effect. A me
On Tue, Apr 23, 2024 at 11:48:24AM -0400, Laine Stump wrote:
> On 4/23/24 6:17 AM, Daniel P. Berrangé wrote:
> > On Sun, Apr 21, 2024 at 10:53:20PM -0400, Laine Stump wrote:
> > > It still can have only one useful value ("iptables"), but once a 2nd
> > > value is supported, it will be selectable by
On 4/23/24 6:46 AM, Daniel P. Berrangé wrote:
On Sun, Apr 21, 2024 at 10:53:35PM -0400, Laine Stump wrote:
We really shouldn't be requiring ebtables and iptables any more, since
they don't always need to be used. Likewise, we probably should at
least Recommend nftables, even though it's pretty m
On 4/23/24 5:52 AM, Daniel P. Berrangé wrote:
On Sun, Apr 21, 2024 at 10:53:09PM -0400, Laine Stump wrote:
These functions are only ever used by the network driver, and are so
specific to the network driver's usage of iptables that they likely
won't ever be used elsewhere. The files are renamed
On 4/23/24 6:53 AM, Daniel P. Berrangé wrote:
On Sun, Apr 21, 2024 at 10:53:24PM -0400, Laine Stump wrote:
diff --git a/tests/networkxml2firewalltest.c b/tests/networkxml2firewalltest.c
index 3a9f409e2a..e61787daec 100644
--- a/tests/networkxml2firewalltest.c
+++ b/tests/networkxml2firewalltest
On 4/23/24 6:59 AM, Daniel P. Berrangé wrote:
On Sun, Apr 21, 2024 at 10:53:26PM -0400, Laine Stump wrote:
+ */
+int
+virFirewallParseXML(virFirewall **firewall,
+xmlNodePtr node,
+xmlXPathContextPtr ctxt)
+{
[...]
+nargs = virXPathNodeSet("
On 4/23/24 7:15 AM, Daniel P. Berrangé wrote:
On Sun, Apr 21, 2024 at 10:53:32PM -0400, Laine Stump wrote:
Support using nftables to setup the firewall for each virtual network,
rather than iptables. The initial implementation of the nftables
backend creates (almost) exactly the same ruleset as
On Tue, Apr 23, 2024 at 01:27:05PM -0400, Laine Stump wrote:
> On 4/23/24 7:15 AM, Daniel P. Berrangé wrote:
> > On Sun, Apr 21, 2024 at 10:53:32PM -0400, Laine Stump wrote:
> > > Support using nftables to setup the firewall for each virtual network,
> > > rather than iptables. The initial implemen
From: Boris Fiuczynski
Two situations will trigger an udev add event:
1) the mdev is created when started (transient) or
2) the mdev was defined and is started
In case 1 there is no node object existing and no config data is copied.
In case 2 copying the active config data of an existing node o
When an udev event occurs for a mediated device (mdev) the mdev config data
requires an update via mdevctl as the udev event does not contain all config
data. This update needs to occur immediately and to be finished before the
libvirt nodedev event is issued to keep the API usage reliable.
Change
Remove the timeout when the udevEventData is disposed, analogous to priv->watch.
Reviewed-by: Boris Fiuczynski
Reviewed-by: Jonathon Jongsma
Signed-off-by: Marc Hartmayer
---
src/node_device/node_device_udev.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/node_device/node_device_u
Commit a99d876a0f58 ("node_device: Use automatic mutex management") replaced the
locking mechanism and accidentally removed the comment with the reason why the
lock is taken. The reason was to "ensure only a single thread can query mdevctl
at a time", but this reason is no longer valid or maybe it
@def is owned by @obj after adding it the node device object list. As soon as
the @obj lock has been released, another thread could free @obj and therefore
@def. If now someone accesses @def this would lead to a heap-use-after-free and
therefore most likely to a segmentation fault, therefore set @d
It is done a little differently everywhere in libvirt, but most common is to
test for != -1.
Reviewed-by: Boris Fiuczynski
Reviewed-by: Jonathon Jongsma
Signed-off-by: Marc Hartmayer
---
src/node_device/node_device_udev.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git
From: Boris Fiuczynski
When a mdev device is destroyed or stopped the udev remove event
handling needs to reset the active config data of the node object
representing a persisted mdev.
Reviewed-by: Marc Hartmayer
Reviewed-by: Jonathon Jongsma
Signed-off-by: Boris Fiuczynski
---
src/node_devi
Since @driver->privateData is modified take the lock.
Reviewed-by: Boris Fiuczynski
Reviewed-by: Jonathon Jongsma
Signed-off-by: Marc Hartmayer
---
src/node_device/node_device_udev.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/node_device/node_device_udev.c
b/sr
From: Boris Fiuczynski
When an udev add, change or remove event occurs the mdev active config data
requires an update via mdevctl as the udev does not contain all config data.
This update needs to occur immediately and to be finished before the libvirt
nodedev event is issued to keep the API usag
Everything is released in `udevEventDataDispose` except for the threads, change
this as this makes the code easier to read as it can be simplified a little.
Reviewed-by: Jonathon Jongsma
Reviewed-by: Boris Fiuczynski
Signed-off-by: Marc Hartmayer
---
src/node_device/node_device_udev.c | 14 +++
Inline `udevRemoveOneDevice` as it's used only once.
Reviewed-by: Boris Fiuczynski
Reviewed-by: Jonathon Jongsma
Signed-off-by: Marc Hartmayer
---
src/node_device/node_device_udev.c | 17 +
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/src/node_device/node_devi
The documentation of gobject signals reads:
"If you are connecting handlers to signals and using a GObject instance as your
signal handler user data, you should remember to pair calls to
g_signal_connect() with calls to g_signal_handler_disconnect() or
g_signal_handlers_disconnect_by_func(). While
Introduce and use the driver functions for the node state shutdown preparation
and wait. As they're also called in the error/cleanup path of
`nodeStateInitialize`, they must be written in a way, that they can safely be
executed even if not everything is initialized.
In the next commit, these funct
Even if `priv->udev_monitor` was never initialized, the mdevctlLock, udevThread
were. Therefore let's match the order of releasing the resources the order of
allocating the resources in `nodeStateInitialize`.
In addition, use `g_steal_pointer` in `g_list_free_full`.
Reviewed-by: Jonathon Jongsma
The new names make it easier to understand the purpose of the data.
Reviewed-by: Boris Fiuczynski
Reviewed-by: Jonathon Jongsma
Signed-off-by: Marc Hartmayer
---
src/node_device/node_device_udev.c | 48 +++---
1 file changed, 24 insertions(+), 24 deletions(-)
diff --gi
Signed-off-by: Marc Hartmayer
---
src/node_device/node_device_udev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index 14d44d5bcd0e..cc725997a39e 100644
--- a/src/node_device/node_device_udev.c
+++ b/
It's better practice for all functions called by the threads to pass the driver
via parameter and not global variables. Easier to test and cleaner.
Reviewed-by: Jonathon Jongsma
Reviewed-by: Boris Fiuczynski
Signed-off-by: Marc Hartmayer
---
src/node_device/node_device_driver.h | 2 +-
src/no
Use this feature in `udevEventDataNew`.
Reviewed-by: Jonathon Jongsma
Reviewed-by: Boris Fiuczynski
Signed-off-by: Marc Hartmayer
---
src/node_device/node_device_udev.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/src/node_device/node_device_udev.c
b/src/n
There is only one case where force is true, therefore let's inline that case.
Reviewed-by: Jonathon Jongsma
Reviewed-by: Boris Fiuczynski
Signed-off-by: Marc Hartmayer
---
src/node_device/node_device_udev.c | 25 +++--
1 file changed, 11 insertions(+), 14 deletions(-)
diff
Use a worker pool for processing the events (e.g. udev, mdevctl config changes)
and the initialization instead of a separate initThread and a mdevctl-thread.
This has the large advantage that we can leverage the job API and now this
thread pool is responsible to do all the "costly-work" and emittin
Instead of accessing the global `driver` object pass the `udevEventData` as
parameter to the thread handler and watch callback. This has the advantage that:
1. proper refcounting
2. easier to read and test
Reviewed-by: Jonathon Jongsma
Reviewed-by: Boris Fiuczynski
Signed-off-by: Marc Hartmayer
On 4/23/24 1:42 PM, Daniel P. Berrangé wrote:
On Tue, Apr 23, 2024 at 01:27:05PM -0400, Laine Stump wrote:
[...]
On 4/23/24 7:15 AM, Daniel P. Berrangé wrote:
What are the uniqueness guarantees of handle numbers.
Each table has a monotonically increasing counter (I'd assume at least 32
bi
Hi,
Does anyone feel strongly against dropping the "micro" part from
libvirt(-python) versions? I think the original idea was to use this
number for maintenance releases in -maint branches, but we stopped doing
those a long time ago (v3.2.1 was the last and most likely even the only
release with m
On Wed, Apr 24, 2024 at 08:43:00 +0200, Jiri Denemark wrote:
> Hi,
>
> Does anyone feel strongly against dropping the "micro" part from
> libvirt(-python) versions? I think the original idea was to use this
> number for maintenance releases in -maint branches, but we stopped doing
> those a long t
74 matches
Mail list logo
