Re: Unable to pass multiple cores / CPUs to guests
Jim, I just don't know how I could thank you! I have put multiple days into that problem and didn't remember nor see that I already have put -smp 1 later in the command. I think it would have cost me some more days or I would have been unable to solve the problem at all if you didn't give me that hint. It's working like expected now; the guest sees and uses the additional CPU core, and the backup from encrypted drives to other encrypted drives is running much faster and with lower CPU load now. An amusing aside: During my experiments, I had a set of parameters which made the Windows guest think it runs at 7.2 GHz (3 x 2.4 GHz where 2.4 GHz is the real clock rate of the host's CPU). Unfortunately, I can't remember the exact command, but I believe it roughly looked like ... -cpu qemu64 -smp2 ... -smp 1 ... Regards, Binarus On 30.01.2012 18:44, Jim Paris wrote: Binarus wrote: Dear all, it seems that I am unable to pass multiple cores / CPUs to KVM guests. Whatever I do, the guests only report one CPU / core. ... /usr/bin/qemu-system-x86_64 -M pc -cpu host -smp cores=2,threads=1,sockets=1 -drive file=/dev/sda6,if=virtio,cache=none,index=0 -drive file=/dev/sdb,if=virtio,cache=none,index=1 -cdrom /dev/cdrom -pidfile ./qemu-garak.pid -boot c -k de -m 4096 -smp 1 -device pci-assign,host=01:05.0 -daemonize -usb -usbdevice tablet -name garak -net nic,vlan=0,model=virtio,macaddr=02:01:01:01:01:01 -net tap,vlan=0,ifname=virtnet1,script=/etc/qemu-ifup,downscript=/etc/qemu-ifup -vnc :1 Remove the -smp 1. -jim -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm tool: Make kvm structure to carry name copy
On Fri, 3 Feb 2012, Cyrill Gorcunov wrote: If guest name is used (which is default case) the kvm might end up carrying the pointer to name which is allocated on stack. kvm_cmd_run_init (on stack) default_name kvm__init(..., default_name) kvm-name = default_name So I think better to allow kvm to carry own copy of guest name. 64 symbols should be more than enough. Signed-off-by: Cyrill Gorcunov gorcu...@gmail.com --- I hope I didn't miss anything? Can't we just use strdup()? Pekka -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm tool: Make kvm structure to carry name copy
On Sat, Feb 04, 2012 at 02:15:36PM +0200, Pekka Enberg wrote: On Fri, 3 Feb 2012, Cyrill Gorcunov wrote: If guest name is used (which is default case) the kvm might end up carrying the pointer to name which is allocated on stack. kvm_cmd_run_init (on stack) default_name kvm__init(..., default_name) kvm-name = default_name So I think better to allow kvm to carry own copy of guest name. 64 symbols should be more than enough. Signed-off-by: Cyrill Gorcunov gorcu...@gmail.com --- I hope I didn't miss anything? Can't we just use strdup()? Yeah, I think this will be even better, I'll update. Cyrill -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm tool: Make kvm structure to carry name copy
On Sat, Feb 04, 2012 at 04:20:05PM +0400, Cyrill Gorcunov wrote: On Sat, Feb 04, 2012 at 02:15:36PM +0200, Pekka Enberg wrote: On Fri, 3 Feb 2012, Cyrill Gorcunov wrote: If guest name is used (which is default case) the kvm might end up carrying the pointer to name which is allocated on stack. kvm_cmd_run_init (on stack) default_name kvm__init(..., default_name) kvm-name = default_name So I think better to allow kvm to carry own copy of guest name. 64 symbols should be more than enough. Signed-off-by: Cyrill Gorcunov gorcu...@gmail.com --- I hope I didn't miss anything? Can't we just use strdup()? Yeah, I think this will be even better, I'll update. Something like below I think. Cyrill --- Subject: [PATCH] kvm tool: Make kvm structure to carry name copy If default guest name is used (which is the default case) the kvm might end up carrying the pointer to a name which is allocated on stack. kvm_cmd_run_init (on stack) default_name kvm__init(..., default_name) kvm-name = default_name So make it to carry a copy of name. Signed-off-by: Cyrill Gorcunov gorcu...@gmail.com --- tools/kvm/kvm.c |9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) Index: linux-2.6.git/tools/kvm/kvm.c === --- linux-2.6.git.orig/tools/kvm/kvm.c +++ linux-2.6.git/tools/kvm/kvm.c @@ -254,6 +254,7 @@ int kvm__exit(struct kvm *kvm) kvm__arch_delete_ram(kvm); kvm_ipc__stop(); kvm__remove_socket(kvm-name); + free((void *)kvm-name); free(kvm); return 0; @@ -377,6 +378,12 @@ struct kvm *kvm__init(const char *kvm_de goto cleanup; } + kvm-name = strdup(name); + if (!kvm-name) { + ret = -ENOMEM; + goto cleanup; + } + if (kvm__check_extensions(kvm)) { pr_err(A required KVM extention is not supported by OS); ret = -ENOSYS; @@ -384,8 +391,6 @@ struct kvm *kvm__init(const char *kvm_de kvm__arch_init(kvm, hugetlbfs_path, ram_size); - kvm-name = name; - kvm_ipc__start(kvm__create_socket(kvm)); kvm_ipc__register_handler(KVM_IPC_PID, kvm__pid); return kvm; -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm tool: Make kvm structure to carry name copy
On Sat, 4 Feb 2012, Cyrill Gorcunov wrote: Index: linux-2.6.git/tools/kvm/kvm.c === --- linux-2.6.git.orig/tools/kvm/kvm.c +++ linux-2.6.git/tools/kvm/kvm.c @@ -254,6 +254,7 @@ int kvm__exit(struct kvm *kvm) kvm__arch_delete_ram(kvm); kvm_ipc__stop(); kvm__remove_socket(kvm-name); + free((void *)kvm-name); Please fix the struct definition and drop the cast. free(kvm); return 0; @@ -377,6 +378,12 @@ struct kvm *kvm__init(const char *kvm_de -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm tool: Make kvm structure to carry name copy
On Sat, Feb 04, 2012 at 02:57:23PM +0200, Pekka Enberg wrote: On Sat, 4 Feb 2012, Cyrill Gorcunov wrote: Index: linux-2.6.git/tools/kvm/kvm.c === --- linux-2.6.git.orig/tools/kvm/kvm.c +++ linux-2.6.git/tools/kvm/kvm.c @@ -254,6 +254,7 @@ int kvm__exit(struct kvm *kvm) kvm__arch_delete_ram(kvm); kvm_ipc__stop(); kvm__remove_socket(kvm-name); +free((void *)kvm-name); Please fix the struct definition and drop the cast. I believe having it as const char * might save us some potential troubles, but sure, here we go ;) Cyrill --- kvm tool: Make kvm structure to carry name copy If default guest name is used (which is the default case) the kvm might end up carrying the pointer to a name which is allocated on stack. kvm_cmd_run_init (on stack) default_name kvm__init(..., default_name) kvm-name = default_name So make it to carry a copy of name. Signed-off-by: Cyrill Gorcunov gorcu...@gmail.com --- tools/kvm/kvm.c |9 +++-- tools/kvm/powerpc/include/kvm/kvm-arch.h |2 +- tools/kvm/x86/include/kvm/kvm-arch.h |2 +- 3 files changed, 9 insertions(+), 4 deletions(-) Index: linux-2.6.git/tools/kvm/kvm.c === --- linux-2.6.git.orig/tools/kvm/kvm.c +++ linux-2.6.git/tools/kvm/kvm.c @@ -254,6 +254,7 @@ int kvm__exit(struct kvm *kvm) kvm__arch_delete_ram(kvm); kvm_ipc__stop(); kvm__remove_socket(kvm-name); + free(kvm-name); free(kvm); return 0; @@ -377,6 +378,12 @@ struct kvm *kvm__init(const char *kvm_de goto cleanup; } + kvm-name = strdup(name); + if (!kvm-name) { + ret = -ENOMEM; + goto cleanup; + } + if (kvm__check_extensions(kvm)) { pr_err(A required KVM extention is not supported by OS); ret = -ENOSYS; @@ -384,8 +391,6 @@ struct kvm *kvm__init(const char *kvm_de kvm__arch_init(kvm, hugetlbfs_path, ram_size); - kvm-name = name; - kvm_ipc__start(kvm__create_socket(kvm)); kvm_ipc__register_handler(KVM_IPC_PID, kvm__pid); return kvm; Index: linux-2.6.git/tools/kvm/powerpc/include/kvm/kvm-arch.h === --- linux-2.6.git.orig/tools/kvm/powerpc/include/kvm/kvm-arch.h +++ linux-2.6.git/tools/kvm/powerpc/include/kvm/kvm-arch.h @@ -64,7 +64,7 @@ struct kvm { unsigned long fdt_gra; unsigned long initrd_gra; unsigned long initrd_size; - const char *name; + char*name; int vm_state; }; Index: linux-2.6.git/tools/kvm/x86/include/kvm/kvm-arch.h === --- linux-2.6.git.orig/tools/kvm/x86/include/kvm/kvm-arch.h +++ linux-2.6.git/tools/kvm/x86/include/kvm/kvm-arch.h @@ -48,7 +48,7 @@ struct kvm { struct disk_image **disks; int nr_disks; - const char *name; + char*name; int vm_state; }; -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] kvm tool: Don't close not yet opened files and SIGSEV fix
In case if there error happened in kvm__init and we have no files opened -- we should not try to close them. Also once kvm failed to init the caller should not try to dereference a pointer obtained, otherwise we might get SIGSEV | [cyrill@moon kvm]$ ./lkvm run ... | Error: '/dev/kvm' not found. Please make sure your kernel has CONFIG_KVM enabled and that the KVM modules are loaded. | Segmentation fault (core dumped) | [cyrill@moon kvm]$ Signed-off-by: Cyrill Gorcunov gorcu...@gmail.com --- tools/kvm/builtin-run.c |4 tools/kvm/kvm.c | 11 --- 2 files changed, 12 insertions(+), 3 deletions(-) Index: linux-2.6.git/tools/kvm/builtin-run.c === --- linux-2.6.git.orig/tools/kvm/builtin-run.c +++ linux-2.6.git/tools/kvm/builtin-run.c @@ -997,6 +997,10 @@ static int kvm_cmd_run_init(int argc, co } kvm = kvm__init(dev, hugetlbfs_path, ram_size, guest_name); + if (IS_ERR_OR_NULL(kvm)) { + r = PTR_ERR(kvm); + goto fail; + } kvm-single_step = single_step; Index: linux-2.6.git/tools/kvm/kvm.c === --- linux-2.6.git.orig/tools/kvm/kvm.c +++ linux-2.6.git/tools/kvm/kvm.c @@ -123,10 +123,12 @@ static int kvm__check_extensions(struct static struct kvm *kvm__new(void) { struct kvm *kvm = calloc(1, sizeof(*kvm)); - if (!kvm) return ERR_PTR(-ENOMEM); + kvm-sys_fd = -1; + kvm-vm_fd = -1; + return kvm; } @@ -394,9 +396,12 @@ struct kvm *kvm__init(const char *kvm_de kvm_ipc__start(kvm__create_socket(kvm)); kvm_ipc__register_handler(KVM_IPC_PID, kvm__pid); return kvm; + cleanup: - close(kvm-vm_fd); - close(kvm-sys_fd); + if (kvm-vm_fd = 0) + close(kvm-vm_fd); + if (kvm-sys_fd = 0) + close(kvm-sys_fd); free(kvm); return ERR_PTR(ret); -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm tool: Don't close not yet opened files and SIGSEV fix
On Sat, 4 Feb 2012, Cyrill Gorcunov wrote: In case if there error happened in kvm__init and we have no files opened -- we should not try to close them. Also once kvm failed to init the caller should not try to dereference a pointer obtained, otherwise we might get SIGSEV | [cyrill@moon kvm]$ ./lkvm run ... | Error: '/dev/kvm' not found. Please make sure your kernel has CONFIG_KVM enabled and that the KVM modules are loaded. | Segmentation fault (core dumped) | [cyrill@moon kvm]$ Signed-off-by: Cyrill Gorcunov gorcu...@gmail.com --- tools/kvm/builtin-run.c |4 tools/kvm/kvm.c | 11 --- 2 files changed, 12 insertions(+), 3 deletions(-) Index: linux-2.6.git/tools/kvm/builtin-run.c === --- linux-2.6.git.orig/tools/kvm/builtin-run.c +++ linux-2.6.git/tools/kvm/builtin-run.c @@ -997,6 +997,10 @@ static int kvm_cmd_run_init(int argc, co } kvm = kvm__init(dev, hugetlbfs_path, ram_size, guest_name); + if (IS_ERR_OR_NULL(kvm)) { PTR_ERR is still not going to work if kvm is NULL. + r = PTR_ERR(kvm); + goto fail; + } -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm tool: Don't close not yet opened files and SIGSEV fix
On Sat, Feb 04, 2012 at 07:38:41PM +0200, Pekka Enberg wrote: On Sat, 4 Feb 2012, Cyrill Gorcunov wrote: In case if there error happened in kvm__init and we have no files opened -- we should not try to close them. Also once kvm failed to init the caller should not try to dereference a pointer obtained, otherwise we might get SIGSEV | [cyrill@moon kvm]$ ./lkvm run ... | Error: '/dev/kvm' not found. Please make sure your kernel has CONFIG_KVM enabled and that the KVM modules are loaded. | Segmentation fault (core dumped) | [cyrill@moon kvm]$ Signed-off-by: Cyrill Gorcunov gorcu...@gmail.com --- tools/kvm/builtin-run.c |4 tools/kvm/kvm.c | 11 --- 2 files changed, 12 insertions(+), 3 deletions(-) Index: linux-2.6.git/tools/kvm/builtin-run.c === --- linux-2.6.git.orig/tools/kvm/builtin-run.c +++ linux-2.6.git/tools/kvm/builtin-run.c @@ -997,6 +997,10 @@ static int kvm_cmd_run_init(int argc, co } kvm = kvm__init(dev, hugetlbfs_path, ram_size, guest_name); +if (IS_ERR_OR_NULL(kvm)) { PTR_ERR is still not going to work if kvm is NULL. It should be IS_ERR rather (kvm__init never returns with NULL), but it's a candidate for another patch where both kvm__init and kvm__new should test for IS_ERR only. Pekka, could you please s/IS_ERR_OR_NULL/IS_ERR/ in this patch, and that's all. Hm? Cyrill -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm tool: Don't close not yet opened files and SIGSEV fix
On Sat, 4 Feb 2012, Cyrill Gorcunov wrote: Index: linux-2.6.git/tools/kvm/kvm.c === --- linux-2.6.git.orig/tools/kvm/kvm.c +++ linux-2.6.git/tools/kvm/kvm.c @@ -123,10 +123,12 @@ static int kvm__check_extensions(struct static struct kvm *kvm__new(void) { struct kvm *kvm = calloc(1, sizeof(*kvm)); - if (!kvm) return ERR_PTR(-ENOMEM); + kvm-sys_fd = -1; + kvm-vm_fd = -1; + return kvm; } @@ -394,9 +396,12 @@ struct kvm *kvm__init(const char *kvm_de kvm_ipc__start(kvm__create_socket(kvm)); kvm_ipc__register_handler(KVM_IPC_PID, kvm__pid); return kvm; + cleanup: - close(kvm-vm_fd); - close(kvm-sys_fd); + if (kvm-vm_fd = 0) + close(kvm-vm_fd); + if (kvm-sys_fd = 0) + close(kvm-sys_fd); No, please don't do this! Use specific error handling labels instead. free(kvm); return ERR_PTR(ret); -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm tool: Don't close not yet opened files and SIGSEV fix
On Sat, Feb 04, 2012 at 07:54:38PM +0200, Pekka Enberg wrote: On Sat, 4 Feb 2012, Cyrill Gorcunov wrote: Index: linux-2.6.git/tools/kvm/kvm.c === --- linux-2.6.git.orig/tools/kvm/kvm.c +++ linux-2.6.git/tools/kvm/kvm.c @@ -123,10 +123,12 @@ static int kvm__check_extensions(struct static struct kvm *kvm__new(void) { struct kvm *kvm = calloc(1, sizeof(*kvm)); - if (!kvm) return ERR_PTR(-ENOMEM); +kvm-sys_fd = -1; +kvm-vm_fd = -1; + return kvm; } @@ -394,9 +396,12 @@ struct kvm *kvm__init(const char *kvm_de kvm_ipc__start(kvm__create_socket(kvm)); kvm_ipc__register_handler(KVM_IPC_PID, kvm__pid); return kvm; + cleanup: -close(kvm-vm_fd); -close(kvm-sys_fd); +if (kvm-vm_fd = 0) +close(kvm-vm_fd); +if (kvm-sys_fd = 0) +close(kvm-sys_fd); No, please don't do this! Use specific error handling labels instead. Strictly speaking, kvm__init need more serious rewrite together with kvm__arch_init/kvm_ipc__start/kvm_ipc__register_handler ret. vals tests, i'll do this a bit late. Cyrill -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Fwd: oVirt Beijing - March 21st
Original Message Subject: oVirt Beijing - March 21st Date: Fri, 03 Feb 2012 15:52:41 -0500 From: Carl Trieloff cctriel...@redhat.com Spread the word/ come join us! http://www.ovirt.org/news-and-events/workshop/ Carl -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html