Skip to contentSkip to conversation

qemu-kvm: remove "boot=on|off" drive parameter compatibility

PATCH In review Marcelo Tosatti 17 participants 52 messages Sep 30, 2012 – Oct 8, 2012 KVM

lore
Share & tools
lore
#0 lore raw

Option is deprecated and warning has been in place for one year.

blockdev.c +0 -6
index 4a5266e..7c83baa 100644
@@ -432,12 +432,6 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
         return NULL;
     }
 
-    if (qemu_opt_get(opts, "boot") != NULL) {
-        fprintf(stderr, "qemu-kvm: boot=on|off is deprecated and will be "
-                "ignored. Future versions will reject this parameter. Please "
-                "update your scripts.\n");
-    }
-
     on_write_error = BLOCK_ERR_STOP_ENOSPC;
     if ((buf = qemu_opt_get(opts, "werror")) != NULL) {
         if (type != IF_IDE && type != IF_SCSI && type != IF_VIRTIO && type != IF_NONE) {
qemu-config.c +0 -4
index 3eaee48..eba977e 100644
@@ -114,10 +114,6 @@ static QemuOptsList qemu_drive_opts = {
             .name = "copy-on-read",
             .type = QEMU_OPT_BOOL,
             .help = "copy read data from backing file into image file",
-        },{
-            .name = "boot",
-            .type = QEMU_OPT_BOOL,
-            .help = "(deprecated, ignored)",
         },
         { /* end of list */ }
     },
Reviewed-by
#1 lore raw

----- Original Message -----

Quoted 41 lines

Option is deprecated and warning has been in place for one year.

blockdev.c +0 -2
index 4a5266e..7c83baa 100644
@@ -432,12 +432,6 @@ DriveInfo *drive_init(QemuOpts *opts, int
 default_to_scsi)
         return NULL;
     }
 
-    if (qemu_opt_get(opts, "boot") != NULL) {
-        fprintf(stderr, "qemu-kvm: boot=on|off is deprecated and

will be "
- "ignored. Future versions will reject this
parameter. Please "

blockdev.c +0 -3
-                "update your scripts.\n");
-    }
-
     on_write_error = BLOCK_ERR_STOP_ENOSPC;
     if ((buf = qemu_opt_get(opts, "werror")) != NULL) {
         if (type != IF_IDE && type != IF_SCSI && type != IF_VIRTIO
         && type != IF_NONE) {
qemu-config.c +0 -4
index 3eaee48..eba977e 100644
@@ -114,10 +114,6 @@ static QemuOptsList qemu_drive_opts = {
             .name = "copy-on-read",
             .type = QEMU_OPT_BOOL,
             .help = "copy read data from backing file into image
             file",
-        },{
-            .name = "boot",
-            .type = QEMU_OPT_BOOL,
-            .help = "(deprecated, ignored)",
         },
         { /* end of list */ }
Quoted 1 line
     },
#2 lore raw
In reply to Marcelo Tosatti
Quoted 2 lines · Marcelo Tosatti

Option is deprecated and warning has been in place for one year.

Do we really care about such cosmetics? What is the big plan for qemu-kvm now? For 1.3 and then beyond?

Jan

Quoted 36 lines
blockdev.c +0 -6
index 4a5266e..7c83baa 100644
@@ -432,12 +432,6 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
         return NULL;
     }
 
-    if (qemu_opt_get(opts, "boot") != NULL) {
-        fprintf(stderr, "qemu-kvm: boot=on|off is deprecated and will be "
-                "ignored. Future versions will reject this parameter. Please "
-                "update your scripts.\n");
-    }
-
     on_write_error = BLOCK_ERR_STOP_ENOSPC;
     if ((buf = qemu_opt_get(opts, "werror")) != NULL) {
         if (type != IF_IDE && type != IF_SCSI && type != IF_VIRTIO && type != IF_NONE) {
qemu-config.c +0 -4
index 3eaee48..eba977e 100644
@@ -114,10 +114,6 @@ static QemuOptsList qemu_drive_opts = {
             .name = "copy-on-read",
             .type = QEMU_OPT_BOOL,
             .help = "copy read data from backing file into image file",
-        },{
-            .name = "boot",
-            .type = QEMU_OPT_BOOL,
-            .help = "(deprecated, ignored)",
         },
         { /* end of list */ }
     },
Signature · 2 lines
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
#3 lore raw
Quoted 5 lines · Jan Kiszka
Quoted 2 lines · Marcelo Tosatti

Option is deprecated and warning has been in place for one year.

Do we really care about such cosmetics?

We care about removing qemu-kvm to null.

Quoted 2 lines

What is the big plan for
qemu-kvm now? For 1.3 and then beyond?

I suggested this: provide a configuration file (and proper guide on how to use it on announce email) to be shipped with qemu 1.3.0.

That is:

"For compatibility with qemu-kvm 1.2.0, use

qemu-system-x86_64 -config /usr/share/qemu/qemu-kvm-1.2-compat.opt"

This would work for rtl8139-as-default, vga-ram-size differences.

And drop all command line option compatibility (which can be easily fixed by an administrator/end user).

Comments?

#4 lore raw
Quoted 19 lines · Marcelo Tosatti
Quoted 5 lines · Jan Kiszka
Quoted 2 lines · Marcelo Tosatti

Option is deprecated and warning has been in place for one year.

Do we really care about such cosmetics?

We care about removing qemu-kvm to null.

Quoted 2 lines

What is the big plan for
qemu-kvm now? For 1.3 and then beyond?

I suggested this: provide a configuration file (and proper guide on how to use it on announce email) to be shipped with qemu 1.3.0.

That is:

"For compatibility with qemu-kvm 1.2.0, use

Quoted 8 lines

qemu-system-x86_64 -config /usr/share/qemu/qemu-kvm-1.2-compat.opt"

This would work for rtl8139-as-default, vga-ram-size differences.

And drop all command line option compatibility (which can be easily fixed by an administrator/end user).

Comments?

It's not just about default configs. We need to validate if the migration formats are truly compatible (qemu-kvm -> QEMU, the other way around definitely not). For the command line switches, we could provide a wrapper script that translates them into upstream format or simply ignores them. That should be harmless to carry upstream.

But I would really stop worrying about the qemu-kvm code base.

Jan

Signature · 2 lines
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
#5 lore raw
Quoted 24 lines · Jan Kiszka
Quoted 23 lines · Marcelo Tosatti
Quoted 5 lines · Jan Kiszka
Quoted 2 lines · Marcelo Tosatti

Option is deprecated and warning has been in place for one year.

Do we really care about such cosmetics?

We care about removing qemu-kvm to null.

Quoted 2 lines

What is the big plan for
qemu-kvm now? For 1.3 and then beyond?

I suggested this: provide a configuration file (and proper guide on how to use it on announce email) to be shipped with qemu 1.3.0.

That is:

"For compatibility with qemu-kvm 1.2.0, use

qemu-system-x86_64 -config /usr/share/qemu/qemu-kvm-1.2-compat.opt"

This would work for rtl8139-as-default, vga-ram-size differences.

Quoted 8 lines
Quoted 4 lines

And drop all command line option compatibility (which can be easily fixed by an administrator/end user).

Comments?

It's not just about default configs. We need to validate if the migration formats are truly compatible (qemu-kvm -> QEMU, the other way around definitely not).

Right, VGA RAM size differences are part of migration compatibility.

The other two are ACPI and i8254 (will be looking into details soon, thanks).

Quoted 3 lines

For the command line switches, we could provide
a wrapper script that translates them into upstream format or simply ignores them. That should be harmless to carry upstream.

Do you have an objection against just pushing the responsability to administrators? It can be seen as configuration file format change.

Most users should not be using direct command line anyway.

Quoted 3 lines

But I would really stop worrying about the qemu-kvm code base.

Jan

Right, thats what we're trying to do here.

#6 lore raw
Quoted 39 lines · Marcelo Tosatti
Quoted 24 lines · Jan Kiszka
Quoted 23 lines · Marcelo Tosatti
Quoted 5 lines · Jan Kiszka
Quoted 2 lines · Marcelo Tosatti

Option is deprecated and warning has been in place for one year.

Do we really care about such cosmetics?

We care about removing qemu-kvm to null.

Quoted 2 lines

What is the big plan for
qemu-kvm now? For 1.3 and then beyond?

I suggested this: provide a configuration file (and proper guide on how to use it on announce email) to be shipped with qemu 1.3.0.

That is:

"For compatibility with qemu-kvm 1.2.0, use

qemu-system-x86_64 -config /usr/share/qemu/qemu-kvm-1.2-compat.opt"

This would work for rtl8139-as-default, vga-ram-size differences.

Quoted 8 lines
Quoted 4 lines

And drop all command line option compatibility (which can be easily fixed by an administrator/end user).

Comments?

It's not just about default configs. We need to validate if the migration formats are truly compatible (qemu-kvm -> QEMU, the other way around definitely not).

Right, VGA RAM size differences are part of migration compatibility.

The other two are ACPI and i8254 (will be looking into details soon, thanks).

Quoted 6 lines
Quoted 3 lines

For the command line switches, we could provide
a wrapper script that translates them into upstream format or simply ignores them. That should be harmless to carry upstream.

Do you have an objection against just pushing the responsability to administrators? It can be seen as configuration file format change.

This is about helping him in the most appropriate way.

Quoted 2 lines

Most users should not be using direct command line anyway.

If you are using the command line, you shouldn't care about qemu-kvm's legacy. But there might be home-grown management stacks or scripts around that have to be adjusted. So some wrapper may help in this process, either as reference or as intermediate adapter.

Quoted 6 lines
Quoted 4 lines

But I would really stop worrying about the qemu-kvm code base.

Jan

Right, thats what we're trying to do here.

Just that I'm missing how this patch correlates with the goal to get QEMU ready for qemu-kvm users. :)

Jan

Signature · 2 lines
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
#7 lore raw
In reply to Jan Kiszka
Quoted 7 lines · Jan Kiszka

On 2012-10-01 11:31, Marcelo Tosatti wrote:

It's not just about default configs. We need to validate if the migration formats are truly compatible (qemu-kvm -> QEMU, the other way around definitely not). For the command line switches, we could provide a wrapper script that translates them into upstream format or simply ignores them. That should be harmless to carry upstream.

qemu-kvm has:

-no-kvm
-no-kvm-irqchip
-no-kvm-pit
-no-kvm-pit-reinjection
-tdf <- does nothing

There are replacements for all of the above. If we need to add them to qemu.git, it's not big deal to add them.

-drive ...,boot= <- this is ignored

cpu_set command for CPU hotplug which is known broken in qemu-kvm.

testdev which is nice but only used for development

Default nic is rtl8139 vs. e1000.

Some logic to move change the default VGA ram size to 16mb for pc-1.2 (QEMU uses 16mb by default now too).

I think at this point, none of this matters but I added the various distro maintainers to the thread.

I think it's time for the distros to drop qemu-kvm and just ship qemu.git. Is there anything else that needs to happen to make that switch?

Regards,

Anthony Liguori

Quoted 12 lines

But I would really stop worrying about the qemu-kvm code base.

Jan

Signature · 6 lines
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#8 lore raw
Quoted 12 lines · Anthony Liguori
Quoted 7 lines · Jan Kiszka

On 2012-10-01 11:31, Marcelo Tosatti wrote:

It's not just about default configs. We need to validate if the migration formats are truly compatible (qemu-kvm -> QEMU, the other way around definitely not). For the command line switches, we could provide a wrapper script that translates them into upstream format or simply ignores them. That should be harmless to carry upstream.

qemu-kvm has:

Quoted 8 lines

-no-kvm
-no-kvm-irqchip
-no-kvm-pit
-no-kvm-pit-reinjection
-tdf <- does nothing

There are replacements for all of the above. If we need to add them to qemu.git, it's not big deal to add them.

But I don't think we should add them to the source code. This can perfectly be handled my a (disposable) script layer on top of qemu-system-x86_64 - the namespace (qemu-kvm in most cases) is also free.

Quoted 4 lines

-drive ...,boot= <- this is ignored

cpu_set command for CPU hotplug which is known broken in qemu-kvm.

Right, so nothing is lost when migrating to QEMU.

Quoted 7 lines

testdev which is nice but only used for development

Default nic is rtl8139 vs. e1000.

Some logic to move change the default VGA ram size to 16mb for pc-1.2 (QEMU uses 16mb by default now too).

Also nicely manageable in a wrapper.

Quoted 6 lines

I think at this point, none of this matters but I added the various distro maintainers to the thread.

I think it's time for the distros to drop qemu-kvm and just ship qemu.git.

+1

Jan

Quoted 6 lines

Is there anything else that needs to happen to make that
switch?

Regards,

Anthony Liguori

Signature · 2 lines
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
#9 lore raw
Quoted 27 lines · Jan Kiszka
Quoted 12 lines · Anthony Liguori
Quoted 7 lines · Jan Kiszka

On 2012-10-01 11:31, Marcelo Tosatti wrote:

It's not just about default configs. We need to validate if the migration formats are truly compatible (qemu-kvm -> QEMU, the other way around definitely not). For the command line switches, we could provide a wrapper script that translates them into upstream format or simply ignores them. That should be harmless to carry upstream.

qemu-kvm has:

Quoted 8 lines

-no-kvm
-no-kvm-irqchip
-no-kvm-pit
-no-kvm-pit-reinjection
-tdf <- does nothing

There are replacements for all of the above. If we need to add them to qemu.git, it's not big deal to add them.

But I don't think we should add them to the source code. This can perfectly be handled my a (disposable) script layer on top of qemu-system-x86_64 - the namespace (qemu-kvm in most cases) is also free.

Quoted 1 line
Quoted 9 lines
Quoted 3 lines

-drive ...,boot= <- this is ignored

cpu_set command for CPU hotplug which is known broken in qemu-kvm.

Right, so nothing is lost when migrating to QEMU.

Quoted 3 lines

testdev which is nice but only used for development

Jan, do you have a plan for testdev device? It would be a pity to have qemu-kvm just for that.

Quoted 16 lines
Quoted 4 lines

Default nic is rtl8139 vs. e1000.

Some logic to move change the default VGA ram size to 16mb for pc-1.2 (QEMU uses 16mb by default now too).

Also nicely manageable in a wrapper.

Quoted 6 lines

I think at this point, none of this matters but I added the various distro maintainers to the thread.

I think it's time for the distros to drop qemu-kvm and just ship qemu.git.

+1

Quoted 12 lines

Jan

Quoted 6 lines

Is there anything else that needs to happen to make that
switch?

Regards,

Anthony Liguori

Signature · 2 lines
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux

--
Gleb.

#10 lore raw
Quoted 32 lines · Gleb Natapov
Quoted 31 lines · Jan Kiszka
Quoted 12 lines · Anthony Liguori
Quoted 7 lines · Jan Kiszka

On 2012-10-01 11:31, Marcelo Tosatti wrote:

It's not just about default configs. We need to validate if the migration formats are truly compatible (qemu-kvm -> QEMU, the other way around definitely not). For the command line switches, we could provide a wrapper script that translates them into upstream format or simply ignores them. That should be harmless to carry upstream.

qemu-kvm has:

Quoted 8 lines

-no-kvm
-no-kvm-irqchip
-no-kvm-pit
-no-kvm-pit-reinjection
-tdf <- does nothing

There are replacements for all of the above. If we need to add them to qemu.git, it's not big deal to add them.

But I don't think we should add them to the source code. This can perfectly be handled my a (disposable) script layer on top of qemu-system-x86_64 - the namespace (qemu-kvm in most cases) is also free.

Quoted 4 lines

-drive ...,boot= <- this is ignored

cpu_set command for CPU hotplug which is known broken in qemu-kvm.

Quoted 7 lines
Quoted 5 lines

Right, so nothing is lost when migrating to QEMU.

Quoted 3 lines

testdev which is nice but only used for development

Jan, do you have a plan for testdev device? It would be a pity to have qemu-kvm just for that.

Nope, not on my schedule.

Jan

#11 lore raw
Quoted 36 lines · Jan Kiszka
Quoted 35 lines · Gleb Natapov
Quoted 34 lines · Jan Kiszka
Quoted 12 lines · Anthony Liguori
Quoted 7 lines · Jan Kiszka

On 2012-10-01 11:31, Marcelo Tosatti wrote:

It's not just about default configs. We need to validate if the migration formats are truly compatible (qemu-kvm -> QEMU, the other way around definitely not). For the command line switches, we could provide a wrapper script that translates them into upstream format or simply ignores them. That should be harmless to carry upstream.

qemu-kvm has:

Quoted 8 lines

-no-kvm
-no-kvm-irqchip
-no-kvm-pit
-no-kvm-pit-reinjection
-tdf <- does nothing

There are replacements for all of the above. If we need to add them to qemu.git, it's not big deal to add them.

But I don't think we should add them to the source code. This can perfectly be handled my a (disposable) script layer on top of qemu-system-x86_64 - the namespace (qemu-kvm in most cases) is also free.

Quoted 4 lines

-drive ...,boot= <- this is ignored

cpu_set command for CPU hotplug which is known broken in qemu-kvm.

Right, so nothing is lost when migrating to QEMU.

Quoted 1 line
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
Quoted 2 lines

testdev which is nice but only used for development

Jan, do you have a plan for testdev device? It would be a pity to have qemu-kvm just for that.

Nope, not on my schedule.

Understood :)

