Bug 1661871 - python bindings wrongly using str for APIs with RBufferOut/FBuffer on Python 3
Summary: python bindings wrongly using str for APIs with RBufferOut/FBuffer on Python 3
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Red Hat Enterprise Linux 8
Classification: Red Hat
Component: libguestfs
Version: 8.0
Hardware: Unspecified
OS: Unspecified
medium
unspecified
Target Milestone: rc
: 8.0
Assignee: Pino Toscano
QA Contact: YongkuiGuo
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2018-12-24 06:47 UTC by zonglin jiang
Modified: 2019-06-14 01:06 UTC (History)
13 users (show)

Fixed In Version: libguestfs-1.38.4-10.module+el8+2709+40ed2f2c
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2019-06-14 01:06:59 UTC
Type: Bug
Target Upstream Version:


Attachments (Terms of Use)
debug log (25.92 KB, text/plain)
2018-12-24 06:47 UTC, zonglin jiang
no flags Details
virt-manager_debug.log (17.02 KB, text/plain)
2019-01-28 07:07 UTC, YongkuiGuo
no flags Details
icon extract script (796 bytes, text/x-python)
2019-01-28 09:29 UTC, Pino Toscano
no flags Details
icon extract script v2 (837 bytes, text/x-python)
2019-01-28 10:16 UTC, Pino Toscano
no flags Details
extract script v2 output (62.50 KB, text/plain)
2019-01-28 10:49 UTC, YongkuiGuo
no flags Details
icon extract script v3 (945 bytes, text/x-python)
2019-01-29 15:17 UTC, Pino Toscano
no flags Details
entire virt-manager debug log (1.53 MB, text/plain)
2019-01-30 03:01 UTC, YongkuiGuo
no flags Details
icon extract script v3 output (63.50 KB, text/plain)
2019-01-30 03:06 UTC, YongkuiGuo
no flags Details
icon extract script v4 (946 bytes, text/x-python)
2019-01-30 13:35 UTC, Pino Toscano
no flags Details

Description zonglin jiang 2018-12-24 06:47:50 UTC
Created attachment 1516476 [details]
debug log

Description of problem:
Icon created by libguestfs-tools cannot been displayed on virt-manager

Version-Release number of selected component (if applicable):
virt-manager-virt-manager-2.0.0-1.el8.noarch
qemu-kvm-2.12.0-45.module+el8+2313+d65431a0.x86_64 
libvirt-4.5.0-15.module+el8+2285+e990ac42.x86_64 
python3-libvirt-4.5.0-1.module+el8+2173+537e5cb5.x86_64
kernel-4.18.0-48.el8.x86_64
libguestfs-tools-c-1.38.4-8.module+el8+2564+79985af8.x86_64
libguestfs-tools-1.38.4-8.module+el8+2564+79985af8.noarch
python3-libguestfs-1.38.4-8.module+el8+2564+79985af8.x86_64

How reproducible:
100%

Steps to Reproduce
1. Make sure you installed the followign packages
libvirt
virt-manager
libguestfs-tools-c
libguestfs-tools
python-libguestfs ( in RHEL8.0, I can only use python3-libguestfs)
2.Make sure you have at least one rhel guest and one windows guest installed on you host.
3.Launch virt-manager .

Actual results:
Redhat icon and windows icon are not displayed on virt-manager

Expected results:
Wait for a few minutes, check redhat icon and windows icon are displayed on virt-manager
for guests.

Additional info:

[Mon, 24 Dec 2018 14:40:46 virt-manager 17340] ERROR (inspection:185) qemu:///system:rhel7.6: exception while processing
Traceback (most recent call last):
  File "/usr/share/virt-manager/virtManager/inspection.py", line 182, in _process_vm
    data = self._inspect_vm(conn, vm)
  File "/usr/share/virt-manager/virtManager/inspection.py", line 270, in _inspect_vm
    icon = g.inspect_get_icon(root, favicon=0, highquality=1)
  File "/usr/lib64/python3.6/site-packages/guestfs.py", line 4650, in inspect_get_icon
    r = libguestfsmod.inspect_get_icon(self._o, root, favicon, highquality)
