Re: [virt-tools-list] [virt-manager] [PATCH 0/5] Use pylint/pycodestyle modules

2018-03-05 Thread Radostin Stoyanov


On 03/03/18 21:09, Cole Robinson wrote:
> On 03/02/2018 03:01 AM, Radostin Stoyanov wrote:
>> Radostin Stoyanov (5):
>>   pylint: Use pylint.lint module
>>   pylint: Silence inconsistent-return-statements
>>   pylint: Resolve logging-not-lazy
>>   pylint: Resolve consider-using-enumerate
>>   pycodestyle: Use module instead of executable
>>
>>  setup.py| 33 -
>>  tests/clitest.py|  2 +-
>>  tests/pylint.cfg|  2 +-
>>  virtManager/addhardware.py  |  4 ++--
>>  virtManager/clone.py|  2 +-
>>  virtManager/connectauth.py  |  2 +-
>>  virtManager/connection.py   |  4 ++--
>>  virtManager/graphwidgets.py | 20 +++-
>>  virtinst/installer.py   |  2 +-
>>  virtinst/interface.py   |  4 ++--
>>  virtinst/storage.py |  4 ++--
>>  virtinst/urlfetcher.py  |  6 +++---
>>  12 files changed, 43 insertions(+), 42 deletions(-)
>>
> ACK and pushed, thanks! How many hits does
> inconsistent-return-statements trigger? I'd take a patch to fix that
Not that many, there are about 90 inconsistent-return-statements.
However most of them are incorrect.
Currently there are some cases that are not recognised by pylint. (See
https://github.com/PyCQA/pylint/issues/1782 and
https://github.com/home-assistant/home-assistant/pull/12274).
Lets wait for this bug to be fixed before modifying virt-manager's code.

Radostin
> Thanks,
> Cole

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH 0/2] Show IP address in virt-manager

2018-03-05 Thread Michal Privoznik
On 03/02/2018 03:58 PM, Dylan Stephano-Shachter wrote:
> That is interesting. Could you possibly run virt-manager with --debug and
> send me any stacktraces you see?