--
Gleb.

#12 lore raw
In reply to Gleb Natapov
Quoted 32 lines · Gleb Natapov
Quoted 31 lines · Jan Kiszka
Quoted 12 lines · Anthony Liguori
Quoted 7 lines · Jan Kiszka

On 2012-10-01 11:31, Marcelo Tosatti wrote:

It's not just about default configs. We need to validate if the migration formats are truly compatible (qemu-kvm -> QEMU, the other way around definitely not). For the command line switches, we could provide a wrapper script that translates them into upstream format or simply ignores them. That should be harmless to carry upstream.

qemu-kvm has:

Quoted 8 lines

-no-kvm
-no-kvm-irqchip
-no-kvm-pit
-no-kvm-pit-reinjection
-tdf <- does nothing

There are replacements for all of the above. If we need to add them to qemu.git, it's not big deal to add them.

But I don't think we should add them to the source code. This can perfectly be handled my a (disposable) script layer on top of qemu-system-x86_64 - the namespace (qemu-kvm in most cases) is also free.

Quoted 4 lines

-drive ...,boot= <- this is ignored

cpu_set command for CPU hotplug which is known broken in qemu-kvm.

Quoted 7 lines
Quoted 5 lines

Right, so nothing is lost when migrating to QEMU.

Quoted 3 lines

testdev which is nice but only used for development

Jan, do you have a plan for testdev device? It would be a pity to have qemu-kvm just for that.

Yep, I did send patches with the testdev device present on qemu-kvm.git to qemu.git a while ago, but there were many comments on the review, I ended up not implementing everything that was asked and the patches were archived.

If nobody wants to step up to port it, I'll re-read the original thread and will spin up new patches (and try to go through the end with it). Executing the KVM unittests is something that we can't afford to lose, so I'd say it's important on this last mile effort to get rid of qemu-kvm.

maintainer
#13 lore raw

Il 03/10/2012 12:57, Lucas Meneghel Rodrigues ha scritto:

Quoted 9 lines

Yep, I did send patches with the testdev device present on qemu-kvm.git to qemu.git a while ago, but there were many comments on the review, I ended up not implementing everything that was asked and the patches were archived.

If nobody wants to step up to port it, I'll re-read the original thread and will spin up new patches (and try to go through the end with it). Executing the KVM unittests is something that we can't afford to lose, so I'd say it's important on this last mile effort to get rid of qemu-kvm.

Absolutely, IIRC the problem was that testdev did a little bit of everything... let's see what's the functionality of testdev:

- write (port 0xf1), can be replaced in autotest with:
-device isa-debugcon,iobase=0xf1,chardev=...

- exit code (port 0xf4), see this series:
http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg00818.html

- ram size (port 0xd1). If we can also patch kvm-unittests, the memory is available in the CMOS or in fwcfg. Here is the SeaBIOS code:

        u32 rs = ((inb_cmos(0x34) << 16) | (inb_cmos(0x35) << 24));
        if (rs)
            rs += 16 * 1024 * 1024;
        else
            rs = (((inb_cmos(0x30) << 10) | (inb_cmos(0x31) << 18))
                  + 1 * 1024 * 1024);

The rest (ports 0xe0..0xe7, 0x2000..0x2017, MMIO) can be left in testdev.

Paolo

#14 lore raw
Quoted 11 lines · Paolo Bonzini

Il 03/10/2012 12:57, Lucas Meneghel Rodrigues ha scritto:

