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 stored in the original xml in /etc/libvirt, so a new and different uuid will be generated every time libvirt is restarted.


I don't know if the solution to this is to modify libvirt so that it rewrites the XML for a network if it has to auto-generate any attributes[*], or to put a canned/static uuid value in the installed xml, or just to declare that we don't care if the uuid changes from one run of libvirtd to the next. But definitely these patches change behavior, so we probably need to point that out and maybe discuss it.


BTW, the whole thing of generating a different default.xml for each install of libvirt is a thorn in the side anyway - I looked for the BZ describing the problem and came up with this:


https://bugzilla.redhat.com/show_bug.cgi?id=1657041


which isn't the entire discussion, but is part of it. Basically while talking about this in *some venue* (maybe it was IRC, maybe a different BZ?) we decided that we shouldn't be (can't be?) generating files during the package %post_install. Our main topic of discussion was the subnet of the network, but adding/modifying the uuid is just another example of the same undesired behavior.


So *that* would be an argument in favor of at least the "don't generate a different uuid at install time" part of your patches. But it doesn't (afaics) lead to a definite resolution of "what should be done instead?".

(btw, I'm working / supposed to be working on eliminating the need to modify the subnet address in this same file with a new "autoaddress" function that will replace a fixed IP in a network definition with an indicator that means "find an unused subnet and use that". Doesn't solve the uuid bit though)


[*] I have a vague memory that there is (or was?) at least one case where we would automatically update the persistent definition of a domain or network on disk when libvirt started (it was needed due to a change in some attribute or something), but I don't recall the details, don't feel like looking for it right now, and it anyway is not the norm.

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 <abolo...@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@</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(),


Reply via email to