On Thu, Dec 10, 2015 at 10:25:28AM +0000, Daniel P. Berrange wrote:
> On Thu, Dec 10, 2015 at 11:45:36AM +0530, Bharata B Rao wrote:
> > Prevent guests from booting with CPU topologies that have partially
> > filled CPU cores or can result in partially filled CPU cores after CPU
> > hotplug like
> >
> > -smp 15,sockets=1,cores=4,threads=4,maxcpus=16 or
> > -smp 15,sockets=1,cores=4,threads=4,maxcpus=17 or
> >
> > Signed-off-by: Bharata B Rao <bhar...@linux.vnet.ibm.com>
> > ---
> > vl.c | 13 +++++++++++++
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/vl.c b/vl.c
> > index 525929b..e656f53 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -1252,6 +1252,19 @@ static void smp_parse(QemuOpts *opts)
> > smp_cores = cores > 0 ? cores : 1;
> > smp_threads = threads > 0 ? threads : 1;
> >
> > + if (smp_cpus % smp_threads) {
> > + error_report("cpu topology: "
> > + "smp_cpus (%u) should be multiple of threads
> > (%u)",
> > + smp_cpus, smp_threads);
> > + exit(1);
> > + }
> > +
> > + if (max_cpus % smp_threads) {
> > + error_report("cpu topology: "
> > + "maxcpus (%u) should be multiple of threads (%u)",
> > + max_cpus, smp_threads);
> > + exit(1);
> > + }
> > }
>
> Adding this seems like it has a pretty high chance of causing regression,
> ie preventing previously working guests from booting with new QEMU. I
> know adding the check makes sense from a semantic POV, but are we willing
> to risk breaking people with such odd configurations ?
I wasn't sure about how much risk that would be and hence in my older
version of PowerPC CPU hotplug patchset, I indeed supported such topologies:
https://lists.gnu.org/archive/html/qemu-ppc/2015-09/msg00102.html
But the code indeed looked ugly to support such special case.
There was some discussion about this recently here:
http://lists.gnu.org/archive/html/qemu-devel/2015-12/msg00396.html
from where I sensed that it may be ok to dis-allow such topologies.
Regards,
Bharata.
