[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v1 06/11] network: Create real network status files

On 30.11.2012 20:04, Laine Stump wrote:
> On 11/19/2012 11:51 AM, Michal Privoznik wrote:
>> Currently, we are only keeping a inactive XML configuration
>> in status dir. This is no longer enough as we need to keep
>> this class_id attribute so we don't overwrite old entries
>> when the daemon restarts. 
> Aha! So you're looking at solving the problem in a different way - save
> everything to the status file rather than recomputing it as you restart.
> While I like the idea of having the network status file hold this
> information, I think its reliability is suspect. What if a guest's
> process is terminated while libvirtd isn't running? Or what if libvirtd
> exits unexpectedly after the commands to setup bandwidth have been
> executed, but before the new network state file has been written (or
> vice versa, depending on the code).
> Also, networks aren't properly shut down when the host is being shutdown
> (there's no equivalent to the libvirt-guests service, although at least
> one person a month or two ago requested it).

If guest is being shut down, the networkReleaseActualDevice() is called,
isn't it? And this should update the floor_sum. Even if libvirtd is
restarted and qemu process dies meanwhile, qemuProcessStop is called and
this calls the networkReleaseActualDevice() so I think we are safe here.

> If everybody agrees that saving this info to a file and re-reading it on
> start up is reliable, though, then we might as well do the same thing
> with the device pool (although it's currently a bit different - the
> inuse count is stored in the virNetworkDef rather than Obj, and is
> reported during net-dumpxml)

With new code we can update this.

>> However, since there has already
>> been a libvirt release 
> *a* libvirt release? :-)

That 'a' belongs to 'release'. It doesn't matter which release it was.
Yeah, now that I re-read it again it's a mess :)

>> which has just <network/> as root element,
>> and we want to keep things compatible, detect that loaded
>> status file is older one, and don't scream about it.
>> ---
>>  src/conf/network_conf.c     |  199 ++++++++++++++++++++++++++++++++++---------
>>  src/conf/network_conf.h     |    2 +
>>  src/libvirt_private.syms    |    1 +
>>  src/network/bridge_driver.c |    9 +--
>>  4 files changed, 165 insertions(+), 46 deletions(-)

> ACK.

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]