On Tue, Mar 05, 2013 at 10:34:08PM +0100, Laszlo Ersek wrote: > On 03/05/13 21:25, Eric Blake wrote: > > > On 03/04/2013 03:19 PM, Laszlo Ersek wrote: > >> Signed-off-by: Laszlo Ersek <ler...@redhat.com> > >> --- > >> GuestLogicalProcessorList *qmp_guest_get_vcpus(Error **errp) > >> { > >> +#if defined(__linux__) > > > >> + > >> + buf = g_strdup_printf("/sys/devices/system/cpu/cpu%ld/online", > >> + current); > >> + f = fopen(buf, "r"); > >> + if (f == NULL) { > >> + error_setg_errno(&local_err, errno, "fopen(\"%s\", \"r\")", > >> buf); > > > > NACK to this portion. If the file doesn't exist, but the > > /sys/devices/system/cpu/cpu%ld/ directory exists, then the cpu should be > > treated as always online, and not an error. In fact, on many machines, > > cpu0 does not have an online file precisely because it cannot be taken > > offline, even if the rest of the cpus can. > > The code always reports cpu0 online (without looking at the sysfs) and > never attempts to remove it (refuses any such requests), but see below. > > > It is also the case that on > > older kernels that did not support offline cpus (such as RHEL 5), there > > will be no per-cpu online file; but again, such kernels cannot support > > hot unplug, so all per-cpu directories imply which cpus are online. In > > other words, you need a sane fallback if the online file does not exist > > but the per-cpu directory does exist. > > In upstream "Documentation/cpu-hotplug.txt", there's > > Q: Why can't i remove CPU0 on some systems? > A: Some architectures may have some special dependency on a certain > CPU. > > For e.g in IA64 platforms we have ability to sent platform > interrupts to the OS. a.k.a Corrected Platform Error Interrupts > (CPEI). In current ACPI specifications, we didn't have a way to > change the target CPU. Hence if the current ACPI version doesn't > support such re-direction, we disable that CPU by making it > not-removable. > > In such cases you will also notice that the online file is missing > under cpu0. > > Q: Is CPU0 removable on X86? > A: Yes. If kernel is compiled with CONFIG_BOOTPARAM_HOTPLUG_CPU0=y, > CPU0 is removable by default. Otherwise, CPU0 is also removable by > kernel option cpu0_hotplug. > > But some features depend on CPU0. Two known dependencies are: > > 1. Resume from hibernate/suspend depends on CPU0. Hibernate/suspend > will fail if CPU0 is offline and you need to online CPU0 before > hibernate/suspend can continue. > 2. PIC interrupts also depend on CPU0. CPU0 can't be removed if a > PIC interrupt is detected. > > It's said poweroff/reboot may depend on CPU0 on some machines > although I haven't seen any poweroff/reboot failure so far after > CPU0 is offline on a few tested machines. > > Please let me know if you know or see any other dependencies of > CPU0. > > If the dependencies are under your control, you can turn on CPU0 > hotplug feature either by CONFIG_BOOTPARAM_HOTPLUG_CPU0 or by kernel > parameter cpu0_hotplug. > > --Fenghua Yu <fenghua...@intel.com> > > I got this wrong in the series. The get method always reports an online > CPU#0 (without even looking at sysfs), plus 'set' always refuses an > attempt to offline it (even if the guest would support it; ie. 'set' too > omits looking at the sysfs for cpu#0). This matches the common specific > case (exactly cpu#0 is fixed online), but it is wrong in general. > > I think I should introduce another boolean flag in the element structure > (only for the get method) that would say whether you can attempt to > offline the VCPU. Then the caller of the set method can either comply or > ignore the hint, and in the latter case it would be smacked down > rightfully.
I think that seems reasonable. > > Thanks! > Laszlo >