Quoted 9 lines

Yep, I did send patches with the testdev device present on qemu-kvm.git to qemu.git a while ago, but there were many comments on the review, I ended up not implementing everything that was asked and the patches were archived.

If nobody wants to step up to port it, I'll re-read the original thread and will spin up new patches (and try to go through the end with it). Executing the KVM unittests is something that we can't afford to lose, so I'd say it's important on this last mile effort to get rid of qemu-kvm.

Quoted 6 lines

Absolutely, IIRC the problem was that testdev did a little bit of everything... let's see what's the functionality of testdev:

- write (port 0xf1), can be replaced in autotest with:
-device isa-debugcon,iobase=0xf1,chardev=...

kvm-unit-tests no longer uses 0xf1 for output. It uses serial.

Quoted 13 lines

- exit code (port 0xf4), see this series:
http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg00818.html

- ram size (port 0xd1). If we can also patch kvm-unittests, the memory is available in the CMOS or in fwcfg. Here is the SeaBIOS code:

        u32 rs = ((inb_cmos(0x34) << 16) | (inb_cmos(0x35) << 24));
        if (rs)
            rs += 16 * 1024 * 1024;
        else
            rs = (((inb_cmos(0x30) << 10) | (inb_cmos(0x31) << 18))
                  + 1 * 1024 * 1024);
Quoted 3 lines

The rest (ports 0xe0..0xe7, 0x2000..0x2017, MMIO) can be left in testdev.

Paolo

--
Gleb.

#15 lore raw
In reply to Paolo Bonzini
Quoted 23 lines · Paolo Bonzini

Il 03/10/2012 12:57, Lucas Meneghel Rodrigues ha scritto:

Quoted 9 lines

Yep, I did send patches with the testdev device present on qemu-kvm.git to qemu.git a while ago, but there were many comments on the review, I ended up not implementing everything that was asked and the patches were archived.

If nobody wants to step up to port it, I'll re-read the original thread and will spin up new patches (and try to go through the end with it). Executing the KVM unittests is something that we can't afford to lose, so I'd say it's important on this last mile effort to get rid of qemu-kvm.

Absolutely, IIRC the problem was that testdev did a little bit of everything... let's see what's the functionality of testdev:

- write (port 0xf1), can be replaced in autotest with:
-device isa-debugcon,iobase=0xf1,chardev=...

- exit code (port 0xf4), see this series:
http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg00818.html

- ram size (port 0xd1). If we can also patch kvm-unittests, the memory is available in the CMOS or in fwcfg. Here is the SeaBIOS code:

Quoted 8 lines
        u32 rs = ((inb_cmos(0x34) << 16) | (inb_cmos(0x35) << 24));
        if (rs)
            rs += 16 * 1024 * 1024;
        else
            rs = (((inb_cmos(0x30) << 10) | (inb_cmos(0x31) << 18))
                  + 1 * 1024 * 1024);

The rest (ports 0xe0..0xe7, 0x2000..0x2017, MMIO) can be left in testdev.

IIRC, one of the biggest problem with testdev was its hack to inject interrupts.

Jan

Signature · 2 lines
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
#16 lore raw
Quoted 31 lines · Jan Kiszka
Quoted 30 lines · Paolo Bonzini

Il 03/10/2012 12:57, Lucas Meneghel Rodrigues ha scritto:

Quoted 9 lines

Yep, I did send patches with the testdev device present on qemu-kvm.git to qemu.git a while ago, but there were many comments on the review, I ended up not implementing everything that was asked and the patches were archived.

If nobody wants to step up to port it, I'll re-read the original thread and will spin up new patches (and try to go through the end with it). Executing the KVM unittests is something that we can't afford to lose, so I'd say it's important on this last mile effort to get rid of qemu-kvm.

Absolutely, IIRC the problem was that testdev did a little bit of everything... let's see what's the functionality of testdev:

- write (port 0xf1), can be replaced in autotest with:
-device isa-debugcon,iobase=0xf1,chardev=...

- exit code (port 0xf4), see this series:
http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg00818.html

- ram size (port 0xd1). If we can also patch kvm-unittests, the memory is available in the CMOS or in fwcfg. Here is the SeaBIOS code:

         u32 rs = ((inb_cmos(0x34) << 16) | (inb_cmos(0x35) << 24));
         if (rs)
             rs += 16 * 1024 * 1024;
         else
             rs = (((inb_cmos(0x30) << 10) | (inb_cmos(0x31) << 18))
                   + 1 * 1024 * 1024);
Quoted 4 lines
Quoted 1 line

The rest (ports 0xe0..0xe7, 0x2000..0x2017, MMIO) can be left in testdev.

IIRC, one of the biggest problem with testdev was its hack to inject interrupts.

Jan, I assume this commit helps to fix this, right?

commit b334ec567f1de9a60349991e7b75083d569ddb0a

Author: Jan Kiszka <[email protected]>
Date:   Fri Mar 2 10:30:47 2012 +0100

qemu-kvm: Use upstream kvm-i8259

Drop the qemu-kvm version in favor of the equivalent upstream implementation. This allows to move the i8259 back into the hwlib.

Note that this also drops the testdev hack and restores proper isa_get_irq. If testdev scripts exist that inject > IRQ15, they need fixing. Testing for these interrupts on the PIIX3 makes no practical sense anyway as those lines are unused.

#17 lore raw
Quoted 48 lines · Lucas Meneghel Rodrigues
Quoted 31 lines · Jan Kiszka
Quoted 30 lines · Paolo Bonzini

Il 03/10/2012 12:57, Lucas Meneghel Rodrigues ha scritto:

Quoted 9 lines

Yep, I did send patches with the testdev device present on qemu-kvm.git to qemu.git a while ago, but there were many comments on the review, I ended up not implementing everything that was asked and the patches were archived.

If nobody wants to step up to port it, I'll re-read the original thread and will spin up new patches (and try to go through the end with it). Executing the KVM unittests is something that we can't afford to lose, so I'd say it's important on this last mile effort to get rid of qemu-kvm.

Absolutely, IIRC the problem was that testdev did a little bit of everything... let's see what's the functionality of testdev:

- write (port 0xf1), can be replaced in autotest with:
-device isa-debugcon,iobase=0xf1,chardev=...

- exit code (port 0xf4), see this series:
http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg00818.html

- ram size (port 0xd1). If we can also patch kvm-unittests, the memory is available in the CMOS or in fwcfg. Here is the SeaBIOS code:

         u32 rs = ((inb_cmos(0x34) << 16) | (inb_cmos(0x35) << 24));
         if (rs)
             rs += 16 * 1024 * 1024;
         else
             rs = (((inb_cmos(0x30) << 10) | (inb_cmos(0x31) << 18))
                   + 1 * 1024 * 1024);
Quoted 4 lines
Quoted 1 line

The rest (ports 0xe0..0xe7, 0x2000..0x2017, MMIO) can be left in testdev.

IIRC, one of the biggest problem with testdev was its hack to inject interrupts.

Jan, I assume this commit helps to fix this, right?

commit b334ec567f1de9a60349991e7b75083d569ddb0a

Author: Jan Kiszka <[email protected]>
Date:   Fri Mar 2 10:30:47 2012 +0100

qemu-kvm: Use upstream kvm-i8259

Drop the qemu-kvm version in favor of the equivalent upstream implementation. This allows to move the i8259 back into the hwlib.

Quoted 7 lines

Note that this also drops the testdev hack and restores proper isa_get_irq. If testdev scripts exist that inject > IRQ15, they need fixing. Testing for these interrupts on the PIIX3 makes no practical sense anyway as those lines are unused.

Yes, this improved it a lot as we no longer depend on additional changes. I'm not sure if there was resistance beyond that.

When cleaning up the code: register_ioport_read must be replaced with the memory API.

Jan

Signature · 2 lines
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
#18 lore raw
Quoted 54 lines · Jan Kiszka
Quoted 53 lines · Lucas Meneghel Rodrigues
Quoted 31 lines · Jan Kiszka
Quoted 30 lines · Paolo Bonzini

Il 03/10/2012 12:57, Lucas Meneghel Rodrigues ha scritto:

Quoted 9 lines

Yep, I did send patches with the testdev device present on qemu-kvm.git to qemu.git a while ago, but there were many comments on the review, I ended up not implementing everything that was asked and the patches were archived.

If nobody wants to step up to port it, I'll re-read the original thread and will spin up new patches (and try to go through the end with it). Executing the KVM unittests is something that we can't afford to lose, so I'd say it's important on this last mile effort to get rid of qemu-kvm.

Absolutely, IIRC the problem was that testdev did a little bit of everything... let's see what's the functionality of testdev:

- write (port 0xf1), can be replaced in autotest with:
-device isa-debugcon,iobase=0xf1,chardev=...

- exit code (port 0xf4), see this series:
http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg00818.html

- ram size (port 0xd1). If we can also patch kvm-unittests, the memory is available in the CMOS or in fwcfg. Here is the SeaBIOS code:

          u32 rs = ((inb_cmos(0x34) << 16) | (inb_cmos(0x35) << 24));
          if (rs)
              rs += 16 * 1024 * 1024;
          else
              rs = (((inb_cmos(0x30) << 10) | (inb_cmos(0x31) << 18))
                    + 1 * 1024 * 1024);
Quoted 4 lines
Quoted 1 line

The rest (ports 0xe0..0xe7, 0x2000..0x2017, MMIO) can be left in testdev.

IIRC, one of the biggest problem with testdev was its hack to inject interrupts.

Jan, I assume this commit helps to fix this, right?

commit b334ec567f1de9a60349991e7b75083d569ddb0a

Author: Jan Kiszka <[email protected]>
Date:   Fri Mar 2 10:30:47 2012 +0100

qemu-kvm: Use upstream kvm-i8259

Drop the qemu-kvm version in favor of the equivalent upstream implementation. This allows to move the i8259 back into the hwlib.

Note that this also drops the testdev hack and restores proper isa_get_irq. If testdev scripts exist that inject > IRQ15, they need fixing. Testing for these interrupts on the PIIX3 makes no practical sense anyway as those lines are unused.

Quoted 8 lines
Quoted 2 lines

Yes, this improved it a lot as we no longer depend on additional changes. I'm not sure if there was resistance beyond that.

When cleaning up the code: register_ioport_read must be replaced with the memory API.

