[virt-tools-list] [PATCH] python-virtinst: type inconsistencies in 'vcpus'

[Cole Robinson]

On 02/09/2011 05:35 AM, Satoru SATOH wrote:
> Hi,
> 
> I looked into this problem further and now feel certain that my previous
> patch (change type of '--vcpus' option) was correct and can fix it.
> 
> 
> Details:
> 
> I run virt-install with the arguments,
> 
> "--name foo --nodisks --ram 256 \
> --cdrom /srv/isos/rhel-server-5.6-i386-dvd.iso \
> --hvm --accelerate --vnc \
> --check-cpu \
> --vcpus 2"
> 
> and got the following traceback:
> 
> ...
> gdb) r "/root/virtinst-debug.py"
> Starting program: /usr/bin/python "/root/virtinst-debug.py"
> [Thread debugging using libthread_db enabled]
> Detaching after fork from child process 11189.
> ERROR    Traceback (most recent call last):
>   File "/root/virtinst-debug.py", line 25, in <module>
>     _vi.main()
>   File "/root/vi/vi.py", line 1259, in main
>     guest = build_guest_instance(conn, options)
>   File "/root/vi/vi.py", line 970, in build_guest_instance
>     cli.get_vcpus(options.vcpus, options.check_cpu, guest)
>   File "/usr/lib/python2.7/site-packages/virtinst/cli.py", line 733, in get_vcpus
>     "but performance will be poor. ") % (vcpus, cpu_num)
> TypeError: %d format: a number is required, not str
> None
> ssato at gescom%
> 
> (note: _vi.main is just an wrapper calls virt-install.main with the
> arguments set.)
> 
> 
> and get_vcpus() in /usr/lib/python2.7/site-packages/virtinst/cli.py
> looks like this:
> 
> def get_vcpus(vcpus, check_cpu, guest, image_vcpus=None):
>     if vcpus is None and image_vcpus is not None:
>         vcpus = int(image_vcpus)
> 
>     parse_vcpu_option(guest, vcpus, image_vcpus)
> 
>     conn = guest.conn
>     if check_cpu:
>         hostinfo = conn.getInfo()
>         cpu_num = hostinfo[4] * hostinfo[5] * hostinfo[6] * hostinfo[7]
>         if not vcpus <= cpu_num:
>             msg = _("You have asked for more virtual CPUs (%d) than there "
>                     "are physical CPUs (%d) on the host. This will work, "
>                     "but performance will be poor. ") % (vcpus, cpu_num)
> 
>             askmsg = _("Are you sure? (yes or no)")
> 
>             if not prompt_for_yes_or_no(msg, askmsg):
>                 nice_exit()
> 
> 
> The type of cpu_num is obviously int:
> 
> $ python -c "import libvirt; c = libvirt.open(None); print c.getInfo()[4:]"
> [1, 1, 1, 2]
> $
> 
> however vcpus is not int always. Its type is str in some cases (came
> from the --vcpus option), and therefore, the error "TypeError: %d
> format:..." occured, I think.
> 
> 
> - satoru
> 
> On Mon, Feb 07, 2011 at 02:55:14PM +0900, Satoru SATOH wrote:
>> Hi,
>>
>> I encountered a possible bug in python-virtinst (0.500.5-1.fc14) causing
>> the type error such as:
>>
>> # virt-install --connect=qemu:///system --name=rhel-5-4-vm-0 --ram=256 \
>> --arch=i686 --vcpu=2 --keymap=en-us --os-type=linux \
>> --location=http://192.168.151.1/contents/RHEL/5/4/Server/i386/ \
>> --os-variant=rhel5 --disk \
>> path=/var/lib/libvirt/images/rhel-5-4-vm-0/disk-1.qcow2,bus=virtio,format=qcow2,cache=none \
>> --network=network:net-1,model=virtio \
>> --network=network:net-1,model=virtio \
>> --network=network:net-2,model=virtio \
>> --network=network:net-2,model=virtio \
>> --extra-args="ks=http://192.168.151.1/autoinst/rhel-5-4-i386-min.ks.cfg \
>> ksdevice=eth0" --check-cpu --hvm --accelerate --vnc --noreboot \
>> --noautoconsole --wait=20
>> ERROR    %d format: a number is required, not str
>>
>>
>> I'm not really sure about the cause of this issue but it seems the
>> following patch may fix it.
>>
>> This patch tries to fix the inconsistencies of type 'vcpu' in cli.py of
>> python-virtinst.
>>

Thanks for the detailed report. FYI in the future, running virt-install with
the --debug flag should print the full backtrace, rather than needing to use gdb.

As you say, in latest virt-install, --vcpus is now a string rather than a
plain int. This is deliberate to facilitate new functionality like specifying
maxvcpus and socket/core/thread topology. So the correct fix is to juss tweak
--check-cpu string format.

I've fixed this upstream now:

http://hg.fedorahosted.org/hg/python-virtinst/rev/cdfd4ebd2233

Thanks,
Cole

>>
>>
>> --- /usr/lib/python2.7/site-packages/virtinst/cli.py.org	2011-02-07 14:19:21.554513828 +0900
>> +++ /usr/lib/python2.7/site-packages/virtinst/cli.py	2011-02-07 14:19:55.955030458 +0900
>> @@ -513,7 +513,7 @@
>>              failed = True
>>  
>>  def vcpu_cli_options(grp):
>> -    grp.add_option("", "--vcpus", type="string", dest="vcpus",
>> +    grp.add_option("", "--vcpus", type="int", dest="vcpus",
>>          help=_("Number of vcpus to configure for your guest. Ex:\n"
>>                 "--vcpus 5\n"
>>                 "--vcpus 5,maxcpus=10\n"
> 
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list