Re: [Qemu-devel] [PATCH 3/3] qga: implement qmp_guest_set_vcpus() for Linux with sysfs

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1459 bytes --]

On 03/05/2013 04:23 PM, Laszlo Ersek wrote:
> 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
> [...]

So we really do have to read existing state to avoid an error when the
user didn't request a state change.  Thanks for the research, and a
convincing answer :)

>>> +                    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.

I guess I had in my mind %c instead of %u; still, I can't help but
wonder if fprintf() and buffering is overkill, compared to just doing
something like this:
 write(fd, &"01"[vcpu->online], 1);

(okay, I hope you would favor readability over my compact
representation, but you get the point).  Oh, and I guess I didn't check
whether a trailing newline is essential to the kernel interpreting the
input, so maybe it would have to be more like:

 char buf[2] = { "01"[vcpu->online], '\n' };
 write(fd, buf, 2);

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]