I did look at the MemoryRegionOps/memory_region_init_io and still did not figure out how to port things. I'll send a v2 addressing all the comments made so far but this one, just to see if people are OK with the direction of the full patch, then if you could give me some pointers of how to do this conversion, it'd be great.

#19 lore raw
Quoted 61 lines · Lucas Meneghel Rodrigues
Quoted 60 lines · Jan Kiszka
Quoted 48 lines · Lucas Meneghel Rodrigues
Quoted 31 lines · Jan Kiszka
Quoted 30 lines · Paolo Bonzini

Il 03/10/2012 12:57, Lucas Meneghel Rodrigues ha scritto:

Quoted 9 lines

Yep, I did send patches with the testdev device present on qemu-kvm.git to qemu.git a while ago, but there were many comments on the review, I ended up not implementing everything that was asked and the patches were archived.

If nobody wants to step up to port it, I'll re-read the original thread and will spin up new patches (and try to go through the end with it). Executing the KVM unittests is something that we can't afford to lose, so I'd say it's important on this last mile effort to get rid of qemu-kvm.

Absolutely, IIRC the problem was that testdev did a little bit of everything... let's see what's the functionality of testdev:

- write (port 0xf1), can be replaced in autotest with:
-device isa-debugcon,iobase=0xf1,chardev=...

- exit code (port 0xf4), see this series:
http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg00818.html

- ram size (port 0xd1). If we can also patch kvm-unittests, the memory is available in the CMOS or in fwcfg. Here is the SeaBIOS code:

          u32 rs = ((inb_cmos(0x34) << 16) | (inb_cmos(0x35) << 24));
          if (rs)
              rs += 16 * 1024 * 1024;
          else
              rs = (((inb_cmos(0x30) << 10) | (inb_cmos(0x31) << 18))
                    + 1 * 1024 * 1024);
Quoted 4 lines
Quoted 1 line

The rest (ports 0xe0..0xe7, 0x2000..0x2017, MMIO) can be left in testdev.

IIRC, one of the biggest problem with testdev was its hack to inject interrupts.

Jan, I assume this commit helps to fix this, right?

commit b334ec567f1de9a60349991e7b75083d569ddb0a

Author: Jan Kiszka <[email protected]>
Date:   Fri Mar 2 10:30:47 2012 +0100

qemu-kvm: Use upstream kvm-i8259

Drop the qemu-kvm version in favor of the equivalent upstream implementation. This allows to move the i8259 back into the hwlib.

Quoted 7 lines

Note that this also drops the testdev hack and restores proper isa_get_irq. If testdev scripts exist that inject > IRQ15, they need fixing. Testing for these interrupts on the PIIX3 makes no practical sense anyway as those lines are unused.

Yes, this improved it a lot as we no longer depend on additional changes. I'm not sure if there was resistance beyond that.

Quoted 9 lines
Quoted 2 lines

When cleaning up the code: register_ioport_read must be replaced with the memory API.

I did look at the MemoryRegionOps/memory_region_init_io and still did not figure out how to port things. I'll send a v2 addressing all the comments made so far but this one, just to see if people are OK with the direction of the full patch, then if you could give me some pointers of how to do this conversion, it'd be great.

See e.g. http://thread.gmane.org/gmane.comp.emulators.qemu/171217

Jan

Signature · 2 lines
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
#20 lore raw
In reply to Anthony Liguori
Quoted 12 lines · Anthony Liguori
Quoted 7 lines · Jan Kiszka

On 2012-10-01 11:31, Marcelo Tosatti wrote:

It's not just about default configs. We need to validate if the migration formats are truly compatible (qemu-kvm -> QEMU, the other way around definitely not). For the command line switches, we could provide a wrapper script that translates them into upstream format or simply ignores them. That should be harmless to carry upstream.

qemu-kvm has:

Quoted 8 lines

-no-kvm
-no-kvm-irqchip
-no-kvm-pit
-no-kvm-pit-reinjection
-tdf <- does nothing

There are replacements for all of the above. If we need to add them to qemu.git, it's not big deal to add them.

At the moment the only purpose of this command line options is for compability with scripts. My view is that scripts are easily fixed, so we can just drop them. No need to carry this to QEMU.

Quoted 7 lines

-drive ...,boot= <- this is ignored

cpu_set command for CPU hotplug which is known broken in qemu-kvm.

testdev which is nice but only used for development

Default nic is rtl8139 vs. e1000.

Config file (as suggested earlier on this thread).

Quoted 2 lines

Some logic to move change the default VGA ram size to 16mb for pc-1.2 (QEMU uses 16mb by default now too).

Config file.

Quoted 23 lines

I think at this point, none of this matters but I added the various distro maintainers to the thread.

I think it's time for the distros to drop qemu-kvm and just ship qemu.git. Is there anything else that needs to happen to make that switch?

Regards,

Anthony Liguori

Quoted 12 lines

But I would really stop worrying about the qemu-kvm code base.

Jan

Signature · 6 lines
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#21 lore raw
Quoted 28 lines · Marcelo Tosatti
Quoted 12 lines · Anthony Liguori
Quoted 7 lines · Jan Kiszka

On 2012-10-01 11:31, Marcelo Tosatti wrote:

It's not just about default configs. We need to validate if the migration formats are truly compatible (qemu-kvm -> QEMU, the other way around definitely not). For the command line switches, we could provide a wrapper script that translates them into upstream format or simply ignores them. That should be harmless to carry upstream.

qemu-kvm has:

Quoted 8 lines

-no-kvm
-no-kvm-irqchip
-no-kvm-pit
-no-kvm-pit-reinjection
-tdf <- does nothing

There are replacements for all of the above. If we need to add them to qemu.git, it's not big deal to add them.

At the moment the only purpose of this command line options is for compability with scripts. My view is that scripts are easily fixed, so we can just drop them. No need to carry this to QEMU.

Quoted 2 lines

-drive ...,boot= <- this is ignored

Quoted 7 lines
Quoted 5 lines

cpu_set command for CPU hotplug which is known broken in qemu-kvm.

testdev which is nice but only used for development

Default nic is rtl8139 vs. e1000.

Config file (as suggested earlier on this thread).

If you need to append -config bla, you can also specify the desired NIC explicitly - I see no value in the former. If we decide to mangle a qemu-kvm command line before calling QEMU binaries, we can adjust this variation there. Otherwise it's the same as with all those -kvm*:
Scripts/management tools will need adjustment.

Jan

Signature · 2 lines
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
#22 lore raw
Quoted 11 lines · Jan Kiszka
Quoted 3 lines · Marcelo Tosatti
Quoted 1 line

Default nic is rtl8139 vs. e1000.

Config file (as suggested earlier on this thread).

If you need to append -config bla, you can also specify the desired NIC explicitly - I see no value in the former. If we decide to mangle a qemu-kvm command line before calling QEMU binaries, we can adjust this variation there. Otherwise it's the same as with all those -kvm*:
Scripts/management tools will need adjustment.

I don't think there's a need to "manage" this rtl8139 at all at this level. Let's declare that "qemu-kvm 1.3+" will switch from rtl8139 to e1000 by default as a more suitable in modern world, -- the same way as qemu did the switch earlier, and be done with that. I think.

Note that this is JUST for "-net nic" users, which should be the minority (proper usage is -device rtl8139 or -device e1000, with explicit model).

Thanks,

/mjt

#23 lore raw
Quoted 19 lines · Michael Tokarev
Quoted 11 lines · Jan Kiszka
Quoted 3 lines · Marcelo Tosatti
Quoted 1 line

Default nic is rtl8139 vs. e1000.

Config file (as suggested earlier on this thread).

If you need to append -config bla, you can also specify the desired NIC explicitly - I see no value in the former. If we decide to mangle a qemu-kvm command line before calling QEMU binaries, we can adjust this variation there. Otherwise it's the same as with all those -kvm*:
Scripts/management tools will need adjustment.

I don't think there's a need to "manage" this rtl8139 at all at this level. Let's declare that "qemu-kvm 1.3+" will switch from rtl8139 to e1000 by default as a more suitable in modern world, -- the same way as qemu did the switch earlier, and be done with that. I think.

Quoted 3 lines

Note that this is JUST for "-net nic" users, which should be the minority (proper usage is -device rtl8139 or -device e1000, with explicit model).

Well, the alternative to config files or wrapper scripts is just a detailed checklist "how to migrate from qemu-kvm to QEMU".

Jan

Signature · 2 lines
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
maintainer
#24 lore raw
In reply to Anthony Liguori

Il 01/10/2012 15:19, Anthony Liguori ha scritto:

Quoted 3 lines

I think it's time for the distros to drop qemu-kvm and just ship qemu.git. Is there anything else that needs to happen to make that switch?

Perhaps change the default to -machine accel=kvm:tcg?

Paolo

#25 lore raw
Quoted 6 lines · Paolo Bonzini

Il 01/10/2012 15:19, Anthony Liguori ha scritto:

Quoted 3 lines

I think it's time for the distros to drop qemu-kvm and just ship qemu.git. Is there anything else that needs to happen to make that switch?

Perhaps change the default to -machine accel=kvm:tcg?

That's the old discussion again: This way we generate that "silent failure" (unless you monitor the console output), where users will complain that "QEMU is so slow" because something is blocking KVM. Maybe the risk for the latter is lower these days as modules are auto-loaded now, don't know.

Jan

Signature · 2 lines
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
#26 lore raw
In reply to Paolo Bonzini
Quoted 8 lines · Paolo Bonzini

Il 01/10/2012 15:19, Anthony Liguori ha scritto:

Quoted 3 lines

I think it's time for the distros to drop qemu-kvm and just ship qemu.git. Is there anything else that needs to happen to make that switch?

Perhaps change the default to -machine accel=kvm:tcg?

Paolo

I would be in favor of:

#if defined(CONFIG_KVM)
accel=kvm
#else
accel=tcg
#endif

If KVM is available for your target, you almost certainly want to use it.

I'd be very happy with that change for 1.3.

Regards,

Anthony Liguori

#27 lore raw

PATCH 1/1 kvm: Set default accelerator to "kvm" if the host supports it

If we built a target for a host that supports KVM in principle, set the default accelerator to KVM as well. This also means the start of QEMU will fail to start if KVM support turns out to be unavailable at runtime.

---
 kvm-all.c  |    1 +
 kvm-stub.c |    1 +
 kvm.h      |    1 +
 vl.c       |    4 ++--
 4 files changed, 5 insertions(+), 2 deletions(-)
