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 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.
> >
> > That doesn't seem to be the case:
> >
> > [... a bunch of command line nonsense ...]
>
> But... I did a nearly identical experiment last night before posting my
> reply, and came up with different results, which is why I posted...
>
>
> Ah! So the difference is that in your example, you have no MAC address
> either, and the code you point out down below is noticing that change,
> and causing the entire network config to be re-written to the disk. I
> hadn't removed the MAC address, so in my case it didn't re-write the config.
>
>
> (...and now I've read to the bottom of your reply and see that you've
> already figured that out :-P)
"And that's why you always read the entire message before
you start replying to it."
-- J. Walter Weatherman
> I guess if we're okay with re-writing the config at daemon start to add
> a MAC address, then we should be okay with doing that to add a uuid
> (they're really just differently-used examples of the same thing). But
> if we're going to do that, the check to see if the config should be
> modified to check both uuid and mac address (even if in current practice
> both are absent from the proto-default.xml, that may not be the case in
> the future).
>
>
> Alternately, if we're *not* okay with rewriting the config at daemon
> start time (even though, as you've pointed out, we've been doing that
> for 6 years already), then we would need to *stop* doing that for MAC
> address. (Which would mean we would need to figure out a different way
> to get a fixed MAC address into the config file, and seeing that the
> commit you reference actually *removes* my original code that added it
> in at install time, we can't really go back there (also because that's
> moving the code in the wrong direction - Fedora SilverBlue people said
> in the BZ I referenced that they don't want %post_install scripts
> modifying the files that are installed).
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-provided configuration and then writing the result back to disk
is quite central to how we handle domains and other objects, and I
see no reason networks should behave any differently.
If anything, I think we should not extend the check to UUIDs, but
just drop it completely and unconditionally write the (possibly)
updated file file to disk. I'll try cooking a patch for that.
> I think I'm okay with the former (adding a check for changed uuid onto
> your patches); it gives us a stable uuid (as long as the modified disk
> contents are saved for the next reboot) while eliminating another bit of
> the undesirable code that runs during %post_install.
>
> Is there anything else wrt. pre-built images that we're not taking into
> consideration though?
Honestly I was not considering the pre-built images angle at all, so
there's probably *loads* that I'm not taking into account O:-)
--
Andrea Bolognani / Red Hat / Virtualization