[Please don't top post on technical lists.]

> 
> On Mar 2, 2018 7:55 AM, "Michal Privoznik"  wrote:
> 
>> On 03/02/2018 07:27 AM, Dylan Stephano-Shachter wrote:
>>> I have added a feature to show a VM's first IP address next to the VM
>> state (Running, etc.). This feature can be toggled in the preferences menu
>> and is disabled by default. It uses the qemu-guest agent to query the IP
>> address.
>>>
>>> Dylan Stephano-Shachter (2):
>>>   show the first ip address of running vms
>>>   add preferences option to enable 'show first ip'
>>>
>>>  data/org.virt-manager.virt-manager.gschema.xml |  6 ++
>>>  ui/preferences.ui  | 15 +++
>>>  virtManager/config.py  |  8 
>>>  virtManager/domain.py  | 16 
>>>  virtManager/engine.py  |  7 +++
>>>  virtManager/manager.py | 19 +++
>>>  virtManager/preferences.py |  9 +
>>>  7 files changed, 76 insertions(+), 4 deletions(-)
>>>
>>
>> Interestingly, this made a machine I have not to be displayed in the
>> list of domains. The domain is running, qemu-ga is installed and running
>> and domain has an IP address:
>>
>>
>> virsh # domifaddr fedora --source agent
>>  Name   MAC address  Protocol Address
>> 
>> ---
>>  lo 00:00:00:00:00:00ipv4 127.0.0.1/8
>>  -  -ipv6 ::1/128
>>  ens3   52:54:00:a4:6f:91ipv4 192.168.122.37/24
>>  -  -ipv6 fe80::ee48:d373:fc65:fce0/64
>>  virbr0 52:54:00:1f:be:17ipv4 192.168.124.1/24
>>  virbr0-nic 52:54:00:1f:be:17N/A  N/A


I've managed to reproduce and here's the interesting part of the debug output:

[Mon, 05 Mar 2018 10:50:43 virt-manager 12584] DEBUG (connection:1197) 
domain=fedora status=Running added
[Mon, 05 Mar 2018 10:50:43 virt-manager 12584] DEBUG (cli:258) Uncaught 
exception:
Traceback (most recent call last):
  File "/home/zippy/work/virt-manager.git/virtManager/manager.py", line 600, in 
vm_added
row = self._build_row(None, vm)
  File "/home/zippy/work/virt-manager.git/virtManager/manager.py", line 671, in 
_build_row
ipaddr = vm.get_first_ip_addr()
  File "/home/zippy/work/virt-manager.git/virtManager/domain.py", line 1093, in 
get_first_ip_addr
if iface != 'lo' and IP_REGEX.match(ifaces[iface]['addrs'][0]['addr']):
TypeError: 'NoneType' object is not subscriptable

Traceback (most recent call last):
  File "/home/zippy/work/virt-manager.git/virtManager/manager.py", line 600, in 
vm_added
row = self._build_row(None, vm)
  File "/home/zippy/work/virt-manager.git/virtManager/manager.py", line 671, in 
_build_row
ipaddr = vm.get_first_ip_addr()
  File "/home/zippy/work/virt-manager.git/virtManager/domain.py", line 1093, in 
get_first_ip_addr
if iface != 'lo' and IP_REGEX.match(ifaces[iface]['addrs'][0]['addr']):
TypeError: 'NoneType' object is not subscriptable

Michal

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [virt-manager PATCH] Allow installation of i686 guests on x86_64 machines

2018-03-05 Thread Daniel P . Berrangé
On Sat, Mar 03, 2018 at 04:21:37PM -0500, Cole Robinson wrote:
> On 03/03/2018 04:03 PM, Martin Kletzander wrote:
> > On Sat, Mar 03, 2018 at 03:53:59PM -0500, Cole Robinson wrote:
> >> On 03/01/2018 03:52 AM, Martin Kletzander wrote:
> >>> Do that by changing the code that was disabling it into code that
> >>> just warns in
> >>> such case.
> >>>
> >>> Signed-off-by: Martin Kletzander 
> >>> ---
> >>>  virtManager/create.py | 10 --
> >>>  1 file changed, 4 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/virtManager/create.py b/virtManager/create.py
> >>> index 0a73309372d9..9a40aec4a068 100644
> >>> --- a/virtManager/create.py
> >>> +++ b/virtManager/create.py
> >>> @@ -495,6 +495,10 @@ class vmmCreate(vmmGObjectUI):
> >>>  msg = _("Failed to setup UEFI for AArch64: %s\n"
> >>>  "Install options are limited.") % e
> >>>  self._show_arch_warning(msg)
> >>> +    elif (self._capsinfo.arch == "i686" and
> >>> +  self.conn.caps.host.cpu.arch == "x86_64"):
> >>> +    msg = _("You are installing 32bit guest on 64bit host")
> >>> +    self._show_arch_warning(msg)
> >>>
> >>>  # Install Options
> >>>  method_tree = self.widget("method-tree")
> >>> @@ -824,12 +828,6 @@ class vmmCreate(vmmGObjectUI):
> >>>  if guest.os_type == self._capsinfo.os_type:
> >>>  archs.append(guest.arch)
> >>>
> >>> -    # Combine x86/i686 to avoid confusion
> >>> -    if (self.conn.caps.host.cpu.arch == "x86_64" and
> >>> -    "x86_64" in archs and "i686" in archs):
> >>> -    archs.remove("i686")
> >>> -    archs.sort()
> >>> -
> >>>  prios = ["x86_64", "i686", "aarch64", "armv7l", "ppc64",
> >>> "ppc64le",
> >>>  "s390x"]
> >>>  if self.conn.caps.host.cpu.arch not in prios:
> >>>
> >>
> >> The idea behind hiding the option is 1) it should be rare that someone
> >> actually wants their VM to present a 32bit cpu on 64bit host, 2) hiding
> >> it means when only x86_64 qemu is installed we can entirely hide the
> >> 'advanced options' expander.
> >>
> >> Example: on rhel7 x86_64, libvirt advertises arch=i686 and arch=x86_64
> >> for /usr/libexec/qemu-kvm. If we hide i686, there's only one arch option
> >> available, and virt-manager will hide the entire 'advanced options'
> >> expander. This is ideal IMO, otherwise users might go clicking, see the
> >> i686 option, misinterpret it to mean OS arch (which in the context of
> >> virt I've seen people mistake many times), set things to i686 needlessly.
> >>
> > 
> > Sure I get that, that's why I kept it as a warning.
> > 
> >> Are there benefits I'm missng of doing arch=i686 on an x86_64 host
> >> exactly? Doesn't it just map to using qemu32 as the default CPU?
> >>
> > 
> > To be honest, I'm not sure if qemu32 CPU type means also 32bit CPU or
> > just limited instruction set and how it looks from the guest OS POV.
> > The reason for this patch emerged simply when we needed to test libvirt
> > build failure on 32bit machine and it was just easier to install 32bit
> > VM than setting up a cross-build.  Or rather installing all 32bit
> > dependencies properly.  I'd love to hear how to do this properly, I'm
> > not saying this patch is needed =)
> 
> I think this is an example of that arch confusion actually. You can
> install a 32 bit OS into a 64 bit machine, doesn't matter if the CPU is
> x86_64, and building in that should reproduce the 32bit build error

Yes, in fact all x86_64 CPUs start off as stupid 16-bit CPUs when first
powered on. The guest OS has to put them into 32-bit or 64-bit mode
themselves. So by installing a 32-bit OS you'll simply never see any
of the things that makes it capable of 64-bit.  If you really absolutely
want to block use of 64-bit mode though, you could do that by simply
telling libvirt to block the "lm" feature flag from the emulated CPU.
That'll stop the guest OS detecting availability of 64-bit and thus
it'll only be 32-bit.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list