Re: [libvirt PATCH 05/10] conf: parse/format
On 2/18/20 12:39 PM, Ján Tomko wrote: On Sun, Feb 16, 2020 at 11:22:54PM -0500, Laine Stump wrote: This is a very simple thing to parse and format, but needs to be done in 4 places, so two trivial utility functions have been made that can be called from all the higher level parser/formatters: (only in domain status) Signed-off-by: Laine Stump --- docs/schemas/domaincommon.rng | 3 + docs/schemas/network.rng | 3 + docs/schemas/networkcommon.rng | 11 docs/schemas/networkport.rng | 3 + src/conf/domain_conf.c | 19 ++ src/conf/domain_conf.h | 4 ++ src/conf/network_conf.c | 32 ++ src/conf/network_conf.h | 9 +++ src/conf/virnetworkportdef.c | 3 + src/conf/virnetworkportdef.h | 1 + src/libvirt_private.syms | 1 + tests/networkxml2xmlin/isolated-ports.xml | 7 +++ tests/networkxml2xmlout/isolated-ports.xml | 7 +++ tests/networkxml2xmltest.c | 1 + tests/qemuxml2argvdata/net-isolated-port.xml | 34 ++ .../net-isolated-port.x86_64-latest.xml | 63 +++ tests/qemuxml2xmltest.c | 1 + 17 files changed, 202 insertions(+) create mode 100644 tests/networkxml2xmlin/isolated-ports.xml create mode 100644 tests/networkxml2xmlout/isolated-ports.xml create mode 100644 tests/qemuxml2argvdata/net-isolated-port.xml create mode 100644 tests/qemuxml2xmloutdata/net-isolated-port.x86_64-latest.xml Not a fan of multi-word elements, because they bring up our inconsistency in using camelCase vs snake_case. Yeah, it always bothers me when I see a multiword element or attribute for that reason. I always use camelCase because I remember asking about which is preferred > 10 years ago and being told that we wanted to have camelCase in libvirt XML. That could even be a false memory, but it has always stuck with me. But I assume you chose the name to make it compatible with all four containing elements. Would something like: look too odd? u God I *HATE* coming up with element and attribute names! (That's the only response I can think of right now since it's late in the day. Let me sleep on it, but in the end I was expecting, even *hoping* someone would object to portOptions and propose an alternative, and yours doesn't really sound any *worse* than mine, so it might be worthwhile to use it just so I wouldn't have to shoulder the blame :-) Code-wise: Reviewed-by: Ján Tomko Jano
Re: [libvirt PATCH 07/10] qemu/lxc: plumb isolatedPort from config down through bridge attachment
On 2/18/20 12:46 PM, Ján Tomko wrote:
On Sun, Feb 16, 2020 at 11:22:56PM -0500, Laine Stump wrote:
This patch pushes the isolatedPort setting from the down
all the way to the callers of virNetDevBridgeAddPort(), and sets
BR_ISOLATED on the port (using virNetDevBridgePortSetIsolated()) after
the port has been successfully added to the bridge.
Signed-off-by: Laine Stump
---
src/bhyve/bhyve_command.c | 1 +
src/conf/domain_conf.c | 1 +
src/lxc/lxc_process.c | 10 ++
src/network/bridge_driver.c | 1 +
src/qemu/qemu_hotplug.c | 16
src/qemu/qemu_interface.c | 1 +
src/util/virnetdevtap.c | 17 -
src/util/virnetdevtap.h | 3 +++
tests/bhyvexml2argvmock.c | 1 +
9 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 6395826c69..af892255c7 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3350,12 +3350,28 @@ qemuDomainChangeNetBridge(virDomainObjPtr vm,
}
ret = virNetDevBridgeAddPort(newbridge, olddev->ifname);
+ if (ret == 0 &&
+ virDomainNetGetActualPortOptionsIsolated(newdev) ==
VIR_TRISTATE_BOOL_YES) {
+
+ ret = virNetDevBridgePortSetIsolated(newbridge,
olddev->ifname, true);
+ if (ret < 0) {
+ virErrorPtr err;
+
+ virErrorPreserveLast();
+ ignore_value(virNetDevBridgeRemovePort(newbridge,
olddev->ifname));
+ virErrorRestore();
+ }
+ }
virDomainAuditNet(vm, NULL, newdev, "attach", ret == 0);
if (ret < 0) {
virErrorPtr err;
virErrorPreserveLast();
ret = virNetDevBridgeAddPort(oldbridge, olddev->ifname);
+ if (ret == 0 &&
+ virDomainNetGetActualPortOptionsIsolated(olddev) ==
VIR_TRISTATE_BOOL_YES) {
+ ignore_value(virNetDevBridgePortSetIsolated(newbridge,
olddev->ifname, true));
Should this use 'oldbridge' instead of 'newbridge'?
Whoops! Cut/paste error. (At least I removed the part about being a Navy
Seal and having a certain set of skills)
+ }
virDomainAuditNet(vm, NULL, olddev, "attach", ret == 0);
virErrorRestore();
return -1;
Reviewed-by: Ján Tomko
Jano
Re: [libvirt PATCH 5/6] Make PATHs unique for a VM object instance
On 2/11/20, 7:06 PM, "Daniel P. Berrangé" wrote: On Tue, Feb 11, 2020 at 10:05:53AM +0100, Martin Kletzander wrote: > > On Wed, Feb 05, 2020 at 05:32:50PM +, Daniel P. Berrangé wrote: > > > On Mon, Feb 03, 2020 at 12:43:32PM +, Daniel P. Berrangé wrote: > > > > From: Shaju Abraham > > > > > > > There are various config paths that a VM uses. The monitor paths and > > > > other lib paths are examples. These paths are tied to the VM name or > > > > UUID. The local migration breaks the assumption that there will be only > > > > one VM with a unique UUID and name. During local migrations there can be > > > > multiple VMs with same name and UUID in the same host. Append the > > > > domain-id field to the path so that there is no duplication of path > > > names. > > > > > >This is the really critical problem with localhost migration. > > > > > >Appending the domain-id looks "simple" but this is a significant > > >behavioural / functional change for applications, and I don't think > > >it can fully solve the problem either. > > > > > >This is changing thue paths used in various places where libvirt > > >internally generates unique paths (eg QMP socket, huge page or > > >file based memory paths, and defaults used for auto-filling > > >device paths ( if not specified). > > > > > >Some of these paths are functionally important to external > > >applications and cannot be changed in this way. eg stuff > > >integrating with QEMU can be expecting certain memory backing > > >file paths, or certain paths & is liable to break > > >if we change the naming convention. > > > > > >For sake of argument, lets assume we can changing the naming > > >convention without breaking anything... > > > > > > >This was already done in (I would say) most places as they use > >virDomainDefGetShortName() to get a short, unique name of a directory -- it uses > >the domain ID as a prefix. > > > > > This only applies to paths libvirt generates at VM startup. > > > > > >There are plenty of configuration elements in the guest XML > > >that are end user / mgmt app defined, and reference paths in > > >the host OS. > > > > > >For example , , , , > > >all support UNIX domain sockets and TCP sockets. A UNIX > > >domain socket cannot be listened on by multiple VMs > > >at once. If the UNIX socket is in client mode, we cannot > > >assume the thing QEMU is connecting to allows multiple > > >concurrent connections. eg 2 QEMU's could have their > > > connected together over a UNIX socket pair. > > >Similarly if automatic TCP port assignment is not used > > >we cannot have multiple QEMU's listening on the same > > >host. > > > > > >One answer is to say that localhost migration is just > > >not supported for such VMs, but I don't find that very > > >convincing because the UNIX domain socket configs > > >affected are in common use. > > > > > > >I would be okay with saying that these either need to be changed in a provided > >destination XML or the migration will probably break. I do not think it is > >unreasonable to say that if users are trying to shoot themselves, we should not > >spend a ridiculous time just so we can prevent that. Otherwise we will get to > >the same point as we are now -- there might be a case where a local migration > >would work, but users cannot execute it even if they were very cautious and went > >through all the things that can prevent it from the technical point of view, > >libvirt will still disallow that. >If there are clashing resources, we can't rely on QEMU reporting an >error. For example with a UNIX domain socket, the first thing QEMU >does is unlink(/socket/path), which will blow away the UNIX domain >socket belonging to the original QEMU. As a result if migration >fails, and we rollback, the original QEMU will be broken. By appending the id field to the path, we are effectively nullifying this particular concern. Each qemu instance will get its own unique path and monitor. If a migration fails, we can roll back. I understand the need to keep the paths unchanged for externally generated configurations. How about the below approach? Symlinks are created so that until the migration is complete the path remains unchanged for the source . I am still having issues with the emails, so pasting the code inline. >From 37ead09491f1e4ae09f8bba1204ae4626390fb2d Mon Sep 17 00:00:00 2001 From: Shaju Abraham Date: Wed, 22 Jan 2020 02:06:03 -0800 Subject: [PATCH 5/6] Make PATHs unique for a VM object instance There are various config paths that a VM uses. The monitor paths and other lib paths are examples. These paths are tied to the VM name or UUID. The local migration breaks the
Re: virtlogd spinning on 100% CPU with the latest libvirt
On Tue, Feb 18, 2020 at 08:34:53PM +0100, Andrea Bolognani wrote: [Dropped Peter from CC. Please don't CC individual developers unless they've explicitly requested that you do so.] On Mon, 2020-02-17 at 17:50 +, Richard W.M. Jones wrote: Build libvirt from git (ccf7567329f). Using the libvirt ‘run’ script, run something like libguestfs-test-tool. I think basically any command which runs a guest will do. NB These commands are all run as NON-root: killall libvirtd lt-libvirtd virtlogd lt-virtlogd ./build/run libguestfs-test-tool Now there will be a lt-virtlogd process using 100% of CPU: PID USER PR NIVIRTRESSHR S %CPU %MEM TIME+ COMMAND 2572972 rjones20 0 47880 16256 14516 R 100.0 0.1 0:19.27 lt-virt+ It's actually worse than that: not only virtlogd usese an unwarranted amount of CPU, but it also keeps the log file for the domain busy, thus preventing the same domain from being started again: $ virsh start alpine Domain alpine started $ virsh destroy alpine Domain alpine destroyed $ virsh start alpine error: Failed to start domain alpine error: can't connect to virtlogd: Cannot open log file: '/var/log/libvirt/qemu/alpine.log': Device or resource busy $ sudo lsof | grep alpine.log virtlogd 146845 root 16w REG 253,0 35103 17195654 /var/log/libvirt/qemu/alpine.log $ Restarting virtlogd makes thing operational again: $ sudo systemctl restart virtlogd $ virsh start alpine Domain alpine started $ My guess is that virtlogd doesn't realize the QEMU process is gone, and sits there spinning forever waiting for some output that will never arrive. Yeah, since the switch to GLib event loop, the virLogHandlerDomainLogFileEvent which was supposed to clean that up is no longer called on hangup. My naive fix: https://www.redhat.com/archives/libvir-list/2020-February/msg00651.html Jano signature.asc Description: PGP signature
[libvirt RFC PATCH] util: vireventglibwatch: watch for G_IO_HUP and G_IO_ERR
To more closely match the previous usage in virEventPollDispatchHandles, where called the handle callback for any revents returned by poll. This should fix the virtlogd error on subsequent domain startup: error: can't connect to virtlogd: Cannot open log file: '/var/log/libvirt/qemu/f28live.log': Device or resource busy as well as virtlogd spinning caused by virLogHandlerDomainLogFileEvent never being called on hangup. Signed-off-by: Ján Tomko Fixes: f8ab47cb4491dd72d866c1a96a9d94b8c3341de9 Fixes: 946a25274c4646323c62f567ae7e753aa921 --- src/util/vireventglibwatch.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/vireventglibwatch.c b/src/util/vireventglibwatch.c index 7694e74f23..178707f6b7 100644 --- a/src/util/vireventglibwatch.c +++ b/src/util/vireventglibwatch.c @@ -89,11 +89,11 @@ GSource *virEventGLibCreateSocketWatch(int fd, sizeof(virEventGLibFDSource)); ssource = (virEventGLibFDSource *)source; -ssource->condition = condition; +ssource->condition = condition | G_IO_HUP | G_IO_ERR; ssource->fd = fd; ssource->pollfd.fd = fd; -ssource->pollfd.events = condition; +ssource->pollfd.events = condition | G_IO_HUP | G_IO_ERR; g_source_add_poll(source, >pollfd); -- 2.24.1
[libvirt PATCH] docs: Expand documentation for the tickpolicy timer attribute
The current documentation is fairly terse and not easy to decode for someone who's not intimately familiar with the inner workings of timer devices. Expand on it by providing a somewhat verbose description of what behavior each policy will result in, as seen from both the guest OS and host point of view. This is lifted directly from QEMU commit commit 2a7d957596786404c4ed16b089273de95a9580ad Author: Andrea Bolognani Date: Tue Feb 11 19:37:44 2020 +0100 qapi: Expand documentation for LostTickPolicy v4.2.0-1442-g2a7d957596 The original text also matched word for word the documentation found in QEMU. Signed-off-by: Andrea Bolognani --- docs/formatdomain.html.in | 32 +--- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f4af65f13f..4fef2a0a97 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2487,26 +2487,36 @@ The tickpolicy attribute determines what happens when QEMU misses a deadline for injecting a -tick to the guest: +tick to the guest. This can happen, for example, because the +guest was paused. delay - Continue to deliver ticks at the normal rate. -The guest time will be delayed due to the late -tick + Continue to deliver ticks at the normal rate. The guest OS +will not notice anything is amiss, as from its point of view +time will have continued to flow normally. The time in the +guest should now be behind the time in the host by exactly +the amount of time during which ticks have been missed. catchup - Deliver ticks at a higher rate to catch up -with the missed tick. The guest time should -not be delayed once catchup is complete. + Deliver ticks at a higher rate to catch up with the missed +ticks. The guest OS will not notice anything is amiss, as +from its point of view time will have continued to flow +normally. Once the timer has managed to catch up with all +the missing ticks, the time in the guest and in the host +should match. merge Merge the missed tick(s) into one tick and inject. The guest time may be delayed, depending on how the OS reacts to the merging of ticks discard - Throw away the missed tick(s) and continue -with future injection normally. The guest time -may be delayed, unless the OS has explicit -handling of lost ticks + Throw away the missed ticks and continue with future +injection normally. The guest OS will see the timer jump +ahead by a potentially quite significant amount all at once, +as if the intervening chunk of time had simply not existed; +needless to say, such a sudden jump can easily confuse a +guest OS which is not specifically prepared to deal with it. +Assuming the guest OS can deal correctly with the time jump, +the time in the guest and in the host should now match. If the policy is "catchup", there can be further details in the catchup sub-element. -- 2.24.1
Re: virtlogd spinning on 100% CPU with the latest libvirt
[Dropped Peter from CC. Please don't CC individual developers unless they've explicitly requested that you do so.] On Mon, 2020-02-17 at 17:50 +, Richard W.M. Jones wrote: > Build libvirt from git (ccf7567329f). > > Using the libvirt ‘run’ script, run something like > libguestfs-test-tool. I think basically any command which runs a > guest will do. NB These commands are all run as NON-root: > > killall libvirtd lt-libvirtd virtlogd lt-virtlogd > ./build/run libguestfs-test-tool > > Now there will be a lt-virtlogd process using 100% of CPU: > > PID USER PR NIVIRTRESSHR S %CPU %MEM TIME+ > COMMAND > 2572972 rjones20 0 47880 16256 14516 R 100.0 0.1 0:19.27 > lt-virt+ It's actually worse than that: not only virtlogd usese an unwarranted amount of CPU, but it also keeps the log file for the domain busy, thus preventing the same domain from being started again: $ virsh start alpine Domain alpine started $ virsh destroy alpine Domain alpine destroyed $ virsh start alpine error: Failed to start domain alpine error: can't connect to virtlogd: Cannot open log file: '/var/log/libvirt/qemu/alpine.log': Device or resource busy $ sudo lsof | grep alpine.log virtlogd 146845 root 16w REG 253,0 35103 17195654 /var/log/libvirt/qemu/alpine.log $ Restarting virtlogd makes thing operational again: $ sudo systemctl restart virtlogd $ virsh start alpine Domain alpine started $ My guess is that virtlogd doesn't realize the QEMU process is gone, and sits there spinning forever waiting for some output that will never arrive. -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH] docs: reduce excessive spacing in ToC for RST files
On Fri, Feb 07, 2020 at 04:05:02PM +, Daniel P. Berrangé wrote: The table of contents in the RST based files uses tags inside the , which results in 1em's worth of spacing above & below each entry. This results in way too much whitespace in the ToC. Signed-off-by: Daniel P. Berrangé --- docs/libvirt.css | 4 1 file changed, 4 insertions(+) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH 10/10] docs: add info about to news file
On Sun, Feb 16, 2020 at 11:22:59PM -0500, Laine Stump wrote: Signed-off-by: Laine Stump --- docs/news.xml | 21 + 1 file changed, 21 insertions(+) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH 09/10] conf: extra validation for
On Sun, Feb 16, 2020 at 11:22:58PM -0500, Laine Stump wrote:
During the hypervisor-agnostic validation of network devices, verify
that the interface type is either "network" or "bridge", and that if
there is any , that it doesn't have any type associated
with it.
This needs to be done both for the parse-time validation and for
runtime validation (after a port has been acquired from any associated
network), because an interface with type='network' could have an
actual type at runtime of "hostdev" or "direct", neither of which
support isolated='true' (yet). Likewise, if an interface is
type='network', then at runtime a with a type that
doesn't support isolated='yes' (e.g. "openvswitch", "802.1Qbh" -
currently *none* of the available virtualport types support it)
Signed-off-by: Laine Stump
---
src/conf/domain_conf.c | 56 ++
1 file changed, 56 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 30b2a53b83..f8ce7d519d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6239,6 +6239,47 @@ virDomainRedirdevDefValidate(const virDomainDef *def,
}
+static int
+virDomainNetDefValidatePortOptions(const char *macstr,
+ virDomainNetType type,
+ const virNetDevVPortProfile *vport,
+ virTristateBool isolatedPort)
+{
+/*
+ * This function can be called for either a config interface
+ * object (NetDef) or a runtime interface object (ActualNetDef),
+ * by calling it with either, e.g., the "type" (what is in the
+ * config) or the "actualType" (what is determined at runtime by
+ * acquiring a port from the network).
+ */
+/*
+ * port isolation can only be set for an interface that is
+ * connected to a Linux host bridge (either a libvirt-managed
+ * network, or plain type='bridge')
+ */
+if (isolatedPort == VIR_TRISTATE_BOOL_YES) {
+if (!(type == VIR_DOMAIN_NET_TYPE_NETWORK ||
+ type == VIR_DOMAIN_NET_TYPE_BRIDGE)) {
consider:
if (type != VIR_DOMAIN_NET_TYPE_NETWORK &&
type != VIR_DOMAIN_NET_TYPE_BRIDGE)
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("interface %s - is not
supported for network interfaces with type='%s'"),
Please don't put XML snippets in the error message.
How about:
... - isolated ports are not supported ...
+ macstr, virDomainNetTypeToString(type));
+return -1;
+}
+/*
+ * also not allowed for anything with setting
+ * (openvswitch or 802.11Qb[gh])
+ */
+if (vport && vport->virtPortType != VIR_NETDEV_VPORT_PROFILE_NONE) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("interface %s - is not
supported for network interfaces with virtualport type='%s'"),
+ macstr,
virNetDevVPortTypeToString(vport->virtPortType));
Same here.
+return -1;
+}
+}
+return 0;
+}
+
+
int
virDomainActualNetDefValidate(const virDomainNetDef *net)
{
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
Re: [libvirt PATCH 08/10] qemu: support updating during device update
On Sun, Feb 16, 2020 at 11:22:57PM -0500, Laine Stump wrote: This setting can be updating very easily on an already active interface by just changing it in sysfs. If the bridge used for connection is also changed, there is no need to separately update it, because the new setting isf done as a part of connecting to the bridge s/isf/is/ anyway. Signed-off-by: Laine Stump --- src/qemu/qemu_hotplug.c | 20 1 file changed, 20 insertions(+) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH 07/10] qemu/lxc: plumb isolatedPort from config down through bridge attachment
On Sun, Feb 16, 2020 at 11:22:56PM -0500, Laine Stump wrote:
This patch pushes the isolatedPort setting from the down
all the way to the callers of virNetDevBridgeAddPort(), and sets
BR_ISOLATED on the port (using virNetDevBridgePortSetIsolated()) after
the port has been successfully added to the bridge.
Signed-off-by: Laine Stump
---
src/bhyve/bhyve_command.c | 1 +
src/conf/domain_conf.c | 1 +
src/lxc/lxc_process.c | 10 ++
src/network/bridge_driver.c | 1 +
src/qemu/qemu_hotplug.c | 16
src/qemu/qemu_interface.c | 1 +
src/util/virnetdevtap.c | 17 -
src/util/virnetdevtap.h | 3 +++
tests/bhyvexml2argvmock.c | 1 +
9 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 6395826c69..af892255c7 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3350,12 +3350,28 @@ qemuDomainChangeNetBridge(virDomainObjPtr vm,
}
ret = virNetDevBridgeAddPort(newbridge, olddev->ifname);
+if (ret == 0 &&
+virDomainNetGetActualPortOptionsIsolated(newdev) ==
VIR_TRISTATE_BOOL_YES) {
+
+ret = virNetDevBridgePortSetIsolated(newbridge, olddev->ifname, true);
+if (ret < 0) {
+virErrorPtr err;
+
+virErrorPreserveLast();
+ignore_value(virNetDevBridgeRemovePort(newbridge, olddev->ifname));
+virErrorRestore();
+}
+}
virDomainAuditNet(vm, NULL, newdev, "attach", ret == 0);
if (ret < 0) {
virErrorPtr err;
virErrorPreserveLast();
ret = virNetDevBridgeAddPort(oldbridge, olddev->ifname);
+if (ret == 0 &&
+virDomainNetGetActualPortOptionsIsolated(olddev) ==
VIR_TRISTATE_BOOL_YES) {
+ignore_value(virNetDevBridgePortSetIsolated(newbridge,
olddev->ifname, true));
Should this use 'oldbridge' instead of 'newbridge'?
+}
virDomainAuditNet(vm, NULL, olddev, "attach", ret == 0);
virErrorRestore();
return -1;
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
Re: [libvirt PATCH 06/10] network: propagate between network and domain
On Sun, Feb 16, 2020 at 11:22:55PM -0500, Laine Stump wrote: Similar to the way that the , , and elements and the trustGuestRxFilters attribute in a (or in the appropriate element of a can be applied to a port when it is allocated for a domain's network interface, this patch checks for a configured value of in either the domain or in the network, setting isolatedPort in the to the first one it finds (the setting from the domain's is preferred). This, in turn, is passed back to the domain when a port is allocated, so that the domain will use that setting. (One difference from , , , and trustGuestRxFilters, is that all of those can be set in a so that they can be applied only to a subset of interfaces connected to the network. This didn't really make sense for the isolated setting due to the way that it's implemented in Linux - the BR_ISOLATED flag will prevent traffic from passing between two ports that both have BR_ISOLATED set, but traffic can still go between those ports and other ports that *don't* have BR_ISOLATED. (It would be nice if all traffic from a BR_ISOLATED port could be blocked except traffic going to/from a designated egress port or ports, but instead the entire feature is implemented as a single flag. Because of this, it's really only useful if all the ports on a network are isolated, so setting it for a subset has no practical utility.) Signed-off-by: Laine Stump --- src/conf/domain_conf.c | 3 +++ src/network/bridge_driver.c | 3 +++ 2 files changed, 6 insertions(+) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH 05/10] conf: parse/format
On Sun, Feb 16, 2020 at 11:22:54PM -0500, Laine Stump wrote: This is a very simple thing to parse and format, but needs to be done in 4 places, so two trivial utility functions have been made that can be called from all the higher level parser/formatters: (only in domain status) Signed-off-by: Laine Stump --- docs/schemas/domaincommon.rng | 3 + docs/schemas/network.rng | 3 + docs/schemas/networkcommon.rng| 11 docs/schemas/networkport.rng | 3 + src/conf/domain_conf.c| 19 ++ src/conf/domain_conf.h| 4 ++ src/conf/network_conf.c | 32 ++ src/conf/network_conf.h | 9 +++ src/conf/virnetworkportdef.c | 3 + src/conf/virnetworkportdef.h | 1 + src/libvirt_private.syms | 1 + tests/networkxml2xmlin/isolated-ports.xml | 7 +++ tests/networkxml2xmlout/isolated-ports.xml| 7 +++ tests/networkxml2xmltest.c| 1 + tests/qemuxml2argvdata/net-isolated-port.xml | 34 ++ .../net-isolated-port.x86_64-latest.xml | 63 +++ tests/qemuxml2xmltest.c | 1 + 17 files changed, 202 insertions(+) create mode 100644 tests/networkxml2xmlin/isolated-ports.xml create mode 100644 tests/networkxml2xmlout/isolated-ports.xml create mode 100644 tests/qemuxml2argvdata/net-isolated-port.xml create mode 100644 tests/qemuxml2xmloutdata/net-isolated-port.x86_64-latest.xml Not a fan of multi-word elements, because they bring up our inconsistency in using camelCase vs snake_case. But I assume you chose the name to make it compatible with all four containing elements. Would something like: look too odd? Code-wise: Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
