Re: [libvirt] [virt-tools-list] [PATCH] virtinst: refresh pools status before fetch_pools
On 12/5/2014 at 09:54 PM, in message <5481B907.4040507@redhat.com>, Cole Robinson <crobinso@redhat.com> wrote: On 12/05/2014 03:40 AM, Chunyan Liu wrote: Currently, when connecting to hypervisor, if there are pools active but in fact target path already deleted (or for other reasons the pool is not working), libvirtd not refresh status yet, fetch_pools will fail, that will cause "connecting to hypervisor" process reporting error and exit. The whole connection work failed.
With the patch, always refresh pool status before fetch pools. Let the libvirtd pool status reflect the reality, avoid the non-synced status affects the hypervisor connection.
Signed-off-by: Chunyan Liu <cyliu@suse.com> --- virtinst/pollhelpers.py | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/virtinst/pollhelpers.py b/virtinst/pollhelpers.py index a9b1527..e8702f0 100644 --- a/virtinst/pollhelpers.py +++ b/virtinst/pollhelpers.py @@ -133,6 +133,19 @@ def fetch_pools(backend, origmap, build_func):
if backend.check_support( backend.SUPPORT_CONN_LISTALLSTORAGEPOOLS) and not _force_old_poll: + + # Refresh pools before poll_helper. For those + # 'active' but target path not exist (or other reasons + # causing the pool not working), but libvirtd not + # refresh the status, this will make it refreshed + # and mark that pool as 'inactive'. + objs = backend.listAllStoragePools() + for obj in objs: + try: + obj.refresh(0) + except Exception, e: + pass + return _new_poll_helper(origmap, name, backend.listAllStoragePools, build_func) else:
This is a very heavy hammer, refresh is a potentially long running operation
so this could cause decent slowdown in some scenarios.
IMO this is essentially a libvirt bug, for pools with target directories (dir, fs, netfs), libvirt should be periodically checking the directory ctime and doing the pool refresh for us. And if the target has disappeared, it shuts down the pool (like shutting down a VM if it crashes).
Hi, libvirt list, I'm not sure if Cole's suggestion could be done in libvirt, so just forward the mail to libvirt mailing list. Any opinions? Chunyan
We have so many hacks sprinkled around in virtinst/virt-manager dealing with the fallout of this lacking libvirt feature. Really wish I had implemented it years ago. But I'd rather focus on that then adding yet more hacks.
If there's a specific issue you're hitting that's manifesting itself elsewhere in the app, let us know and maybe there's a way to mitigate it in the interim
- Cole
_______________________________________________ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
On 11.12.2014 03:27, Chun Yan Liu wrote:
>
>
>>>> On 12/5/2014 at 09:54 PM, in message <5481B907.4040507@redhat.com>, Cole
> Robinson <crobinso@redhat.com> wrote:
>> On 12/05/2014 03:40 AM, Chunyan Liu wrote:
>>> <snip/>
>>> + # Refresh pools before poll_helper. For those
>>> + # 'active' but target path not exist (or other reasons
>>> + # causing the pool not working), but libvirtd not
>>> + # refresh the status, this will make it refreshed
>>> + # and mark that pool as 'inactive'.
>>> + objs = backend.listAllStoragePools()
>>> + for obj in objs:
>>> + try:
>>> + obj.refresh(0)
>>> + except Exception, e:
>>> + pass
>>> +
>>> return _new_poll_helper(origmap, name,
>>> backend.listAllStoragePools, build_func)
>>> else:
>>>
>>
>> This is a very heavy hammer, refresh is a potentially long running operation
>>
>> so this could cause decent slowdown in some scenarios.
>>
>> IMO this is essentially a libvirt bug, for pools with target directories
>> (dir,
>> fs, netfs), libvirt should be periodically checking the directory ctime and
>> doing the pool refresh for us. And if the target has disappeared, it shuts
>> down the pool (like shutting down a VM if it crashes).
Makes sense to me. Although, refreshing a dir can again be a long
running job (consider dir pool built in a NFS mount). I wonder if we
should use {d,i,fa}notify or if checking ctime is enough.
Michal
On Thu, Dec 11, 2014 at 02:03:37PM +0100, Michal Privoznik wrote:
> On 11.12.2014 03:27, Chun Yan Liu wrote:
> >
> >
> >>>>On 12/5/2014 at 09:54 PM, in message <5481B907.4040507@redhat.com>, Cole
> >Robinson <crobinso@redhat.com> wrote:
> >>On 12/05/2014 03:40 AM, Chunyan Liu wrote:
> >>> <snip/>
> >>>+ # Refresh pools before poll_helper. For those
> >>>+ # 'active' but target path not exist (or other reasons
> >>>+ # causing the pool not working), but libvirtd not
> >>>+ # refresh the status, this will make it refreshed
> >>>+ # and mark that pool as 'inactive'.
> >>>+ objs = backend.listAllStoragePools()
> >>>+ for obj in objs:
> >>>+ try:
> >>>+ obj.refresh(0)
> >>>+ except Exception, e:
> >>>+ pass
> >>>+
> >>> return _new_poll_helper(origmap, name,
> >>> backend.listAllStoragePools, build_func)
> >>> else:
> >>>
> >>
> >>This is a very heavy hammer, refresh is a potentially long running operation
> >>
> >>so this could cause decent slowdown in some scenarios.
> >>
> >>IMO this is essentially a libvirt bug, for pools with target directories
> >>(dir,
> >>fs, netfs), libvirt should be periodically checking the directory ctime and
> >>doing the pool refresh for us. And if the target has disappeared, it shuts
> >>down the pool (like shutting down a VM if it crashes).
>
>
> Makes sense to me. Although, refreshing a dir can again be a long running
> job (consider dir pool built in a NFS mount). I wonder if we should use
> {d,i,fa}notify or if checking ctime is enough.
I think ultimately libvirt should make use of *notify in many places in
its code, including the filesystem based storage pools. ie if we can do
any automatic refresh so we're guaranteed always up2date we should. Not
all storage pools will support that, so we'd probably want to expose
some kind of capability info about storage pool backends to describe
their features to apps.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (3)
-
Chun Yan Liu -
Daniel P. Berrange -
Michal Privoznik