9:47 a.m.
On 11/16/20 11:43 AM, Andrea Bolognani wrote:
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 understand why you would consider doing this (low/no maintenance,
easier to verify correct results), but I don't think it's a generally
good idea to rewrite every config file every time we restart libvirtd.
Some media has slow write speed and a limited number of writes in its
lifetime (sure, it's not 1982 and we're not talking about a 2716 EPROM,
but still :-)) and some people may rely on /etc/libvirt to be in a
read-only filesystem for some reason (I haven't tried that, but am just
assuming that it would work). Also, the date stamp on a file can provide
useful clues about when something got screwed up (although admittedly it
is useless in the quest to actually undo the screwup)
Except for three cases (that I can think of; I'm sure there are others),
what is read from libvirt's config files in /etc is exactly what would
be written if we were to re-write it immediately after a read-parse-format:
1) the first read of an xml file that was generated outside libvirt, and
has been manually put into /etc/libvirt, e.g. xml files that are a part
of a package installation/upgrade. NB: all other cases of externally
generated files being placed in /etc/libvirt are expressly forbidden
(well, at least "discouraged and declared unsupported") by libvirt
documentation.
2) a user manually edits a file in /etc/libvirt - see (1)
3) upgrading libvirt causes something to be parsed differently than it
was before. This should never happen, as there should always be enough
info in the xml we write to the disk after its initial arrival via the
libvirt API that any future read/write cycles are completely idempotent.
> 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:-)
Or maybe I'm just paranoid. That is just as likely.
(Hmmph. After typing this my brain has figured out that the
EPROM-writing card that should be stored away in a box somewhere inside
another box on a shelf in my garage is an ISA-bus card, and so I haven't
owned any computer it would plug into in at least 11 years. What will I
do if I ever need to reprogram the EPROM on my 1980's homebrew
computer?? :-( Should my lesson be that I should throw away less old
hardware? Or more?)