Re: [libvirt] [PATCH 3/5] network: read status from old location and move to new

On 04/15/2014 03:27 PM, John Ferlan wrote: Thinking about it, there was one similar situation - when sessionmode libvirtd switched from using gnome2 config directories to gnome3. I don't recall if anything special was done in that case, or if it was just considered not important because ... session mode. AFAIK, the run directory for the qemu, lxc, and network drivers hasn't before been changed, so there's no precendent to follow there. No, this is exactly what the qemu and lxc drivers do. Their persistent config is stored in /etc/libvirt, and their runtime status is stored in /var/run/libvirt. This is specifically so that when the host is rebooted, stale status is automatically cleared out to avoid confusing libvirtd when it is started the first time after a reboot. When libvirtd is subsequently restarted, its pre-restart state for all domains and networks *must* be maintained to avoid interruption of service, and /var/run is just the right place to do that. (As an aside, in the case of the network driver, much of the state of a network is reconstructed when libvirtd is restarted rather than relying on a correct statedir; this was mostly necessary due to the unreliability of the state data in the past. The only reason I'm going to all this trouble now is that I have a need for a piece of status that can't be reconstructed from other host and domain information but can only come the status file itself - the answer to the basic "is this network active?" question. Oops! Right. Not needed because we haven't changed the location of the stateDir for non-privileged (and, as I said previously, the network driver doesn't actually work for non-privileged mode, beyond simply not crashing :-P) No. STRNEQ_NULLABLE(a, b) will be TRUE in the case that b is NULL, but we only want the body of the conditional executed if oldStateDir is NON-NULL and it doesn't equal stateDir. Well, we haven't changed the grammar at all, only the location we're reading from. And we know that, by definition, if we change the grammar in the future, it will be backward compatible with the current grammar, so there will be no failure caused by incompatible grammar during an upgrade. In the case that there is some other problem reading the status files during upgrade, the reaction would be no different from what would happen if there was a problem reading the status files when we *aren't* upgrading - the network driver would fail to load, and the status files would still exist in the oldStateDir, ready to a retry the next time we start. For downgrades, we're not trying to move the state files back to the old location anyway, so I don't see any extra problem beyond the fact that the status directory known by the old libvirt will be empty, so some bandwidth state will be lost, and macvtap/hostdev networks will be marked inactive (but that's already covered in the comments) In a theoretic sense, we should never get rid of it. But practically speaking someday somebody may decide to eliminate it (when libvirt has advanced so many versions that we're sure everyone is running on a "post-epoch" version, or at least that they can't do a live upgrade from a pre-epoch version to the newest version anyway). In that case, the most recent status file will exist in the new stateDir anyway, but if I renamed any file I found in oldStateDir and left it there for potential re-use during a future downgrade, it might be several *months* old and have survived multiple edits to the network + reboots of the host. In the meantime, if we *don't* downgrade, we'll have extra files cluttering up /var/lib/libvirt confusing people essentially forever. Much better to get rid of it as soon as possible. Any particular reason for this? It is only used once in one place, and the function it's in isn't all that long. I suppose I can move it into a static function declared just above this one though.