kvm-all.c +1 -0
index 92a7137..4d5f86c 100644
@@ -103,6 +103,7 @@ struct KVMState
 #endif
 };
 
+bool kvm_configured = true;
 KVMState *kvm_state;
 bool kvm_kernel_irqchip;
 bool kvm_async_interrupts_allowed;
kvm-stub.c +1 -0
index 3c52eb5..86a6451 100644
@@ -17,6 +17,7 @@
 #include "gdbstub.h"
 #include "kvm.h"
 
+bool kvm_configured;
 KVMState *kvm_state;
 bool kvm_kernel_irqchip;
 bool kvm_async_interrupts_allowed;
kvm.h +1 -0
index dea2998..9936e5f 100644
@@ -22,6 +22,7 @@
 #include <linux/kvm.h>
 #endif
 
+extern bool kvm_configured;
 extern int kvm_allowed;
 extern bool kvm_kernel_irqchip;
 extern bool kvm_async_interrupts_allowed;
vl.c +2 -2
index 8d305ca..f557bd1 100644
@@ -2215,8 +2215,8 @@ static int configure_accelerator(void)
     }
 
     if (p == NULL) {
-        /* Use the default "accelerator", tcg */
-        p = "tcg";
+        /* The default accelerator depends on the availability of KVM. */
+        p = kvm_configured ? "kvm" : "tcg";
     }
 
     while (!accel_initialised && *p != '\0') {
Signature · 1 line
1.7.3.4
Acked-by on v1 1/1
#28 lore raw

Re: [PATCH] kvm: Set default accelerator to "kvm" if the host supports it

Quoted 54 lines · Jan Kiszka

If we built a target for a host that supports KVM in principle, set the default accelerator to KVM as well. This also means the start of QEMU will fail to start if KVM support turns out to be unavailable at runtime.

---
 kvm-all.c  |    1 +
 kvm-stub.c |    1 +
 kvm.h      |    1 +
 vl.c       |    4 ++--
 4 files changed, 5 insertions(+), 2 deletions(-)
kvm-all.c +1 -0
index 92a7137..4d5f86c 100644
@@ -103,6 +103,7 @@ struct KVMState
 #endif
 };
 
+bool kvm_configured = true;
 KVMState *kvm_state;
 bool kvm_kernel_irqchip;
 bool kvm_async_interrupts_allowed;
kvm-stub.c +1 -0
index 3c52eb5..86a6451 100644
@@ -17,6 +17,7 @@
 #include "gdbstub.h"
 #include "kvm.h"
 
+bool kvm_configured;
 KVMState *kvm_state;
 bool kvm_kernel_irqchip;
 bool kvm_async_interrupts_allowed;
kvm.h +1 -0
index dea2998..9936e5f 100644
@@ -22,6 +22,7 @@
 #include <linux/kvm.h>
 #endif
 
+extern bool kvm_configured;
 extern int kvm_allowed;
 extern bool kvm_kernel_irqchip;
 extern bool kvm_async_interrupts_allowed;
vl.c +0 -0
index 8d305ca..f557bd1 100644

vl.c static int configure_accelerator(void) · line 2220

Quoted 8 lines
vl.c +2 -2
@@ -2215,8 +2215,8 @@ static int configure_accelerator(void)
     }
 
     if (p == NULL) {
-        /* Use the default "accelerator", tcg */
-        p = "tcg";
+        /* The default accelerator depends on the availability of KVM. */
+        p = kvm_configured ? "kvm" : "tcg";
     }

How about making this an arch_init() function call and then using a #if defined(KVM_CONFIG) in arch_init.c?

I hate to introduce another global variable if we can avoid it...

Otherwise:

Blue/Aurelien, any objections?

Regards,

Anthony Liguori

Quoted 4 lines
vl.c +0 -0
     while (!accel_initialised && *p != '\0') {
Signature · 1 line
1.7.3.4
#29 lore raw

Re: [PATCH] kvm: Set default accelerator to "kvm" if the host supports it

Quoted 68 lines · Anthony Liguori
Quoted 54 lines · Jan Kiszka

If we built a target for a host that supports KVM in principle, set the default accelerator to KVM as well. This also means the start of QEMU will fail to start if KVM support turns out to be unavailable at runtime.

---
 kvm-all.c  |    1 +
 kvm-stub.c |    1 +
 kvm.h      |    1 +
 vl.c       |    4 ++--
 4 files changed, 5 insertions(+), 2 deletions(-)
kvm-all.c +1 -0
index 92a7137..4d5f86c 100644
@@ -103,6 +103,7 @@ struct KVMState
 #endif
 };
 
+bool kvm_configured = true;
 KVMState *kvm_state;
 bool kvm_kernel_irqchip;
 bool kvm_async_interrupts_allowed;
kvm-stub.c +1 -0
index 3c52eb5..86a6451 100644
@@ -17,6 +17,7 @@
 #include "gdbstub.h"
 #include "kvm.h"
 
+bool kvm_configured;
 KVMState *kvm_state;
 bool kvm_kernel_irqchip;
 bool kvm_async_interrupts_allowed;
kvm.h +1 -0
index dea2998..9936e5f 100644
@@ -22,6 +22,7 @@
 #include <linux/kvm.h>
 #endif
 
+extern bool kvm_configured;
 extern int kvm_allowed;
 extern bool kvm_kernel_irqchip;
 extern bool kvm_async_interrupts_allowed;
vl.c +0 -0
index 8d305ca..f557bd1 100644

vl.c static int configure_accelerator(void) · line 2220

Quoted 8 lines
vl.c +2 -2
@@ -2215,8 +2215,8 @@ static int configure_accelerator(void)
     }
 
     if (p == NULL) {
-        /* Use the default "accelerator", tcg */
-        p = "tcg";
+        /* The default accelerator depends on the availability of KVM. */
+        p = kvm_configured ? "kvm" : "tcg";
     }

How about making this an arch_init() function call and then using a #if defined(KVM_CONFIG) in arch_init.c?

Quoted 8 lines

I hate to introduce another global variable if we can avoid it...

Otherwise:

Blue/Aurelien, any objections?

I am not sure this way of doing is really distribution friendly, where the QEMU package is built for a large variety of hosts, some of them maybe without KVM support.

I am all for enabling KVM by default, but I think it should fall back to TCG if it is not enabled (probably with a warning so that the user is aware of that). On the other hand, if the user explicitly enables KVM support with -enable-kvm or with accel=kvm, it should fail to start. In other words, a bit like the configure script options, by default we use auto-detection, but if an option is explicitly enabled, it fails if it can't be enabled.

Signature · 2 lines
Aurelien Jarno                          GPG: 1024D/F1BCDB73
[email protected]                 http://www.aurel32.net
#30 lore raw

Re: [PATCH] kvm: Set default accelerator to "kvm" if the host supports it

In reply to Anthony Liguori
Quoted 68 lines · Anthony Liguori
Quoted 54 lines · Jan Kiszka

If we built a target for a host that supports KVM in principle, set the default accelerator to KVM as well. This also means the start of QEMU will fail to start if KVM support turns out to be unavailable at runtime.

---
 kvm-all.c  |    1 +
 kvm-stub.c |    1 +
 kvm.h      |    1 +
 vl.c       |    4 ++--
 4 files changed, 5 insertions(+), 2 deletions(-)
kvm-all.c +1 -0
index 92a7137..4d5f86c 100644
@@ -103,6 +103,7 @@ struct KVMState
 #endif
 };
 
+bool kvm_configured = true;
 KVMState *kvm_state;
 bool kvm_kernel_irqchip;
 bool kvm_async_interrupts_allowed;
kvm-stub.c +1 -0
index 3c52eb5..86a6451 100644
@@ -17,6 +17,7 @@
 #include "gdbstub.h"
 #include "kvm.h"
 
+bool kvm_configured;
 KVMState *kvm_state;
 bool kvm_kernel_irqchip;
 bool kvm_async_interrupts_allowed;
kvm.h +1 -0
index dea2998..9936e5f 100644
@@ -22,6 +22,7 @@
 #include <linux/kvm.h>
 #endif
 
+extern bool kvm_configured;
 extern int kvm_allowed;
 extern bool kvm_kernel_irqchip;
 extern bool kvm_async_interrupts_allowed;
vl.c +0 -0
index 8d305ca..f557bd1 100644

vl.c static int configure_accelerator(void) · line 2220

Quoted 8 lines
vl.c +2 -2
@@ -2215,8 +2215,8 @@ static int configure_accelerator(void)
     }
 
     if (p == NULL) {
-        /* Use the default "accelerator", tcg */
-        p = "tcg";
+        /* The default accelerator depends on the availability of KVM. */
+        p = kvm_configured ? "kvm" : "tcg";
     }

How about making this an arch_init() function call and then using a #if defined(KVM_CONFIG) in arch_init.c?

Quoted 1 line

I hate to introduce another global variable if we can avoid it...

Hacked too quickly. In fact, kvm_configured is simply kvm_available(). However, resistance appear to be too high here.

Jan

Quoted 20 lines

Otherwise:

Blue/Aurelien, any objections?

Regards,

Anthony Liguori

Quoted 4 lines
vl.c +0 -0
     while (!accel_initialised && *p != '\0') {
Signature · 1 line
1.7.3.4

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

#31 lore raw

Re: [PATCH] kvm: Set default accelerator to "kvm" if the host supports it

In reply to Anthony Liguori
Quoted 68 lines · Anthony Liguori
Quoted 54 lines · Jan Kiszka

If we built a target for a host that supports KVM in principle, set the default accelerator to KVM as well. This also means the start of QEMU will fail to start if KVM support turns out to be unavailable at runtime.

---
 kvm-all.c  |    1 +
 kvm-stub.c |    1 +
 kvm.h      |    1 +
 vl.c       |    4 ++--
 4 files changed, 5 insertions(+), 2 deletions(-)
kvm-all.c +1 -0
index 92a7137..4d5f86c 100644
@@ -103,6 +103,7 @@ struct KVMState
 #endif
 };
 
+bool kvm_configured = true;
 KVMState *kvm_state;
 bool kvm_kernel_irqchip;
 bool kvm_async_interrupts_allowed;
kvm-stub.c +1 -0
index 3c52eb5..86a6451 100644
@@ -17,6 +17,7 @@
 #include "gdbstub.h"
 #include "kvm.h"
 
+bool kvm_configured;
 KVMState *kvm_state;
 bool kvm_kernel_irqchip;
 bool kvm_async_interrupts_allowed;
