[Qemu-devel] [PATCH] add --accel option

[Qemu-devel] [PATCH] add --accel option

[Glauber Costa]

Since libvirt dudes seem to prefer it, add a --accel option. For now,
it only does the same as --enable-kvm, but it can easily be extended
in the future.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 qemu-options.hx |    7 +++++++
 vl.c            |    7 ++++++-
 2 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 1b420a3..990513f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1429,6 +1429,13 @@ Enable KQEMU kernel module usage. KQEMU options are only available if
 KQEMU support is enabled when compiling.
 ETEXI
 
+DEF("accel", HAS_ARG, QEMU_OPTION_accel, \
+    "-accel name	enable a given accelerator module\n")
+STEXI
+@item -accel @var{name}
+Enable a given accelerator module, for instance, KVM.
+ETEXI
+
 #ifdef CONFIG_KVM
 DEF("enable-kvm", 0, QEMU_OPTION_enable_kvm, \
     "-enable-kvm     enable KVM full virtualization support\n")
diff --git a/vl.c b/vl.c
index bb56644..e03f3af 100644
--- a/vl.c
+++ b/vl.c
@@ -3091,7 +3091,6 @@ static int ram_save_live(QEMUFile *f, int stage, void *opaque)
         while (ram_save_block(f) != 0) {
             bytes_transferred += TARGET_PAGE_SIZE;
         }
-        cpu_physical_memory_set_dirty_tracking(0);
     }
 
     qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
@@ -5431,6 +5430,12 @@ int main(int argc, char **argv, char **envp)
                 kqemu_allowed = 2;
                 break;
 #endif
+            case QEMU_OPTION_accel:
+                if (strstart(optarg, "kvm", NULL)) {
+                    kvm_allowed = 1;
+                    kqemu_allowed = 0;
+                }
+                break;
 #ifdef CONFIG_KVM
             case QEMU_OPTION_enable_kvm:
                 kvm_allowed = 1;
-- 
1.6.2.2

[Qemu-devel] Re: [PATCH] add --accel option

[Anthony Liguori]

Glauber Costa wrote:
> Since libvirt dudes seem to prefer it, add a --accel option. For now,
> it only does the same as --enable-kvm, but it can easily be extended
> in the future.
>
> Signed-off-by: Glauber Costa <glommer@redhat.com>
>   

To be useful, it should also include kqemu and a none option.

-- 
Regards,

Anthony Liguori

[Qemu-devel] Re: [PATCH] add --accel option

[Glauber Costa]

On Tue, Jul 28, 2009 at 03:27:19PM -0500, Anthony Liguori wrote:
> Glauber Costa wrote:
>> Since libvirt dudes seem to prefer it, add a --accel option. For now,
>> it only does the same as --enable-kvm, but it can easily be extended
>> in the future.
>>
>> Signed-off-by: Glauber Costa <glommer@redhat.com>
>>   
>
> To be useful, it should also include kqemu and a none option.
yeah, agree. This can be added later (even now!). I just wanted to signal it
out that it will happen, because it will allow us to change qemu-kvm semantics in a
positive way. I can redo the patch or send an incremental, whatever you prefer

[Qemu-devel] [PATCH] add --accel option

[Glauber Costa]

Since libvirt dudes seem to prefer it, add a --accel option. For now,
it only does the same as --enable-kvm, but it can easily be extended
in the future. It also accepts "none" and "kqemu" options.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 qemu-options.hx |    7 +++++++
 vl.c            |   26 +++++++++++++++++++++++++-
 2 files changed, 32 insertions(+), 1 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 1b420a3..990513f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1429,6 +1429,13 @@ Enable KQEMU kernel module usage. KQEMU options are only available if
 KQEMU support is enabled when compiling.
 ETEXI
 
+DEF("accel", HAS_ARG, QEMU_OPTION_accel, \
+    "-accel name	enable a given accelerator module\n")
+STEXI
+@item -accel @var{name}
+Enable a given accelerator module, for instance, KVM.
+ETEXI
+
 #ifdef CONFIG_KVM
 DEF("enable-kvm", 0, QEMU_OPTION_enable_kvm, \
     "-enable-kvm     enable KVM full virtualization support\n")
diff --git a/vl.c b/vl.c
index bb56644..5728715 100644
--- a/vl.c
+++ b/vl.c
@@ -3091,7 +3091,6 @@ static int ram_save_live(QEMUFile *f, int stage, void *opaque)
         while (ram_save_block(f) != 0) {
             bytes_transferred += TARGET_PAGE_SIZE;
         }
-        cpu_physical_memory_set_dirty_tracking(0);
     }
 
     qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