UnicodeDecodeError: 'utf-8' codec can't decode byte 0x89 in position 0: invalid start byte

Comment 1 Richard W.M. Jones 2018-12-24 10:25:31 UTC
This is part of the general problem with libguestfs bindings and Python strings, but
yes it's a bug.

Comment 3 Pino Toscano 2019-01-23 12:07:08 UTC
This was fixed upstream with
https://github.com/libguestfs/libguestfs/commit/0ee02e0117527b86a31b2a88a14994ce7f15571f
which is in libguestfs >= 1.41.1.

This was a Python 3 issue (str vs bytes) of the Python bindings, so retitling accordingly.

Comment 4 Pino Toscano 2019-01-23 12:08:57 UTC
(In reply to Red Hat Bugzilla Rules Engine from comment #2)
> * What is the risk with putting this into the release vs. doingit in the
> next release? (For example, is this a regression?)

Yes: virt-manager (even though it is deprecated) will not show the icons of any guest (like it does on RHEL 7, for example)
In general, this is a problem that affects part of the Python bindings when built with Python 3: some of the APIs (9 directly, and 5 indirectly) will basically not work at all. One of them affects the icon extraction from the guest. Unfortunately it is hard to say what may use the others.

> * Is this related to an MVP or Layered Product request, if so what?

Not to my knowledge.

> * Do you have signoff from your Dev/QE/PM leads on the status of this as an
> exception for the team?

Rich? Ming? Martin?

Comment 5 Whitney Chadwick 2019-01-23 16:33:08 UTC
granted exception+ per review of RHEL Team

Comment 7 YongkuiGuo 2019-01-28 07:03:06 UTC
Verified with package:
libguestfs-1.38.4-10.module+el8+2709+40ed2f2c.x86_64
libvirt-4.5.0-18.module+el8+2691+dc742e5d.x86_64
kernel-4.18.0-60.el8.x86_64
qemu-kvm-2.12.0-57.module+el8+2683+02b3b955.x86_64
python3-libguestfs-1.38.4-10.module+el8+2709+40ed2f2c.x86_64


Steps:

1. Install related packages: libguestfs,virt-manager,python3-libguestfs,...
2. Install rhel8 and windows 7 guests on rhel8 host.
3. 
# virt-manager --debug
...
[Mon, 28 Jan 2019 14:53:04 virt-manager 21508] DEBUG (inspection:286) qemu:///system:rhel8: detected operating system: linux rhel 8.0 (Red Hat Enterprise Linux 8.0 Beta (Ootpa))
[Mon, 28 Jan 2019 14:53:04 virt-manager 21508] DEBUG (inspection:287) hostname: localhost.localdomain
[Mon, 28 Jan 2019 14:53:04 virt-manager 21508] DEBUG (inspection:289) icon: 7046 bytes
[Mon, 28 Jan 2019 14:53:04 virt-manager 21508] DEBUG (inspection:291) # apps: 1238
...
[Mon, 28 Jan 2019 14:53:06 virt-manager 21508] DEBUG (inspection:286) qemu:///system:win7: detected operating system: windows windows 6.1 (Windows 7 Starter)
[Mon, 28 Jan 2019 14:53:06 virt-manager 21508] DEBUG (inspection:287) hostname: REDHATQ-OOMI6GN
[Mon, 28 Jan 2019 14:53:06 virt-manager 21508] ERROR (manager:74) Error loading inspection icon data
Traceback (most recent call last):
  File "/usr/share/virt-manager/virtManager/manager.py", line 71, in _get_inspection_icon_pixbuf
    pb.close()
GLib.GError: gdk-pixbuf-error-quark: Unrecognized image file format (3)

The rhel8 icon can be displayed on virt-manager, but failed to inspect the windows icon. 
The gdk-pixbuf2 package is installed on rhel8 host.

Comment 8 YongkuiGuo 2019-01-28 07:07:44 UTC
Created attachment 1524153 [details]
virt-manager_debug.log

Comment 9 Pino Toscano 2019-01-28 08:40:04 UTC
(In reply to YongkuiGuo from comment #7)
> The rhel8 icon can be displayed on virt-manager, but failed to inspect the
> windows icon. 
> The gdk-pixbuf2 package is installed on rhel8 host.

Is libguestfs-inspect-icons installed too?

Comment 10 YongkuiGuo 2019-01-28 08:43:34 UTC
(In reply to Pino Toscano from comment #9)
> (In reply to YongkuiGuo from comment #7)
> > The rhel8 icon can be displayed on virt-manager, but failed to inspect the
> > windows icon. 
> > The gdk-pixbuf2 package is installed on rhel8 host.
> 
> Is libguestfs-inspect-icons installed too?

Yes. libguestfs-inspect-icons-1.38.4-10.module+el8+2709+40ed2f2c.noarch.

Comment 11 Pino Toscano 2019-01-28 09:29:38 UTC
Created attachment 1524170 [details]
icon extract script

Simple icon extraction script.

Run it like `python3 icon.py windows-image`, and it will save the icon of the guest as icon.png in the current directory.

Comment 12 Pino Toscano 2019-01-28 09:32:49 UTC
YongkuiGuo,

can you please try the attached icon extraction script on the disk of the Windows guest?
It should not fail, and save the icon as "icon.png".

Comment 13 YongkuiGuo 2019-01-28 10:06:51 UTC
It failed and I tried to debug the error line.
# python3 -m pdb icon.py Win7-32-hvm.raw
...
(Pdb) 
> /home/icon.py(23)<module>()
-> g.mount_ro(mps[device], device)
(Pdb) pp mps[device]
'/dev/sda1'
(Pdb) pp device
'/'
(Pdb) n
RuntimeError: mount: unsupported filesystem type    




The following steps can be run successfully.

# guestfish -a Win7-32-hvm.raw 
><fs> set-program virt-trickery
><fs> run
><fs> inspect-os
/dev/sda1
><fs> mount /dev/sda1 /
><fs> list-filesystems
/dev/sda1: ntfs
><fs>

Comment 14 Pino Toscano 2019-01-28 10:16:43 UTC
Created attachment 1524181 [details]
icon extract script v2

v2 of the script, working with libguestfs-winsupport in RHEL.

Comment 15 YongkuiGuo 2019-01-28 10:33:01 UTC
# python3 icon.py Win7-32-hvm.raw 
# ll | grep  icon.png 
-rw-r--r--.  1 root root           0 Jan 28 18:30 icon.png

The size of icon.png is zero byte.

Comment 16 Pino Toscano 2019-01-28 10:41:04 UTC
(In reply to YongkuiGuo from comment #15)
> # python3 icon.py Win7-32-hvm.raw 
> # ll | grep  icon.png 
> -rw-r--r--.  1 root root           0 Jan 28 18:30 icon.png
> 
> The size of icon.png is zero byte.

Hmm...
1) can you please run the script with LIBGUESTFS_TRACE=1 LIBGUESTFS_DEBUG=1, and attach its output?
2) if you copy/migrate the same guest on a RHEL 7 installation, is virt-manager able to show the icon?

Comment 17 YongkuiGuo 2019-01-28 10:49:30 UTC
Created attachment 1524190 [details]
extract script v2 output

Comment 18 YongkuiGuo 2019-01-28 10:50:55 UTC
> 2) if you copy/migrate the same guest on a RHEL 7 installation, is
> virt-manager able to show the icon?
Yes, virt-manager can be able to show the icon of Win7-32-hvm.raw on rhel7.6 host.

Comment 19 Pino Toscano 2019-01-29 15:15:46 UTC
(In reply to YongkuiGuo from comment #18)
> > 2) if you copy/migrate the same guest on a RHEL 7 installation, is
> > virt-manager able to show the icon?
> Yes, virt-manager can be able to show the icon of Win7-32-hvm.raw on rhel7.6
> host.

Hmm interesting:
- there are no "high quality" icons that libguestfs extracts from Windows guests (so highquality=True gives no results for any Windows guest)
- virt-manager already falls back on the low quality icon if no high quality icon is available; this logic has been in place for years
- the icon extraction logic in libguestfs has been the same for years

Can you please run virt-manager with LIBGUESTFS_TRACE=1 LIBGUESTFS_DEBUG=1, and attach the (long) resulting output?
Also, I will update the icon.py script with a logic closer to what virt-manager does.

Comment 20 Pino Toscano 2019-01-29 15:17:01 UTC
Created attachment 1524693 [details]
icon extract script v3

v3 of the script, falling back to low quality icons.

Comment 21 YongkuiGuo 2019-01-30 03:01:00 UTC
Created attachment 1524826 [details]
entire virt-manager debug log

Comment 22 YongkuiGuo 2019-01-30 03:06:49 UTC
Created attachment 1524827 [details]
icon extract script v3 output

Comment 23 YongkuiGuo 2019-01-30 03:11:49 UTC
> 
> Can you please run virt-manager with LIBGUESTFS_TRACE=1 LIBGUESTFS_DEBUG=1,
> and attach the (long) resulting output?

Done.

> Also, I will update the icon.py script with a logic closer to what
> virt-manager does.

The v3 script also cannot extract the icon. I have attached the output.

Comment 24 Pino Toscano 2019-01-30 13:35:38 UTC
Created attachment 1525036 [details]
icon extract script v4

v4 of the script, actually falling back to low quality icons...

Comment 25 Pino Toscano 2019-01-30 17:31:18 UTC
The missing Windows icon most probably is a bug in virt-manager; I just sent a patch upstream to fix this:
https://www.redhat.com/archives/virt-tools-list/2019-January/msg00084.html

Can you please try this patch? It should be possible to directly edit (as root) the Python script of the virt-manager installation (see `rpm -ql virt-manager`).

Comment 26 YongkuiGuo 2019-01-31 02:41:02 UTC
(In reply to Pino Toscano from comment #25)
> The missing Windows icon most probably is a bug in virt-manager; I just sent
> a patch upstream to fix this:
> https://www.redhat.com/archives/virt-tools-list/2019-January/msg00084.html
> 
> Can you please try this patch? It should be possible to directly edit (as
> root) the Python script of the virt-manager installation (see `rpm -ql
> virt-manager`).

Good news. I have tried this patch and it works well. I modified the v4 script according to this patch, and it also works.
...
[Thu, 31 Jan 2019 10:12:23 virt-manager 6020] DEBUG (inspection:288) qemu:///system:win7: detected operating system: windows windows 6.1 (Windows 7 Starter)
[Thu, 31 Jan 2019 10:12:23 virt-manager 6020] DEBUG (inspection:289) hostname: REDHATQ-OOMI6GN
[Thu, 31 Jan 2019 10:12:23 virt-manager 6020] DEBUG (inspection:291) icon: 3292 bytes
...

Comment 27 Pino Toscano 2019-01-31 09:54:03 UTC
(In reply to YongkuiGuo from comment #26)
> (In reply to Pino Toscano from comment #25)
> > The missing Windows icon most probably is a bug in virt-manager; I just sent
> > a patch upstream to fix this:
> > https://www.redhat.com/archives/virt-tools-list/2019-January/msg00084.html
> > 
> > Can you please try this patch? It should be possible to directly edit (as
> > root) the Python script of the virt-manager installation (see `rpm -ql
> > virt-manager`).
> 
> Good news. I have tried this patch and it works well. I modified the v4
> script according to this patch, and it also works.
> ...
> [Thu, 31 Jan 2019 10:12:23 virt-manager 6020] DEBUG (inspection:288)
> qemu:///system:win7: detected operating system: windows windows 6.1 (Windows
> 7 Starter)
> [Thu, 31 Jan 2019 10:12:23 virt-manager 6020] DEBUG (inspection:289)
> hostname: REDHATQ-OOMI6GN
> [Thu, 31 Jan 2019 10:12:23 virt-manager 6020] DEBUG (inspection:291) icon:
> 3292 bytes
> ...

Thanks for testing!  I just filed bug 1671278 for virt-manager.

Should this bug considered VERIFIED, then?

Comment 28 YongkuiGuo 2019-01-31 10:15:09 UTC
Verified this bug according to c#7 and c#26.

Comment 29 YongkuiGuo 2019-01-31 10:44:02 UTC
(In reply to Pino Toscano from comment #27)
> (In reply to YongkuiGuo from comment #26)
> > (In reply to Pino Toscano from comment #25)
> > > The missing Windows icon most probably is a bug in virt-manager; I just sent
> > > a patch upstream to fix this:
> > > https://www.redhat.com/archives/virt-tools-list/2019-January/msg00084.html
> > > 
> > > Can you please try this patch? It should be possible to directly edit (as
> > > root) the Python script of the virt-manager installation (see `rpm -ql
> > > virt-manager`).
> > 
> > Good news. I have tried this patch and it works well. I modified the v4
> > script according to this patch, and it also works.
> > ...
> > [Thu, 31 Jan 2019 10:12:23 virt-manager 6020] DEBUG (inspection:288)
> > qemu:///system:win7: detected operating system: windows windows 6.1 (Windows
> > 7 Starter)
> > [Thu, 31 Jan 2019 10:12:23 virt-manager 6020] DEBUG (inspection:289)
> > hostname: REDHATQ-OOMI6GN
> > [Thu, 31 Jan 2019 10:12:23 virt-manager 6020] DEBUG (inspection:291) icon:
> > 3292 bytes
> > ...
> 
> Thanks for testing!  I just filed bug 1671278 for virt-manager.
> 
> Should this bug considered VERIFIED, then?

np, done.

Comment 30 zhoujunqin 2019-02-01 03:21:00 UTC
Hi Pino,
I also tested this bug  with your new patch in Comment 25 with package virt-manager-2.0.0-2.el8.noarch
1. Modifying /usr/share/virt-manager/virtManager/inspection.py to 

        if filesystems_mounted:
            # string containing PNG data
            icon = g.inspect_get_icon(root, favicon=0, highquality=1)
            if icon is None or len(icon) == 0:
                # no high quality icon, try a low quality one
                icon = g.inspect_get_icon(root, favicon=0, highquality=0)
                if icon is None or len(icon) == 0:
                    icon = None
2. Prepare 1 windows guest and rhel guest.
3. Launch virt-manager, then finding that icons shows correctly for rhel7.6 and win7 guest, thanks.

Comment 31 Pino Toscano 2019-02-05 17:10:15 UTC
(In reply to zhoujunqin from comment #30)
> I also tested this bug  with your new patch in Comment 25 with package
> virt-manager-2.0.0-2.el8.noarch
> 1. Modifying /usr/share/virt-manager/virtManager/inspection.py to 
> 
>         if filesystems_mounted:
>             # string containing PNG data
>             icon = g.inspect_get_icon(root, favicon=0, highquality=1)
>             if icon is None or len(icon) == 0:
>                 # no high quality icon, try a low quality one
>                 icon = g.inspect_get_icon(root, favicon=0, highquality=0)
>                 if icon is None or len(icon) == 0:
>                     icon = None
> 2. Prepare 1 windows guest and rhel guest.
> 3. Launch virt-manager, then finding that icons shows correctly for rhel7.6
> and win7 guest, thanks.

Yes, the above is correct.


Note You need to log in before you can comment on or make changes to this bug.