On 04/15/2014 03:26 PM, John Ferlan wrote: > > On 04/10/2014 09:19 AM, Laine Stump wrote: >> For some reason these have been stored in /var/lib, although other >> drivers store their state files in /var/run. >> >> It's much nicer to store them in /var/run because they are >> automatically cleared out when the system reboots. This can be a >> useful way of learning whether or not a particular network is active. >> >> Note that this change by itself will cause problems in the case of an >> upgrade from an older libvirt that uses /var/lib, and also in the case >> of a downgrade from a newer libvirt using /var/run to an older version >> using /var/lib. This compatibility problem will be addressed in the >> next patch. >> --- >> src/network/bridge_driver.c | 11 ++++++----- >> 1 file changed, 6 insertions(+), 5 deletions(-) >> >> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c >> index fdddc9a..a027b47 100644 >> --- a/src/network/bridge_driver.c >> +++ b/src/network/bridge_driver.c >> @@ -446,10 +446,11 @@ networkStateInitialize(bool privileged, >> * ~/.config/libvirt/... (session/unprivileged) >> * /etc/libvirt/... && /var/(run|lib)/libvirt/... (system/privileged). >> * >> - * NB: The qemu driver puts its domain state in /var/run, and I >> - * think the network driver should have used /var/run too (instead >> - * of /var/lib), but it's been this way for a long time, and we >> - * probably shouldn't change it now. >> + * NB: The network driver previously stored its status xml in >> + * /var/lib/libvirt/network instead of >> + * /var/run/libvirt/network. An upgrade downgrade between libvirt >> + * versions using different state directories may or may not cause >> + * problems. >> */ >> if (privileged) { >> if (VIR_STRDUP(driverState->networkConfigDir, >> @@ -457,7 +458,7 @@ networkStateInitialize(bool privileged, >> VIR_STRDUP(driverState->networkAutostartDir, >> SYSCONFDIR "/libvirt/qemu/networks/autostart") < 0 || >> VIR_STRDUP(driverState->stateDir, >> - LOCALSTATEDIR "/lib/libvirt/network") < 0 || >> + LOCALSTATEDIR "/run/libvirt/network") < 0 || >> VIR_STRDUP(driverState->pidDir, >> LOCALSTATEDIR "/run/libvirt/network") < 0 || >> VIR_STRDUP(driverState->dnsmasqStateDir, >> > This is only being done for the /var/... files and not the > non-privileged case, right? Correct. > I guess part of me wonders why not follow > through and do the same in the else here Because that else clause for non-privileged was added long after the original code for privileged was written, and the directories are already in the correct place from the beginning so they don't need to be fixed. > - wouldn't that code have the > same 'feature' you're trying to take advantage of regarding how the run > directory is used? I'm actually not sure exactly what is done with the directories in ~/.config (when/if the $run directory is cleared, etc), but there was some sort of reconfig/standardization of the locations with gnome3 that the qemu driver was made to follow, and the network driver copies those locations faithfully (replacing "qemu" with "network" or "qemu/network" as appropriate). > I suppose I assume that the non priv'd environment > acts just like the priv'd one except for location. non-privileged libvirtd is a bit odd. For starters it isn't run until someone tries to connect to their user's qemu:///session, and then it automatically exits after some timeout. Beyond that, the network driver doesn't actually work (!!) for non-privileged libvirtd - adding the ~/.config directories was only done to prevent it from crashing, with the thinking that if we *did* make it work sometime in the future, those would be the right places to store the files (since it works for qemu...) > > Beyond that - it feels like 2 & 3 need to be combined or reworked to > avoid the git bisect issues. Well, yes and no. Both patches independently pass make check and make syntax-check, and I hadn't considered that anyone would do a bisect where the test at each step was to install an older libvirt, then upgrade to the newly built bisect binary. However, my main reason for separating the two was just to make review easier (as well as in case I decided to make any more complicated alternative to 3/5), and there's no point in dooming such a bisect test to failure for no good reason, so I would be fine with merging 2/5 and 3/5. -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list