@@ -5431,6 +5430,31 @@ int main(int argc, char **argv, char **envp)
                 kqemu_allowed = 2;
                 break;
 #endif
+            case QEMU_OPTION_accel:
+            {
+                const char *kqemu_opt;
+                if (strstart(optarg, "kvm", NULL)) {
+                    kvm_allowed = 1;
+                    kqemu_allowed = 0;
+                }
+                if (strstart(optarg, "none", NULL)) {
+                    kvm_allowed = 0;
+                    kqemu_allowed = 0;
+                }
+
+                if (strstart(optarg, "kqemu", &kqemu_opt)) {
+                    kvm_allowed = 0;
+                    if (*kqemu_opt == ',' && strstart(kqemu_opt,",mode=kernel", NULL)) {
+                        kqemu_allowed = 2;
+                    } else if (*kqemu_opt != ',' || strstart(kqemu_opt, ",mode=user", NULL)) {
+                        kqemu_allowed = 1;
+                    } else {
+                        fprintf(stderr, "invalid kqemu mode. use mode=user or mode=kernel\n");
+                        exit(1);
+                    }
+                }
+                break;
+            }
 #ifdef CONFIG_KVM
             case QEMU_OPTION_enable_kvm:
                 kvm_allowed = 1;
-- 
1.6.2.2

Re: [Qemu-devel] [PATCH] add --accel option

[Kevin Wolf]

Glauber Costa schrieb:
> Since libvirt dudes seem to prefer it, add a --accel option. For now,
> it only does the same as --enable-kvm, but it can easily be extended
> in the future. It also accepts "none" and "kqemu" options.

The documentation should include which values are valid for the option.
Guessing that the syntax is -accel kvm is probably possible, but for
-accel kqemu,mode=kernel it's definitely hard.

Kevin

Re: [Qemu-devel] [PATCH] add --accel option

[Glauber Costa]

On Wed, Jul 29, 2009 at 10:16:28AM +0200, Kevin Wolf wrote:
> Glauber Costa schrieb:
> > Since libvirt dudes seem to prefer it, add a --accel option. For now,
> > it only does the same as --enable-kvm, but it can easily be extended
> > in the future. It also accepts "none" and "kqemu" options.
> 
> The documentation should include which values are valid for the option.
> Guessing that the syntax is -accel kvm is probably possible, but for
> -accel kqemu,mode=kernel it's definitely hard.
right now kqemu needs an extra argument.

But very soon kvm will need it too. Like for example controlling knobs
like kvm-irqchip and kvm-pit. So maybe we want a standard way of passing
arguments for all accels?

How about --accel foo,args=bar,bar,bar with the args part being optional ?

An exampel would be --accel kvm,args=userpit,userirqchip (assuming we want the
default to be in the kernel)

Otherwise we're stuck with an evergrowing combination to support

[Qemu-devel] Re: [PATCH] add --accel option

[Kevin Wolf]

Juan Quintela schrieb:
> Glauber Costa <glommer@redhat.com> wrote:
>> On Wed, Jul 29, 2009 at 10:16:28AM +0200, Kevin Wolf wrote:
>>> Glauber Costa schrieb:
>>>> Since libvirt dudes seem to prefer it, add a --accel option. For now,
>>>> it only does the same as --enable-kvm, but it can easily be extended
>>>> in the future. It also accepts "none" and "kqemu" options.
>>> The documentation should include which values are valid for the option.
>>> Guessing that the syntax is -accel kvm is probably possible, but for
>>> -accel kqemu,mode=kernel it's definitely hard.
>> right now kqemu needs an extra argument.
>>
>> But very soon kvm will need it too. Like for example controlling knobs
>> like kvm-irqchip and kvm-pit. So maybe we want a standard way of passing
>> arguments for all accels?
>>
>> How about --accel foo,args=bar,bar,bar with the args part being optional ?
>>
>> An exampel would be --accel kvm,args=userpit,userirqchip (assuming we want the
>> default to be in the kernel)
>>
>> Otherwise we're stuck with an evergrowing combination to support
> 
> No, pleaseeee
> "," shouldn't have two meanings in the same command line
> 
> --accel kvm,userpit,userirqchip
> 
> is ok
> 
> --accell kvm,userpit=on,userirqchip=off
> 
> also ok

QemuOpts supports both of these (and none of the versions below).

Kevin


> 
> --accell kvm,args=userpit,userirqchip
> 
> parses as
>    args=userpit
>    userirqchip
> or
>    args=userpit,userirqchip
> 
> IMHO, having lists inside lists is just insane, and if you want it, do
> it marking the list properly:
> 
> --accell kvm,args=[userpit,userirqchip]
> 
> or any other opening/closing pair.
> 
> Later, Juan.