On 03/05/13 22:19, Eric Blake wrote: > On 03/04/2013 03:19 PM, Laszlo Ersek wrote:
>> + } else { >> + unsigned online; >> + >> + if (fscanf(f, "%u", &online) != 1) { >> + error_setg(&local_err, "failed to read or parse \"%s\"", >> + buf); > > Does doing a scan of the file's existing contents buy us any safety? > Why not just blindly write into the file, instead of first reading it? :) For an already online CPU: # dd of=/sys/devices/system/cpu/cpu1/online bs=1 count=1 <<<1 dd: writing `/sys/devices/system/cpu/cpu1/online': Invalid argument [...] In the kernel, store_online() [drivers/base/cpu.c] cpu_up() [kernel/cpu.c] _cpu_up() if (cpu_online(cpu) || !cpu_present(cpu)) { ret = -EINVAL; goto out; } This logic seems to have been present since the origin of the current Linux repo (1da177e4 "Linux-2.6.12-rc2"). Checking the history tree at <git://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git>, the logic dates back to the very first committed version of cpu_up(): commit c5e062079a7090891ea5cd1b23a7eab52b156b2a Author: Rusty Russell <ru...@rustcorp.com.au> Date: Fri Jul 26 01:28:07 2002 -0700 [PATCH] Hot-plug CPU Boot Changes ... > >> + } else if ((online != 0) != vcpu->online) { >> + errno = 0; >> + rewind(f); >> + if (errno != 0 || >> + fprintf(f, "%u\n", (unsigned)vcpu->online) < 0) { > > Do you really want to be printing NUL or \1? I though the kernel > interface expected the literal character '0' or '1' (in ascii, \x30 or > \x31). I'm using the %u conversion specifier, which turns the unsigned int argument into decimal string representation. Same as %d for signed int. Thanks for all review comments! Laszlo