Re: [libvirt] [patch v3 2/2] add inotify handler to qemu driver

On Thu, 2014-05-22 at 13:12 +0200, Jiri Denemark wrote: > On Thu, May 22, 2014 at 02:07:32 +0000, chen.fan.fnst(a)cn.fujitsu.com wrote: > > On Thu, 2014-05-15 at 11:25 +0200, Jiri Denemark wrote: > > > On Wed, May 07, 2014 at 18:11:50 +0800, Chen Fan wrote: > > > > we don't expect to reload 'migrate_uri' with restarting libvirtd everytime while > > > > updating the URI, so adding inotify handler to reload 'migrate_uri' configuration > > > > without restarting libvirtd, it will be also helpful for virt-manager to get > > > > 'migrate_uri'. > > > > > > NACK, if we ever want configuration to be automatically reloaded when > > > configuration file changes (which I seriously doubt, SIGHUP is the > > > standard way of reloading configuration files), we certainly would not > > > want to do it this hacky way and only for one specific option. Not to > > > mention that the content of virQEMUDriverConfig must not change, a > > > completely new structure has to be created with the changed values. > > > > > Hi Jiri, > > thanks for reminding me, actually, I knew the virQEMUDriverConfig > > without lock protect to reload is wrong. except that I think there are > > some options in virQEMUDriverConfig can be reloaded. as you said, If > > we want to reload them, Do we must create a new structure to save the > > changes, then we must use 'lock' to make code thread safe and override > > the qemuDriverCfg values everywhere while we use them. right? > > or are there any other ideas? > > virQEMUDriverConfig is immutable so the structure returned by > virQEMUDriverGetConfig(qemu_driver) must never be changed. You have to > create a copy of it, update the values in the new structure, take a > driver lock, unref the original driver->config and put the new pointer > to driver->config. This way, any code that already got > virQEMUDriverConfig pointer will be using the old value and all calls to > virQEMUDriverGetConfig will return the updated configuration. The old > one will be freed when its last user unrefs it. > > But anyway, we should not be updating the config whenever the > configuration file changes. We should rather do that in qemuStateReload. > However, I'm not sure what to do if there are options that just cannot > be changed while libvirtd is running. Applying some changes while > ignoring others when SIGHUP is sent to libvirtd would certainly lead to > confusion. How about dividing this qemu_config into two parts, one is used for the immutable values, and the other is used for the mutable values? then if SIGHUP is sent to libvirtd, we can use the above method you mentioned to update the mutable configuration. maybe we can add an independent structure pointer in virQEMUDriverConfig to save them. and only make a copy of this mutable config rather than the entire virQEMUDriverConfig.