[virt-manager PATCH 0/2] Couple of bhyve related improvements

[Roman Bogorodskiy]

  Cole Robinson wrote:

> On 2/6/21 8:18 AM, Roman Bogorodskiy wrote:
> > Roman Bogorodskiy (2):
> >   virtinst: prefer SATA bus for bhyve
> >   virtinst: bhyve: properly configure loader
> > 
> >  virtinst/connection.py   |  2 ++
> >  virtinst/devices/disk.py |  3 +++
> >  virtinst/domain/os.py    | 16 ++++++++++++++++
> >  3 files changed, 21 insertions(+)
> > 
> 
> Thanks for the patches. We need to add test coverage though, which since
> these will be the first bhyve tests will take some plumbing.
> 
> * Drop bhyve `virsh capabilities` in tests/data/capabilities/bhyve.xml
> * Drop bhyve `virsh domcapabilities` in
> tests/data/capabilities/bhyve-domcaps.xml
> * In tests/utils.py:_URIs, add self.bhyve = _m("bhyve:///") +
> _caps("bhyve.xml") + _domcaps("bhyve-domcaps.xml")
> * in tests/test_cli.py add something like this:
> 
> ```
> ########################
> 
> # bhyve specific tests #
> 
> ########################
> 
> c = vinst.add_category("bhyve", "--name foobhyve --noautoconsole
> --connect " + utils.URIs.bhyve)
> c.add_compare("--os-variant fedora27", "bhyve-default-f27")
> ```
> 
> * Run the test suite, verify the generated
> ./tests/data/cli/compare/virt-install-bhyve-default-f27.xml looks sane.
> 
> 
> That will all be the first commit. The next patches should cause test
> output changes, use `pytest --regenerate-output` to refresh the test XML
> and commit the difference.
> 
> Also check `pytest --cov` doesn't regress
> 
> Thanks,
> Cole
> 
> 
> 
> - Cole
> 

Thanks for the detailed instruction, will do that.

BTW, I noticed a strange issue with test_cli.py which I was hoping to
fix later, but apparently need to figure out first for smooth testing:

Many tests are failing with the following trace:

Traceback (most recent call last):                                                                                                                                                                                  
  File "/usr/home/novel/code/virt-manager/tests/test_cli.py", line 228, in _launch_command                                                                                                                          
    ret = virtinstall.main(conn=conn)                                                                                                                                                                               
  File "/usr/home/novel/code/virt-manager/virtinst/virtinstall.py", line 1145, in main                    
    guest, installer = build_guest_instance(conn, options)                                                                                                                                                          
  File "/usr/home/novel/code/virt-manager/virtinst/virtinstall.py", line 598, in build_guest_instance                                                                                                               
    cli.validate_disk(disk)                                                                                                                                                                                         
  File "/usr/home/novel/code/virt-manager/virtinst/cli.py", line 377, in validate_disk                    
    check_inuse_conflict()                                                                                                                                                                                          
  File "/usr/home/novel/code/virt-manager/virtinst/cli.py", line 359, in check_inuse_conflict                                                                                                                       
    names = dev.is_conflict_disk()                                                                        
  File "/usr/home/novel/code/virt-manager/virtinst/devices/disk.py", line 832, in is_conflict_disk        
    read_only=self.read_only)                                                                                                                                                                                       
  File "/usr/home/novel/code/virt-manager/virtinst/devices/disk.py", line 322, in path_in_use_by          
    checkpath = disk.get_source_path()                                                                                                                                                                              
  File "/usr/home/novel/code/virt-manager/virtinst/devices/disk.py", line 629, in get_source_path                                                                                                                   
    self._resolve_storage_backend()                                                                                                                                                                                 
  File "/usr/home/novel/code/virt-manager/virtinst/devices/disk.py", line 616, in _resolve_storage_backend 
    self.conn, path)                                                                                                                                                                                                
  File "/usr/home/novel/code/virt-manager/virtinst/diskbackend.py", line 143, in manage_path              
    if not conn.support.conn_storage():                                                                                                                                                                             
ReferenceError: weakly-referenced object no longer exists 

Traceback is always similar. I tried to find a point where it loses the
reference using pdb and it happens to be here:

  File "/usr/home/novel/code/virt-manager/virtinst/devices/disk.py", line 322, in path_in_use_by          
    checkpath = disk.get_source_path()                                                                                                                                                                              
  File "/usr/home/novel/code/virt-manager/virtinst/devices/disk.py", line 629, in get_source_path                                                                                                                   
    self._resolve_storage_backend()                                                                                                                                                                                 

In path_in_use_by() I can still access 'conn', but in get_source_path()
self.conn is already lost. I was able to add a workaround like:

diff --git a/virtinst/devices/disk.py b/virtinst/devices/disk.py
index a8971581..7e609914 100644
--- a/virtinst/devices/disk.py
+++ b/virtinst/devices/disk.py
@@ -319,6 +319,7 @@ class DeviceDisk(Device):
                     continue

             for disk in vm.devices.disk:
+                disk.conn = conn
                 checkpath = disk.get_source_path()
                 if checkpath in vols and vm.name not in ret:
                     # VM uses the path indirectly via backing store

But that doesn't seem like a proper fix.
Also wondering why it doesn't show up for others? Doesn't seem to have
anything FreeBSD specific happening here.

Thanks,

Roman Bogorodskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20210209/7d2c9792/attachment.sig>