Re: Unable to pass multiple cores / CPUs to guests

2012-02-04 Thread Binarus
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

2012-02-04 Thread Pekka Enberg

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

2012-02-04 Thread Cyrill Gorcunov
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

2012-02-04 Thread Cyrill Gorcunov
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

2012-02-04 Thread Pekka Enberg

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

2012-02-04 Thread Cyrill Gorcunov
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

2012-02-04 Thread Cyrill Gorcunov
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

2012-02-04 Thread Pekka Enberg

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

2012-02-04 Thread Cyrill Gorcunov
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

2012-02-04 Thread Pekka Enberg

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

2012-02-04 Thread Cyrill Gorcunov
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

2012-02-04 Thread Itamar Heim



 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