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

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

 



On Tue, 2014-05-27 at 11:17 +0200, Jiri Denemark wrote: 
> On Mon, May 26, 2014 at 10:21:09 +0000, chen.fan.fnst@xxxxxxxxxxxxxx wrote:
> > 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@xxxxxxxxxxxxxx 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.
> 
> As I said, I don't really like the idea of applying changes to selected
> options while ignoring the rest of the options. Moreover since some
> options cannot be easily changed even on SIGHUP and since changing
> libvirt's configuration is not something anyone would do every minute, I
> think the requirement to restart libvirtd to apply configuration changes
> is good enough.
> 
I'm thinking more about virt-manager, If having to restart libvirtd to
apply changed configuration, the virt-manager must reconnect with this
host's libvirtd every time. if support this without restart libvirtd, 
isn't it better?

Thanks,
Chen




> Jirka


--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]