4:36 p.m.
Changes from [v1]:
* ensure the updated network configuration is written back to disk
whenever a UUID has been generated.
[v1] https://www.redhat.com/archives/libvir-list/2020-November/msg00785.html
Andrea Bolognani (3):
conf: Write network config to disk after generating UUID
network: Drop UUID handling for default network
spec: Drop UUID handling for default network
libvirt.spec.in | 4 ----
src/conf/virnetworkobj.c | 17 ++++++++++++++++-
src/network/meson.build | 32 +++++++-------------------------
3 files changed, 23 insertions(+), 30 deletions(-)
--
2.26.2
4:36 p.m.
New subject: [libvirt PATCH v2 1/3] conf: Write network config to disk after generating UUID
While we generally expect libvirt objects to be defined using the
appropriate APIs, there are cases where it's reasonable for an
external entity, usually a package manager, to drop a valid
configuration file under /etc/libvirt and have libvirt take over
from there: notably, this is exactly how the default network is
handled.
For the most part, whether the configuration is saved back to disk
after being parsed by libvirt doesn't matter, because we'll end up
with the same values anyway, but an obvious exception to this is
data that gets randomly generated when not present, namely MAC
address and UUID.
Historically, both were handled by our build system, but commit
a47ae7c004e9 moved handling of the former inside libvirt proper;
this commit extends such behavior to the latter as well.
Proper error handling for the virNetworkSaveConfig() call, which
was missing until now, is introduced in the process.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/conf/virnetworkobj.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index 1b29d7cc64..1e167c7874 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -1001,6 +1001,7 @@ virNetworkLoadConfig(virNetworkObjListPtr nets,
char *configFile = NULL, *autostartLink = NULL;
virNetworkDefPtr def = NULL;
virNetworkObjPtr obj;
+ bool saveConfig = false;
int autostart;
if ((configFile = virNetworkConfigFile(configDir, name)) == NULL)
@@ -1029,7 +1030,10 @@ virNetworkLoadConfig(virNetworkObjListPtr nets,
case VIR_NETWORK_FORWARD_OPEN:
if (!def->mac_specified) {
virNetworkSetBridgeMacAddr(def);
- virNetworkSaveConfig(configDir, def, xmlopt);
+ /* We just generated a new MAC address, and we need to persist
+ * the configuration to disk to avoid the network getting a
+ * different one the next time the daemon is started */
+ saveConfig = true;
}
break;
@@ -1049,6 +1053,17 @@ virNetworkLoadConfig(virNetworkObjListPtr nets,
goto error;
}
+ /* The network didn't have a UUID so we generated a new one, and
+ * we need to persist the configuration to disk to avoid the network
+ * getting a different one the next time the daemon is started */
+ if (!def->uuid_specified)
+ saveConfig = true;
+
+ if (saveConfig &&
+ virNetworkSaveConfig(configDir, def, xmlopt) < 0) {
+ goto error;
+ }
+
if (!(obj = virNetworkObjAssignDef(nets, def, 0)))
goto error;
--
2.26.2
4:36 p.m.
New subject: [libvirt PATCH v2 2/3] network: Drop UUID handling for default network
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; the fact that we only do this if the uuidgen command happens
to be available on the build machine is further proof that we can
safely skip this step.
This patch is best viewed with 'git show -w'.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/network/meson.build | 32 +++++++-------------------------
1 file changed, 7 insertions(+), 25 deletions(-)
diff --git a/src/network/meson.build b/src/network/meson.build
index 13dd2c26b2..3ec598c3f9 100644
--- a/src/network/meson.build
+++ b/src/network/meson.build
@@ -84,31 +84,13 @@ if conf.has('WITH_NETWORK')
runstatedir / 'libvirt' / 'network',
]
- uuidgen_prog = find_program('uuidgen', required: false)
-
- if uuidgen_prog.found()
- uuid = run_command(uuidgen_prog).stdout().strip()
-
- configure_file(
- input: 'default.xml.in',
- output: '@BASENAME@',
- command: [
- 'sed', '-e', 's|</name>|</name>\\n
<uuid>@0(a)</uuid>|'.format(uuid),
- '@INPUT@',
- ],
- capture: true,
- install: true,
- install_dir: confdir / 'qemu' / 'networks',
- )
- else
- configure_file(
- input: 'default.xml.in',
- output: '@BASENAME@',
- copy: true,
- install: true,
- install_dir: confdir / 'qemu' / 'networks',
- )
- endif
+ configure_file(
+ input: 'default.xml.in',
+ output: '@BASENAME@',
+ copy: true,
+ install: true,
+ install_dir: confdir / 'qemu' / 'networks',
+ )
meson.add_install_script(
meson_python_prog.path(), python3_prog.path(), meson_install_symlink_prog.path(),
--
2.26.2
4:36 p.m.
New subject: [libvirt PATCH v2 3/3] spec: Drop UUID handling for default network
We're no longer generating a UUID during installation, so we
clearly don't need to strip it afterwards; and since the network
driver is perfectly capable of generating a UUID if necessary, we
don't need to do that at %post time either.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
Reviewed-by: Neal Gompa <ngompa13(a)gmail.com>
---
libvirt.spec.in | 4 ----
1 file changed, 4 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index de76878bc6..880051b6f8 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1240,8 +1240,6 @@ cp -a $RPM_BUILD_ROOT%{_sysconfdir}/libvirt/nwfilter/*.xml \
# libvirt saves these files with mode 600
chmod 600 $RPM_BUILD_ROOT%{_sysconfdir}/libvirt/nwfilter/*.xml
-# Strip auto-generated UUID - we need it generated per-install
-sed -i -e "/<uuid>/d"
$RPM_BUILD_ROOT%{_datadir}/libvirt/networks/default.xml
%if ! %{with_qemu}
rm -f $RPM_BUILD_ROOT%{_datadir}/augeas/lenses/libvirtd_qemu.aug
rm -f $RPM_BUILD_ROOT%{_datadir}/augeas/lenses/tests/test_libvirtd_qemu.aug
@@ -1422,9 +1420,7 @@ if test $1 -eq 1 && test ! -f
%{_sysconfdir}/libvirt/qemu/networks/default.xml ;
;;
esac
- UUID=`/usr/bin/uuidgen`
sed -e "s/${orig_sub}/${sub}/g" \
- -e "s,</name>,</name>\n <uuid>$UUID</uuid>,"
\
< %{_datadir}/libvirt/networks/default.xml \
%{_sysconfdir}/libvirt/qemu/networks/default.xml
ln -s
../default.xml %{_sysconfdir}/libvirt/qemu/networks/autostart/default.xml
--
2.26.2
5:03 p.m.
On 11/19/20 10:36 AM, Andrea Bolognani wrote:
Changes from [v1]:
* ensure the updated network configuration is written back to disk
whenever a UUID has been generated.
[v1] https://www.redhat.com/archives/libvir-list/2020-November/msg00785.html
Andrea Bolognani (3):
conf: Write network config to disk after generating UUID
network: Drop UUID handling for default network
spec: Drop UUID handling for default network
libvirt.spec.in | 4 ----
src/conf/virnetworkobj.c | 17 ++++++++++++++++-
src/network/meson.build | 32 +++++++-------------------------
3 files changed, 23 insertions(+), 30 deletions(-)
Reviewed-by: Laine Stump <laine(a)redhat.com>
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 that will mean
that it actually gets done though :-)
11:53 a.m.
On Thu, 2020-11-19 at 11:03 -0500, Laine Stump wrote:
On 11/19/20 10:36 AM, Andrea Bolognani wrote:
> Andrea Bolognani (3):
> conf: Write network config to disk after generating UUID
> network: Drop UUID handling for default network
> spec: Drop UUID handling for default network
Reviewed-by: Laine Stump <laine(a)redhat.com>
for the series.
Thanks, pushed now.
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 that will mean
that it actually gets done though :-)
I'm going to take a stab at that, but I felt like there was a very
strong case for introducing this behavior for networks specifically
because 1) we already have it for MAC addresses and 2) we also
already do it for nwfilters, the other libvirt object for which we
know for sure we're going to be dropping configuration files into
/etc/libvirt at install time.
While I think extending this behavior to all libvirt objects is the
way to go, I can also see that being a more controversial change, so
I didn't to tie the two together and risk this never getting merged
because of that :)
--
Andrea Bolognani / Red Hat / Virtualization
9:13 p.m.
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 guarantee that will mean
> that it actually gets done though :-)
I'm going to take a stab at that, but I felt like there was a very
strong case for introducing this behavior for networks specifically
because 1) we already have it for MAC addresses and 2) we also
already do it for nwfilters, the other libvirt object for which we
know for sure we're going to be dropping configuration files into
/etc/libvirt at install time.
While I think extending this behavior to all libvirt objects is the
way to go, I can also see that being a more controversial change, so
I didn't to tie the two together and risk this never getting merged
because of that :)
Yup. I was only suggesting it as something to add to the todo list, not
something that needed to be done right away.
1400
days inactive
1401
days old
6 comments
2 participants
participants (2)
-
Andrea Bolognani -
Laine Stump