kvm.h +1 -0
index dea2998..9936e5f 100644
@@ -22,6 +22,7 @@
 #include <linux/kvm.h>
 #endif
 
+extern bool kvm_configured;
 extern int kvm_allowed;
 extern bool kvm_kernel_irqchip;
 extern bool kvm_async_interrupts_allowed;
vl.c +0 -0
index 8d305ca..f557bd1 100644

vl.c static int configure_accelerator(void) · line 2220

Quoted 8 lines
vl.c +2 -2
@@ -2215,8 +2215,8 @@ static int configure_accelerator(void)
     }
 
     if (p == NULL) {
-        /* Use the default "accelerator", tcg */
-        p = "tcg";
+        /* The default accelerator depends on the availability of KVM. */
+        p = kvm_configured ? "kvm" : "tcg";
     }

How about making this an arch_init() function call and then using a #if defined(KVM_CONFIG) in arch_init.c?

Quoted 7 lines

I hate to introduce another global variable if we can avoid it...

Otherwise:

Blue/Aurelien, any objections?

No, maybe a message could be printed that says that the default has changed, for a few releases.

Quoted 9 lines

Regards,

Anthony Liguori

Quoted 4 lines
vl.c +0 -1
     while (!accel_initialised && *p != '\0') {
--

1.7.3.4

#32 lore raw

Re: [Qemu-devel] [PATCH] kvm: Set default accelerator to "kvm" if the host supports it

Quoted 10 lines · Blue Swirl
Quoted 6 lines · Anthony Liguori

vl.c static int configure_accelerator(void) · line 2220

Quoted 3 lines · Jan Kiszka
vl.c +2 -0
+        /* The default accelerator depends on the availability of KVM. */
+        p = kvm_configured ? "kvm" : "tcg";
     }

Blue/Aurelien, any objections?

No, maybe a message could be printed that says that the default has changed, for a few releases.

I've lost track of the conversation, are we currently proposing the accelerator default to be "kvm" (as per the original patch you quote here) or "kvm:tcg" ?

I'm not entirely sure which I prefer from an ARM perspective For some time to come and for a lot of targets (ie any target CPU except A15), having a default of "kvm" is going to cause existing working commandlines to stop working. [I expect that ARM-host qemu binaries will be built with CONFIG_KVM once ARM KVM support lands, but the same binary will be run on hosts without virtualization extensions.] On the other hand, perhaps there just aren't really very many people who run QEMU on ARM hosts, and so we can ignore them :-)

-- PMM

#33 lore raw

Re: [Qemu-devel] [PATCH] kvm: Set default accelerator to "kvm" if the host supports it

Quoted 16 lines · Peter Maydell
Quoted 10 lines · Blue Swirl
Quoted 6 lines · Anthony Liguori

vl.c static int configure_accelerator(void) · line 2220

Quoted 3 lines · Jan Kiszka
vl.c +2 -0
+        /* The default accelerator depends on the availability of KVM. */
+        p = kvm_configured ? "kvm" : "tcg";
    }

Blue/Aurelien, any objections?

No, maybe a message could be printed that says that the default has changed, for a few releases.

I've lost track of the conversation, are we currently proposing the accelerator default to be "kvm" (as per the original patch you quote here) or "kvm:tcg" ?

Quoted 9 lines

I'm not entirely sure which I prefer from an ARM perspective For some time to come and for a lot of targets (ie any target CPU except A15), having a default of "kvm" is going to cause existing working commandlines to stop working. [I expect that ARM-host qemu binaries will be built with CONFIG_KVM once ARM KVM support lands, but the same binary will be run on hosts without virtualization extensions.] On the other hand, perhaps there just aren't really very many people who run QEMU on ARM hosts, and so we can ignore them :-)

We get similar problems on PPC. Take the following example:

  $ qemu-system-ppc -M mpc8544ds -kernel uImage -nographic

That command line would work just fine if you run it on a G5 today. However, if we switch to accel=kvm:tcg, it will stop working, because kvm on the G5 can not virtualize an e500 CPU.

I don't know any good way around that one either the way the code is layered today. We only know the type of CPU we want to create after we made the decision whether we do KVM or not.

Alex

#34 lore raw

Re: [PATCH] kvm: Set default accelerator to "kvm" if the host supports it

Quoted 28 lines · Alexander Graf
Quoted 16 lines · Peter Maydell
Quoted 10 lines · Blue Swirl
Quoted 6 lines · Anthony Liguori

vl.c static int configure_accelerator(void) · line 2220

Quoted 3 lines · Jan Kiszka
vl.c +2 -0
+        /* The default accelerator depends on the availability of KVM. */
+        p = kvm_configured ? "kvm" : "tcg";
    }

Blue/Aurelien, any objections?

No, maybe a message could be printed that says that the default has changed, for a few releases.

I've lost track of the conversation, are we currently proposing the accelerator default to be "kvm" (as per the original patch you quote here) or "kvm:tcg" ?

Quoted 9 lines

I'm not entirely sure which I prefer from an ARM perspective For some time to come and for a lot of targets (ie any target CPU except A15), having a default of "kvm" is going to cause existing working commandlines to stop working. [I expect that ARM-host qemu binaries will be built with CONFIG_KVM once ARM KVM support lands, but the same binary will be run on hosts without virtualization extensions.] On the other hand, perhaps there just aren't really very many people who run QEMU on ARM hosts, and so we can ignore them :-)

Quoted 3 lines

We get similar problems on PPC. Take the following example:

  $ qemu-system-ppc -M mpc8544ds -kernel uImage -nographic

But do you really expect people to do this? I have to believe that people running on PPC hardware and running qemu-system-ppc most likely want to do KVM...

Kernel development seems unlikely... The folks that I know doing PPC kernel development with QEMU still qemu-system-ppc on x86.

Regards,

Anthony Liguori

Quoted 7 lines

That command line would work just fine if you run it on a G5 today. However, if we switch to accel=kvm:tcg, it will stop working, because kvm on the G5 can not virtualize an e500 CPU.

I don't know any good way around that one either the way the code is layered today. We only know the type of CPU we want to create after we made the decision whether we do KVM or not.

Alex

#35 lore raw

Re: [Qemu-devel] [PATCH] kvm: Set default accelerator to "kvm" if the host supports it

Quoted 30 lines · Anthony Liguori
Quoted 28 lines · Alexander Graf
Quoted 25 lines · Peter Maydell
Quoted 10 lines · Blue Swirl
Quoted 6 lines · Anthony Liguori

vl.c static int configure_accelerator(void) · line 2220

Quoted 3 lines · Jan Kiszka
vl.c +2 -0
+        /* The default accelerator depends on the availability of KVM. */
+        p = kvm_configured ? "kvm" : "tcg";
   }

Blue/Aurelien, any objections?

No, maybe a message could be printed that says that the default has changed, for a few releases.

I've lost track of the conversation, are we currently proposing the accelerator default to be "kvm" (as per the original patch you quote here) or "kvm:tcg" ?

I'm not entirely sure which I prefer from an ARM perspective For some time to come and for a lot of targets (ie any target CPU except A15), having a default of "kvm" is going to cause existing working commandlines to stop working. [I expect that ARM-host qemu binaries will be built with CONFIG_KVM once ARM KVM support lands, but the same binary will be run on hosts without virtualization extensions.] On the other hand, perhaps there just aren't really very many people who run QEMU on ARM hosts, and so we can ignore them :-)

Quoted 7 lines
Quoted 3 lines

We get similar problems on PPC. Take the following example:

 $ qemu-system-ppc -M mpc8544ds -kernel uImage -nographic

But do you really expect people to do this? I have to believe that people running on PPC hardware and running qemu-system-ppc most likely want to do KVM...

Sure. But we wouldn't be able to even tell them what went wrong, as we don't have a negotiation mechanism right now that could tell user space "hey, the CPU you selected is unknown to me".

However, if during cpu init we could add such a check and then fall back to tcg mode if accel=kvm:tcg with a warning, that'd be nice user experience.

We could do the same for ARM. If you do -M beagle on an A15 KVM enabled machine, you would still be able to do so, but KVM tells you it can't emulate an A8 right now. And if in the future KVM learns how to expose an A8 on A15, we could just not bail out and things would magically work.

Apart from that, I like the idea of kvm:tcg with a warning as the default for qemu. We should still have a qemu-kvm binary in the distro that does accel=kvm so people don't accidentally fall back to tcg mode.

Alex

#36 lore raw

Re: [Qemu-devel] [PATCH] kvm: Set default accelerator to "kvm" if the host supports it

Quoted 6 lines · Alexander Graf
Quoted 5 lines · Anthony Liguori
Quoted 3 lines · Alexander Graf

We get similar problems on PPC. Take the following example:

 $ qemu-system-ppc -M mpc8544ds -kernel uImage -nographic
Quoted 7 lines
Quoted 3 lines

But do you really expect people to do this? I have to believe that people running on PPC hardware and running qemu-system-ppc most likely want to do KVM...

Sure. But we wouldn't be able to even tell them what went wrong, as we don't have a negotiation mechanism right now that could tell user space "hey, the CPU you selected is unknown to me".

On ARM we will at least be able to say what happened, because our kvm_arch_init_vcpu() will fail (when we try the ioctl to tell the kernel "be this kind of CPU") and we can print a message there. However QEMU as a whole doesn't have any way of falling back to TCG at that point so it just bails out.

-- PMM

#37 lore raw

Re: [Qemu-devel] [PATCH] kvm: Set default accelerator to "kvm" if the host supports it

In reply to Alexander Graf

Am 05.10.2012 04:24, schrieb Alexander Graf:

Quoted 35 lines · Anthony Liguori
Quoted 33 lines
Quoted 30 lines · Alexander Graf
Quoted 16 lines · Peter Maydell
Quoted 10 lines · Blue Swirl
Quoted 6 lines · Anthony Liguori

Review

Quoted 3 lines · Jan Kiszka
(quoted diff) +2 -0
+        /* The default accelerator depends on the availability of KVM. */
+        p = kvm_configured ? "kvm" : "tcg";
   }

Blue/Aurelien, any objections?

No, maybe a message could be printed that says that the default has changed, for a few releases.

I've lost track of the conversation, are we currently proposing the accelerator default to be "kvm" (as per the original patch you quote here) or "kvm:tcg" ?

Quoted 9 lines

I'm not entirely sure which I prefer from an ARM perspective For some time to come and for a lot of targets (ie any target CPU except A15), having a default of "kvm" is going to cause existing working commandlines to stop working. [I expect that ARM-host qemu binaries will be built with CONFIG_KVM once ARM KVM support lands, but the same binary will be run on hosts without virtualization extensions.] On the other hand, perhaps there just aren't really very many people who run QEMU on ARM hosts, and so we can ignore them :-)

We get similar problems on PPC. Take the following example:

Quoted 7 lines
Quoted 5 lines
Quoted 1 line
 $ qemu-system-ppc -M mpc8544ds -kernel uImage -nographic

But do you really expect people to do this? I have to believe that people running on PPC hardware and running qemu-system-ppc most likely want to do KVM...

Sure. But we wouldn't be able to even tell them what went wrong, as we don't have a negotiation mechanism right now that could tell user space "hey, the CPU you selected is unknown to me".

Would it help to split out the cpu_model -> CPUClass lookup from cpu_ppc_init() to invoke a hook or inquire a field indicating KVM support?

Andreas

Quoted 10 lines

However, if during cpu init we could add such a check and then fall back to tcg mode if accel=kvm:tcg with a warning, that'd be nice user experience.

We could do the same for ARM. If you do -M beagle on an A15 KVM enabled machine, you would still be able to do so, but KVM tells you it can't emulate an A8 right now. And if in the future KVM learns how to expose an A8 on A15, we could just not bail out and things would magically work.

Apart from that, I like the idea of kvm:tcg with a warning as the default for qemu. We should still have a qemu-kvm binary in the distro that does accel=kvm so people don't accidentally fall back to tcg mode.

Alex

Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
#38 lore raw

Re: [Qemu-devel] [PATCH] kvm: Set default accelerator to "kvm" if the host supports it

Quoted 38 lines · Andreas Färber

Am 05.10.2012 04:24, schrieb Alexander Graf:

Quoted 37 lines · Anthony Liguori
Quoted 35 lines
Quoted 31 lines · Alexander Graf
Quoted 16 lines · Peter Maydell
Quoted 10 lines · Blue Swirl
Quoted 6 lines · Anthony Liguori

Review

Quoted 3 lines · Jan Kiszka
(quoted diff) +2 -0
+        /* The default accelerator depends on the availability of KVM. */
+        p = kvm_configured ? "kvm" : "tcg";
  }

Blue/Aurelien, any objections?

No, maybe a message could be printed that says that the default has changed, for a few releases.

I've lost track of the conversation, are we currently proposing the accelerator default to be "kvm" (as per the original patch you quote here) or "kvm:tcg" ?

Quoted 9 lines

I'm not entirely sure which I prefer from an ARM perspective For some time to come and for a lot of targets (ie any target CPU except A15), having a default of "kvm" is going to cause existing working commandlines to stop working. [I expect that ARM-host qemu binaries will be built with CONFIG_KVM once ARM KVM support lands, but the same binary will be run on hosts without virtualization extensions.] On the other hand, perhaps there just aren't really very many people who run QEMU on ARM hosts, and so we can ignore them :-)

We get similar problems on PPC. Take the following example:

$ qemu-system-ppc -M mpc8544ds -kernel uImage -nographic
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

But do you really expect people to do this? I have to believe that people running on PPC hardware and running qemu-system-ppc most likely want to do KVM...

Sure. But we wouldn't be able to even tell them what went wrong, as we don't have a negotiation mechanism right now that could tell user space "hey, the CPU you selected is unknown to me".

Would it help to split out the cpu_model -> CPUClass lookup from cpu_ppc_init() to invoke a hook or inquire a field indicating KVM support?

Well, we need to basically determine whether KVM is enabled only after cpu creation of the machine file.

Alex

#39 lore raw

Re: [Qemu-devel] [PATCH] kvm: Set default accelerator to "kvm" if the host supports it

In reply to Jan Kiszka

Hello Jan,

Am 01.10.2012 16:34, schrieb Jan Kiszka:

Quoted 4 lines

If we built a target for a host that supports KVM in principle, set the default accelerator to KVM as well. This also means the start of QEMU will fail to start if KVM support turns out to be unavailable at runtime.

From a distro point of view this of course means that we will build against KVM and that the new KVM default will start to fail for users on very old hardware. Can't we do a runtime check to select the default?

Would be nice to at least amend the commit message with how they are expected to remedy that via command line. -machine accel=tcg?

Regards,
Andreas

Quoted 61 lines
---
 kvm-all.c  |    1 +
 kvm-stub.c |    1 +
 kvm.h      |    1 +
 vl.c       |    4 ++--
 4 files changed, 5 insertions(+), 2 deletions(-)
kvm-all.c +1 -0
index 92a7137..4d5f86c 100644
@@ -103,6 +103,7 @@ struct KVMState
 #endif
 };
 
+bool kvm_configured = true;
 KVMState *kvm_state;
 bool kvm_kernel_irqchip;
 bool kvm_async_interrupts_allowed;
kvm-stub.c +1 -0
index 3c52eb5..86a6451 100644
@@ -17,6 +17,7 @@
 #include "gdbstub.h"
 #include "kvm.h"
 
+bool kvm_configured;
 KVMState *kvm_state;
 bool kvm_kernel_irqchip;
 bool kvm_async_interrupts_allowed;
kvm.h +1 -0
index dea2998..9936e5f 100644
@@ -22,6 +22,7 @@
 #include <linux/kvm.h>
 #endif
 
+extern bool kvm_configured;
 extern int kvm_allowed;
 extern bool kvm_kernel_irqchip;
 extern bool kvm_async_interrupts_allowed;
vl.c +2 -2
index 8d305ca..f557bd1 100644
@@ -2215,8 +2215,8 @@ static int configure_accelerator(void)
     }
 
     if (p == NULL) {
-        /* Use the default "accelerator", tcg */
-        p = "tcg";
+        /* The default accelerator depends on the availability of KVM. */
+        p = kvm_configured ? "kvm" : "tcg";
     }
 
     while (!accel_initialised && *p != '\0') {
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
#40 lore raw

Re: [Qemu-devel] [PATCH] kvm: Set default accelerator to "kvm" if the host supports it

Quoted 11 lines · Andreas Färber

Hello Jan,

Am 01.10.2012 16:34, schrieb Jan Kiszka:

Quoted 4 lines

If we built a target for a host that supports KVM in principle, set the default accelerator to KVM as well. This also means the start of QEMU will fail to start if KVM support turns out to be unavailable at runtime.

From a distro point of view this of course means that we will build against KVM and that the new KVM default will start to fail for users on very old hardware. Can't we do a runtime check to select the default?

NB, this is *not* only about old hardware. There are plenty of users who use QEMU inside VMs. One very common usage I know of is image building tools which are run inside Amazon VMs, using libguestfs & QEMU.

IMHO, default to KVM, fallback to TCG is the most friendly default behaviour.

Daniel

Signature · 4 lines
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|
#41 lore raw

Re: [Qemu-devel] [PATCH] kvm: Set default accelerator to "kvm" if the host supports it

Quoted 9 lines · Daniel P. Berrange
Quoted 8 lines · Andreas Färber

Hello Jan,

Am 01.10.2012 16:34, schrieb Jan Kiszka:

Quoted 4 lines

If we built a target for a host that supports KVM in principle, set the default accelerator to KVM as well. This also means the start of QEMU will fail to start if KVM support turns out to be unavailable at runtime.

Quoted 7 lines
Quoted 3 lines

From a distro point of view this of course means that we will build against KVM and that the new KVM default will start to fail for users on very old hardware. Can't we do a runtime check to select the default?

NB, this is *not* only about old hardware. There are plenty of users who use QEMU inside VMs. One very common usage I know of is image building tools which are run inside Amazon VMs, using libguestfs & QEMU.

But libguest can set it's accelerator option to whatever it wants.

If your running QEMU under a VM, it's pretty reasonable to have to use a special option IMHO.

Quoted 2 lines

IMHO, default to KVM, fallback to TCG is the most friendly default behaviour.

Except if a user expects good network performance and can't understand why they're getting 100kb/s.

Regards,

Anthony Liguori

Quoted 7 lines

Daniel

Signature · 4 lines
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|
maintainer
#42 lore raw

Re: [PATCH] kvm: Set default accelerator to "kvm" if the host supports it

Quoted 5 lines

But libguest can set it's accelerator option to whatever it wants.

If your running QEMU under a VM, it's pretty reasonable to have to use a special option IMHO.

It's also reasonable to have consecutive releases change defaults in a more "friendly" way (i.e. from tcg to kvm:tcg), especially since we'll get users that formerly used qemu-kvm and never had to specify neither -machine accel nor --enable-kvm.

Quoted 5 lines
Quoted 2 lines

IMHO, default to KVM, fallback to TCG is the most friendly default behaviour.

Except if a user expects good network performance and can't understand why they're getting 100kb/s.

libguestfs doesn't need network at all (though I wonder if they could use lxc instead...).

Paolo

#43 lore raw

Re: [Qemu-devel] [PATCH] kvm: Set default accelerator to "kvm" if the host supports it

Quoted 9 lines · Paolo Bonzini
Quoted 4 lines

But libguest can set it's accelerator option to whatever it wants.

If your running QEMU under a VM, it's pretty reasonable to have to use a special option IMHO.

It's also reasonable to have consecutive releases change defaults in a more "friendly" way (i.e. from tcg to kvm:tcg), especially since we'll get users that formerly used qemu-kvm and never had to specify neither -machine accel nor --enable-kvm.

I agree with you except for the 'kvm:tcg' part.

Quoted 9 lines
Quoted 6 lines
Quoted 3 lines

IMHO, default to KVM, fallback to TCG is the most friendly default behaviour.

Except if a user expects good network performance and can't understand why they're getting 100kb/s.

libguestfs doesn't need network at all (though I wonder if they could use lxc instead...).

FWIW, libguestfs already uses -machine accel=kvm:tcg so we can completely the libguestfs use-case for this discussion.

Regards,

Anthony Liguori

Quoted 2 lines

Paolo

#44 lore raw

Re: [PATCH] kvm: Set default accelerator to "kvm" if the host supports it

In reply to Daniel P. Berrange
Quoted 13 lines · Daniel P. Berrange
Quoted 11 lines · Andreas Färber

Hello Jan,

Am 01.10.2012 16:34, schrieb Jan Kiszka:

Quoted 4 lines

If we built a target for a host that supports KVM in principle, set the default accelerator to KVM as well. This also means the start of QEMU will fail to start if KVM support turns out to be unavailable at runtime.

From a distro point of view this of course means that we will build against KVM and that the new KVM default will start to fail for users on very old hardware. Can't we do a runtime check to select the default?

Quoted 6 lines

NB, this is *not* only about old hardware. There are plenty of users who use QEMU inside VMs. One very common usage I know of is image building tools which are run inside Amazon VMs, using libguestfs & QEMU.

IMHO, default to KVM, fallback to TCG is the most friendly default behaviour.

Friendly perhaps, generating an infinite series of questions "why is my guest slow as molasses?" certainly.

And for each instance of the question, there's an unknown number of users who give QEMU a quick try, screw up KVM unknowingly, observe the glacial speed, and conclude it's crap.

#45 lore raw

Re: [Qemu-devel] [PATCH] kvm: Set default accelerator to "kvm" if the host supports it

Quoted 22 lines · Markus Armbruster
Quoted 13 lines · Daniel P. Berrange
Quoted 11 lines · Andreas Färber

Hello Jan,

Am 01.10.2012 16:34, schrieb Jan Kiszka:

Quoted 4 lines

If we built a target for a host that supports KVM in principle, set the default accelerator to KVM as well. This also means the start of QEMU will fail to start if KVM support turns out to be unavailable at runtime.

From a distro point of view this of course means that we will build against KVM and that the new KVM default will start to fail for users on very old hardware. Can't we do a runtime check to select the default?

Quoted 6 lines

NB, this is *not* only about old hardware. There are plenty of users who use QEMU inside VMs. One very common usage I know of is image building tools which are run inside Amazon VMs, using libguestfs & QEMU.

IMHO, default to KVM, fallback to TCG is the most friendly default behaviour.

Quoted 7 lines

Friendly perhaps, generating an infinite series of questions "why is my guest slow as molasses?" certainly.

And for each instance of the question, there's an unknown number of users who give QEMU a quick try, screw up KVM unknowingly, observe the glacial speed, and conclude it's crap.

That's why it should not fallback silently to TCG, but warn the user about that.

On the other hand, on a machine without KVM support (which might just be because of local policy admin policy which doesn't provide access the /dev/kvm, nested virtualization, etc.), if QEMU fails to start (while previous versions were working), the user can conclude that QEMU is crap.

Signature · 2 lines
Aurelien Jarno                          GPG: 1024D/F1BCDB73
[email protected]                 http://www.aurel32.net
#46 lore raw

Re: [Qemu-devel] [PATCH] kvm: Set default accelerator to "kvm" if the host supports it

In reply to Markus Armbruster
Quoted 7 lines · Markus Armbruster
Quoted 2 lines · Daniel P. Berrange

IMHO, default to KVM, fallback to TCG is the most friendly default behaviour.

Friendly perhaps, generating an infinite series of questions "why is my guest slow as molasses?" certainly.

With a warning about "switching to slow emulation mode because .." printed at startup that becomes a non-issue, because there's no reason to ask more questions about why it is slow - it already said why. Yes some may try to ask what to do, which is different.

Every howto nowadays mentions kvm modules and /dev/kvm device permissions.

Quoted 3 lines

And for each instance of the question, there's an unknown number of users who give QEMU a quick try, screw up KVM unknowingly, observe the glacial speed, and conclude it's crap.

This is, again, I think, unfair. With the warning message it becomes more or less obvious.

If you're talking about users who run it with -daemonize argument - this is a) stupid to do when TRYING it out, so it's not a big deal to lose another stupid user, and b) qemu should init everything first and throw all warnings and fatal errors before daemonizing, if this is not the case it should be fixed in the code.

And if you're talking about management software (libvirt and others), it controls all the required privileges already and explicitly requests acceleration and other stuff.

So the best thing to do is what Daniel, Aurelien, Paolo and others are suggested: accel=kvm:tcg with a warning.

Thanks,

/mjt

#47 lore raw

Re: [PATCH] kvm: Set default accelerator to "kvm" if the host supports it

Quoted 30 lines · Michael Tokarev
Quoted 7 lines · Markus Armbruster
Quoted 2 lines · Daniel P. Berrange

IMHO, default to KVM, fallback to TCG is the most friendly default behaviour.

Friendly perhaps, generating an infinite series of questions "why is my guest slow as molasses?" certainly.

With a warning about "switching to slow emulation mode because .." printed at startup that becomes a non-issue, because there's no reason to ask more questions about why it is slow - it already said why. Yes some may try to ask what to do, which is different.

Every howto nowadays mentions kvm modules and /dev/kvm device permissions.

Quoted 3 lines

And for each instance of the question, there's an unknown number of users who give QEMU a quick try, screw up KVM unknowingly, observe the glacial speed, and conclude it's crap.

This is, again, I think, unfair. With the warning message it becomes more or less obvious.

If you're talking about users who run it with -daemonize argument - this is a) stupid to do when TRYING it out, so it's not a big deal to lose another stupid user, and b) qemu should init everything first and throw all warnings and fatal errors before daemonizing, if this is not the case it should be fixed in the code.

Quoted 6 lines

And if you're talking about management software (libvirt and others), it controls all the required privileges already and explicitly requests acceleration and other stuff.

So the best thing to do is what Daniel, Aurelien, Paolo and others are suggested: accel=kvm:tcg with a warning.

Well, we had a lot of problems with such a fallback in the past, but I think we had no proper warnings back then.

I'm not fully believing in users will always realize the console message. I would therefore suggest to change the window title of QEMU as well if we fail to initialize some accelerator. Something like "QEMU without KVM" (could be "QEMU without $FAILED_ACCEL" in the end, i.e. not just for KVM). If that makes sense for everyone, I'll hack the required patches.

Jan

#48 lore raw
In reply to Anthony Liguori
Quoted 10 lines · Anthony Liguori
Quoted 7 lines · Jan Kiszka

On 2012-10-01 11:31, Marcelo Tosatti wrote:

It's not just about default configs. We need to validate if the migration formats are truly compatible (qemu-kvm -> QEMU, the other way around definitely not). For the command line switches, we could provide a wrapper script that translates them into upstream format or simply ignores them. That should be harmless to carry upstream.

Quoted 3 lines

qemu-kvm has:

-no-kvm

Should be easy to have around as backwards compat hack. All it needs to do is set accel=tcg.

Review

Quoted 3 lines
(quoted diff) +0 -3
-no-kvm-irqchip
-no-kvm-pit
-no-kvm-pit-reinjection

Those are quite important, as we need cmdline backwards compatibility.

Quoted 6 lines

-tdf <- does nothing

There are replacements for all of the above. If we need to add them to qemu.git, it's not big deal to add them.

-drive ...,boot= <- this is ignored

It's ignored, but useful for certain things. I don't know how many of our users use boot= today, but it's certainly still in the code, and supported. I honestly wouldn't mind to carry a SUSE specific patch that implements boot= for now until we can deem it deprecated enough that we can drop it.

Quoted 5 lines

cpu_set command for CPU hotplug which is known broken in qemu-kvm.

testdev which is nice but only used for development

Default nic is rtl8139 vs. e1000.

Couldn't we have a machine option that tells us that -M pc-1.2 is really -M pc-kvm-1.2? That way we could implement the device difference, right?

Alex

Quoted 28 lines

Some logic to move change the default VGA ram size to 16mb for pc-1.2 (QEMU uses 16mb by default now too).

I think at this point, none of this matters but I added the various distro maintainers to the thread.

I think it's time for the distros to drop qemu-kvm and just ship qemu.git. Is there anything else that needs to happen to make that switch?

Regards,

Anthony Liguori

Quoted 12 lines

But I would really stop worrying about the qemu-kvm code base.

Jan

Signature · 6 lines
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
maintainer
#49 lore raw

Il 01/10/2012 16:07, Alexander Graf ha scritto:

Quoted 7 lines
Quoted 1 line
Quoted 1 line

-drive ...,boot= <- this is ignored

It's ignored, but useful for certain things. I don't know how many of our users use boot= today, but it's certainly still in the code, and supported. I honestly wouldn't mind to carry a SUSE specific patch that implements boot= for now until we can deem it deprecated enough that we can drop it.

Extboot is not going to be backported to QEMU, but SeaBIOS can now boot from all emulated device models except MegaSAS. So we may at most have just the option, for example making it an alias for bootindex=1 on the corresponding device.

Paolo

#50 lore raw
In reply to Anthony Liguori
Quoted 33 lines
Quoted 1 line
Quoted 1 line · Anthony Liguori
Quoted 7 lines · Jan Kiszka

On 2012-10-01 11:31, Marcelo Tosatti wrote:

It's not just about default configs. We need to validate if the migration formats are truly compatible (qemu-kvm -> QEMU, the other way around definitely not). For the command line switches, we could provide a wrapper script that translates them into upstream format or simply ignores them. That should be harmless to carry upstream.

qemu-kvm has:

-no-kvm
-no-kvm-irqchip
-no-kvm-pit
-no-kvm-pit-reinjection
-tdf <- does nothing

There are replacements for all of the above. If we need to add them to qemu.git, it's not big deal to add them.

-drive ...,boot= <- this is ignored

cpu_set command for CPU hotplug which is known broken in qemu-kvm.

testdev which is nice but only used for development

Default nic is rtl8139 vs. e1000.

Some logic to move change the default VGA ram size to 16mb for pc-1.2 (QEMU uses 16mb by default now too).

Quoted 6 lines

I think at this point, none of this matters but I added the various distro maintainers to the thread.

I think it's time for the distros to drop qemu-kvm and just ship qemu.git. Is there anything else that needs to happen to make that switch?

We are seriously considering moving to qemu.git for our SP3 release of SUSE SLES 11. There are just a handful of patches that provide the backwards compatibility we need to maintain (default to kvm, default nic model, vga ram size), so assuming there is a 100% commitment to fully supporting kvm in qemu going forward (which I don't doubt) I think this is a good time for us to make that switch.

Bruce

#51 lore raw
In reply to Anthony Liguori
Quoted 7 lines · Anthony Liguori

Jan Kiszka <[email protected]> writes:
I think at this point, none of this matters but I added the various distro maintainers to the thread.

I think it's time for the distros to drop qemu-kvm and just ship qemu.git. Is there anything else that needs to happen to make that switch?

If that is upstream's recommendation, then I see no issue with switching Fedora 19 / RHEL-7 to use qemu.git instead of qemu-kvm.git.

Regards,
Daniel

Signature · 4 lines
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|