Bug 708892 - virt-manager should check if allocated cpu value equal to sockets*threads*cores
Summary: virt-manager should check if allocated cpu value equal to sockets*threads*cores
Keywords:
Status: CLOSED UPSTREAM
Alias: None
Product: Virtualization Tools
Classification: Community
Component: virt-manager
Version: unspecified
Hardware: x86_64
OS: Linux
high
medium
Target Milestone: ---
Assignee: Cole Robinson
QA Contact:
URL:
Whiteboard:
Depends On: 725271
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-05-30 03:37 UTC by Min Zhan
Modified: 2014-07-06 19:31 UTC (History)
9 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2014-02-01 14:15:36 UTC
Embargoed:


Attachments (Terms of Use)
cpu info (9.18 KB, image/png)
2011-05-30 03:38 UTC, Min Zhan
no flags Details

Description Min Zhan 2011-05-30 03:37:08 UTC
Description of problem:
No error display when setting allocated cpu value is not same as sockets*threads*cores. Also the guest will take effect with socket, threads and cores setting no matter how many cpu are.

Version-Release number of selected component (if applicable):
libvirt-0.9.1-1.el6.x86_64
virt-manager-0.8.6-4.el6.noarch


How reproducible:
Always

Steps to Reproduce:
1.In virt-manager, choose a shutdown guest, changes cpu allocation as 4
2.Tick Manually set cpu topology, set socket as 1, threads as 1, cores as 1. click Apply.
3.Start the guest. login and check the cpu info
# cat /proc/cpuinfo |more
there is only 1 processor listed just as attachment. As we see it is not 4. 
  
Actual results:
As above

Expected results:
As we know, the cpu number should be equal to sockets*threads*cores. so when user manually set cpu topology, pls check their values and cpu number; and pop up an error message to inform if they are not consistent.

Additional info:

Comment 1 Min Zhan 2011-05-30 03:38:01 UTC
Created attachment 501690 [details]
cpu info

Comment 2 Cole Robinson 2011-07-24 22:32:10 UTC
There are some issues here across the stack.

For starters, qemu -smp has some strange semantics. doing -smp 1,sockets=4,cores=4,threads=4 exposes 1 logical CPU to the guest that claims via cpuid that is has 4 cores, each with 4 threads. -smp X is basically short hand for -smp sockets=X, so if sockets= is specified, I think the oldstyle value should be ignored.

Next, libvirt tickles this qemu weirdness by _always_ specifying the oldstyle -smp X value, even if topology is specified. What libvirt should be doing here is only ever specifying topology if the qemu binary supports it, otherwise use the oldstyle option, so it doesn't need to depend on how qemu interprets back compat options.

Next, libvirt shouldn't allow an ambiguous XML configuration regarding topology. Example, what guest CPU config does the following XML represent?

<domain>
  ...
  <vcpus>3</vcpus>
  <cpu>
    <topology sockets='2' cores='2' threads='2'/>
  </cpu>
  ...
</domain>

It is unclear, and libvirt shouldn't accept that config. What I think libvirt should do is make sockets= a readonly attribute that just mirrors the old style <vcpus> value. Clarify the docs that for HVM, <vcpus> is not 'logical guest cpus' or anything like that, it represents a distinct socket to the guest. That should allow people to edit <vcpus> like normal. And my understanding is that HVs that do CPU hotplug do it at the socket level anyways, so setvcpus commands still retain the correct semantics.

(I'll be filing bugs for the above issues shortly)

If libvirt fixes those issues, then virt-manager will need to just make the 'sockets' value a non-editable text field.

Comment 3 Cole Robinson 2011-07-24 22:40:54 UTC
qemu -smp 1,cores=2 ambiguity: https://bugzilla.redhat.com/show_bug.cgi?id=725268

libvirt shouldn't generate old style -smp X: https://bugzilla.redhat.com/show_bug.cgi?id=725269

libvirt XML ambiguity: https://bugzilla.redhat.com/show_bug.cgi?id=725271

Comment 4 Cole Robinson 2011-07-24 22:44:39 UTC
Also virt-install may need to be tweaked to not generate an ambiguous XML config via CLI options.

Comment 5 Cole Robinson 2011-09-27 15:33:22 UTC
Since the libvirt issues still aren't fixed, deferring this to 6.3

Comment 6 Bhavna Sarathy 2011-10-24 17:24:08 UTC
Are the changes limited to libvirt and virt-manager to solve this issue?    Or is qemu-kvm changes necessary?

Comment 7 Cole Robinson 2011-10-24 17:37:52 UTC
The QEMU isn't strictly required to be fixed, so it's just libvirt and virt-manager

Comment 9 Cole Robinson 2012-01-29 21:36:46 UTC
Since libvirt validates this now, it's not really that urgent to fix in virt-manager. Dropping ACKs and moving to upstream tracker.

Comment 10 Cole Robinson 2014-02-01 14:15:36 UTC
Upstream now will enforce in the UI that maxvcpus and cpu topology make sense if both are specified.


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