Re: [Qemu-devel] [PATCH for-1.4 04/12] kvm: Create kvm_arch_vcpu_id() function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jan 18, 2013 at 02:03:09PM +0100, Andreas Färber wrote:
> Am 18.01.2013 13:53, schrieb Eduardo Habkost:
> > On Fri, Jan 18, 2013 at 12:11:29PM +0100, Andreas Färber wrote:
> > [...]
> >>> +/* Returns VCPU ID to be used on KVM_CREATE_VCPU ioctl() */
> >>> +unsigned long kvm_arch_vcpu_id(CPUState *cpu);
> >>> +
> >>>  void kvm_arch_reset_vcpu(CPUState *cpu);
> >>>  
> >>>  int kvm_arch_on_sigbus_vcpu(CPUState *cpu, int code, void *addr);
> >>> diff --git a/kvm-all.c b/kvm-all.c
> >>> index 6278d61..995220d 100644
> >>> --- a/kvm-all.c
> >>> +++ b/kvm-all.c
> >>> @@ -222,7 +222,7 @@ int kvm_init_vcpu(CPUState *cpu)
> >>>  
> >>>      DPRINTF("kvm_init_vcpu\n");
> >>>  
> >>> -    ret = kvm_vm_ioctl(s, KVM_CREATE_VCPU, cpu->cpu_index);
> >>> +    ret = kvm_vm_ioctl(s, KVM_CREATE_VCPU, kvm_arch_vcpu_id(cpu));
> >>>      if (ret < 0) {
> >>>          DPRINTF("kvm_create_vcpu failed\n");
> >>>          goto err;
> >>
> >> This is changing the vararg from int to unsigned long. I have no
> >> insights yet on how this is handled and whether that is okay; I would at
> >> least expect this change to be mentioned in the commit message.
> > 
> > It was an unexpected change (I didn't notice that cpu_index was int),
> > but strictly speaking the previous code was incorrect (as ioctl() gets
> > an unsigned long argument, not int). I doubt there are cases where it
> > would really break, but it is a good thing to fix it.
> > 
> > I agree this should be mentioned in the commit message, though. Will you
> > add it before committing, or should I resubmit?
> 
> Could you suggest a text for me to add please?

"The argument passed to KVM_CREATE_VCPU now has 'unsigned long' type
instead of 'int', as expected by the Linux ioctl() syscall. Maybe an int
works on most or all architectures supporting KVM, but it is safer to
use an appropriate 'unsigned long' parameter."

To find out if 'int' breaks on any architecture, I would need to check
the ABI specification for each architecture. I didn't do that, but I am
sure we should pass an unsigned long instead, if that's the type
expected by the kernel.

-- 
Eduardo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux