Roman Bogorodskiy wrote:
> 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
Another workaround that works for me is:
diff --git a/virtinst/connection.py b/virtinst/connection.py
index 669cf715..24df9621 100644
--- a/virtinst/connection.py
+++ b/virtinst/connection.py
@@ -190,7 +190,7 @@ class VirtinstConnection(object):
except libvirt.libvirtError as e: # pragma: no cover
log.debug("Fetching domain XML failed: %s", e)
continue
- domains.append(Guest(weakref.proxy(self), parsexml=xml))
+ domains.append(Guest(self, parsexml=xml))
return domains
def _build_pool_raw(self, poolobj):
Roman Bogorodskiy
Attachment:
signature.asc
Description: PGP signature
