[Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Mark McLoughlin]

From: Adam Jackson <ajax@redhat.com>

We're using PCI_CLASS_DISPLAY_OTHER now, but qemu-kvm.git is using
PCI_CLASS_OTHERS because:

  "As a PCI_CLASS_DISPLAY_OTHER, it reduces primary display somehow on
   Windows XP (possibly Windows disables acceleration since it fails
   to find a driver)."

While this is valid, many versions of X will get confused by it.
Class major number of 0 gets treated as a possibly prehistoric VGA
device, and then the autoconfig logic gets confused trying to figure
out whether the virtio console or the pv vga device are the real VGA.

We should really set a proper class ID. 0x0780 (serial / other) seems
most appropriate. This shouldn't require any kernel changes, the
modalias for virtio looks like:

  alias:          pci:v00001AF4d*sv*sd*bc*sc*i*

so won't care what the base class or subclass are.

It shows up in the guest as:

  00:05.0 Communication controller: Qumranet, Inc. Virtio console

Signed-off-by: Adam Jackson <ajax@redhat.com>
Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 hw/pci_ids.h    |    2 ++
 hw/virtio-pci.c |    2 +-
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/hw/pci_ids.h b/hw/pci_ids.h
index 427fcd5..cbd9a32 100644
--- a/hw/pci_ids.h
+++ b/hw/pci_ids.h
@@ -35,6 +35,8 @@
 #define PCI_CLASS_BRIDGE_PCI             0x0604
 #define PCI_CLASS_BRIDGE_OTHER           0x0680
 
+#define PCI_CLASS_SERIAL_OTHER           0x0780
+
 #define PCI_CLASS_PROCESSOR_CO           0x0b40
 #define PCI_CLASS_PROCESSOR_POWERPC      0x0b20
 
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index c072423..642e744 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -304,7 +304,7 @@ static void virtio_console_init_pci(PCIDevice *pci_dev)
     virtio_init_pci(proxy, vdev,
                     PCI_VENDOR_ID_REDHAT_QUMRANET,
                     PCI_DEVICE_ID_VIRTIO_CONSOLE,
-                    PCI_CLASS_DISPLAY_OTHER,
+                    PCI_CLASS_SERIAL_OTHER,
                     0x00);
 }
 
-- 
1.6.2.2

Re: [Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Avi Kivity]

Mark McLoughlin wrote:
> From: Adam Jackson <ajax@redhat.com>
>
> We're using PCI_CLASS_DISPLAY_OTHER now, but qemu-kvm.git is using
> PCI_CLASS_OTHERS because:
>
>   "As a PCI_CLASS_DISPLAY_OTHER, it reduces primary display somehow on
>    Windows XP (possibly Windows disables acceleration since it fails
>    to find a driver)."
>   

That's really unnecessary now, since virtio-console is not enabled by 
default.

> While this is valid, many versions of X will get confused by it.
> Class major number of 0 gets treated as a possibly prehistoric VGA
> device, and then the autoconfig logic gets confused trying to figure
> out whether the virtio console or the pv vga device are the real VGA.
>
> We should really set a proper class ID. 0x0780 (serial / other) seems
> most appropriate. This shouldn't require any kernel changes, the
> modalias for virtio looks like:
>   

Yes, serial is a much better fit.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Anthony Liguori]

Mark McLoughlin wrote:
> From: Adam Jackson <ajax@redhat.com>
>
> We're using PCI_CLASS_DISPLAY_OTHER now, but qemu-kvm.git is using
> PCI_CLASS_OTHERS because:
>
>   "As a PCI_CLASS_DISPLAY_OTHER, it reduces primary display somehow on
>    Windows XP (possibly Windows disables acceleration since it fails
>    to find a driver)."
>
> While this is valid, many versions of X will get confused by it.
> Class major number of 0 gets treated as a possibly prehistoric VGA
> device, and then the autoconfig logic gets confused trying to figure
> out whether the virtio console or the pv vga device are the real VGA.
>
> We should really set a proper class ID. 0x0780 (serial / other) seems
> most appropriate. This shouldn't require any kernel changes, the
> modalias for virtio looks like:
>
>   alias:          pci:v00001AF4d*sv*sd*bc*sc*i*
>
> so won't care what the base class or subclass are.
>
> It shows up in the guest as:
>
>   00:05.0 Communication controller: Qumranet, Inc. Virtio console
>
> Signed-off-by: Adam Jackson <ajax@redhat.com>
> Signed-off-by: Mark McLoughlin <markmc@redhat.com>
>   

We need a mechanism to toggle this for both this and virtio-blk.  The 
reason a toggle is needed is so that 0.11 can create the same device 
model as 0.10.

In principle though, I think both changes are reasonable, we just can't 
do it unconditionally.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Mark McLoughlin]

On Wed, 2009-05-27 at 09:21 -0500, Anthony Liguori wrote:

> We need a mechanism to toggle this for both this and virtio-blk.  The 
> reason a toggle is needed is so that 0.11 can create the same device 
> model as 0.10.

Okay, so the scenario is:

  - 0.10 guest running on source machine

  - migrate to dest machine running 0.11

  - the device model cannot change or the guest OS will get confused

How about running qemu on the dest machine with -model-version=0.10 ?

At least that way it's something the management system can easily figure
out (i.e. source is running in qemu 0.10, rather than source qemu is
using class=XXX).

Nasty thing about it is that the distributions couldn't cherry-pick
changes like this because they'd deviate from the device model contract
for a given upstream version.

Could we not put this stuff in savevm and change the device model
post-init?

Cheers,
Mark.

Re: [Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Anthony Liguori]

Mark McLoughlin wrote:
> On Wed, 2009-05-27 at 09:21 -0500, Anthony Liguori wrote:
>
>   
>> We need a mechanism to toggle this for both this and virtio-blk.  The 
>> reason a toggle is needed is so that 0.11 can create the same device 
>> model as 0.10.
>>     
>
> Okay, so the scenario is:
>
>   - 0.10 guest running on source machine
>
>   - migrate to dest machine running 0.11
>
>   - the device model cannot change or the guest OS will get confused
>   

Correct, but in this case, the problem is when you shutdown and start up 
the VM again.  In the process, it may trigger something like a driver 
reinstallation or worse, reactivation.  It's necessary to be able to 
prevent this by creating exactly the same device model that was 
previously created.

This is can't really be a hard rule yet without the machine config, but 
we should do our best when we can.

> How about running qemu on the dest machine with -model-version=0.10 ?
>   

Linear versioning is a bit too fickle esp due to distro backporting.

> Nasty thing about it is that the distributions couldn't cherry-pick
> changes like this because they'd deviate from the device model contract
> for a given upstream version.
>
> Could we not put this stuff in savevm and change the device model
> post-init?
>   

It's shutdown/restart that's the problem.  Windows uses a lot of 
different information to calculate whether activation is required again 
and even small changes can end up pushing things over the edge.

Regards,

Anthony Liguori

> Cheers,
> Mark.
>
>   

Re: [Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Mark McLoughlin]

On Wed, 2009-05-27 at 17:37 -0500, Anthony Liguori wrote:
> Mark McLoughlin wrote:
> > On Wed, 2009-05-27 at 09:21 -0500, Anthony Liguori wrote:
> >
> >   
> >> We need a mechanism to toggle this for both this and virtio-blk.  The 
> >> reason a toggle is needed is so that 0.11 can create the same device 
> >> model as 0.10.
> >>     
> >
> > Okay, so the scenario is:
> >
> >   - 0.10 guest running on source machine
> >
> >   - migrate to dest machine running 0.11
> >
> >   - the device model cannot change or the guest OS will get confused
> >   
> 
> Correct, but in this case, the problem is when you shutdown and start up 
> the VM again.  In the process, it may trigger something like a driver 
> reinstallation or worse, reactivation.  It's necessary to be able to 
> prevent this by creating exactly the same device model that was 
> previously created.

Okay.

> This is can't really be a hard rule yet without the machine config, but 
> we should do our best when we can.

There's not much point in doing this unless there's some practical way
to use it.

If we e.g. add a 'class' parameter for virtio-blk and virtio-console,
we'd also need to add some way for a management tool to figure out what
the class value is when it initially creates the guest and, for ever
more, supply that value.

Options I see:

  1) Add a monitor command to query the class values and delay actually 
     changing the values until the next release cycle

  2) Use 'info version' to achieve this - management tools would need to
     start retaining the version that guests were created with and
     supplying that version to qemu on the command line. As you say,
     though, linear versioning sucks

  3) Decide we can only make these guarantees with the machine config
     stuff - management tools would need to query a running qemu for a
     detailed machine config when initially creating the guest and
     always supply that when later starting the guest

Cheers,
Mark.

Re: [Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Anthony Liguori]

Mark McLoughlin wrote:
> On Wed, 2009-05-27 at 17:37 -0500, Anthony Liguori wrote:
>   
>> This is can't really be a hard rule yet without the machine config, but 
>> we should do our best when we can.
>>     
>
> There's not much point in doing this unless there's some practical way
> to use it.
>
> If we e.g. add a 'class' parameter for virtio-blk and virtio-console,
> we'd also need to add some way for a management tool to figure out what
> the class value is when it initially creates the guest and, for ever
> more, supply that value.
>
> Options I see:
>
>   1) Add a monitor command to query the class values and delay actually 
>      changing the values until the next release cycle
>
>   2) Use 'info version' to achieve this - management tools would need to
>      start retaining the version that guests were created with and
>      supplying that version to qemu on the command line. As you say,
>      though, linear versioning sucks
>
>   3) Decide we can only make these guarantees with the machine config
>      stuff - management tools would need to query a running qemu for a
>      detailed machine config when initially creating the guest and
>      always supply that when later starting the guest
>   

Today, the qdev stuff supports properties for devices.  If you make
class a property of these devices, you would just need something to dump
the device properties for a given device.

I think it should be a relatively straight forward thing today.

Regards,

Anthony Liguori

> Cheers,
> Mark.
>
>   

Re: [Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Paul Brook]

On Wednesday 27 May 2009, Mark McLoughlin wrote:
> On Wed, 2009-05-27 at 09:21 -0500, Anthony Liguori wrote:
> > We need a mechanism to toggle this for both this and virtio-blk.  The
> > reason a toggle is needed is so that 0.11 can create the same device
> > model as 0.10.
>
> Okay, so the scenario is:
>
>   - 0.10 guest running on source machine
>
>   - migrate to dest machine running 0.11
>
>   - the device model cannot change or the guest OS will get confused

IMHO think the only sane response is "don't do that". Trying to support 
migration between different qemu versions just isn't worth the pain.

Paul

Re: [Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Anthony Liguori]

Paul Brook wrote:
> On Wednesday 27 May 2009, Mark McLoughlin wrote:
>   
>> On Wed, 2009-05-27 at 09:21 -0500, Anthony Liguori wrote:
>>     
>>> We need a mechanism to toggle this for both this and virtio-blk.  The
>>> reason a toggle is needed is so that 0.11 can create the same device
>>> model as 0.10.
>>>       
>> Okay, so the scenario is:
>>
>>   - 0.10 guest running on source machine
>>
>>   - migrate to dest machine running 0.11
>>
>>   - the device model cannot change or the guest OS will get confused
>>     
>
> IMHO think the only sane response is "don't do that". Trying to support 
> migration between different qemu versions just isn't worth the pain.
>   

It is very worth the pain.  I consider it a core requirement.

Supporting legacy workloads is an important aspect of virtualization. 
If we cannot provide a consistent environment as we evolve over time,
then we cannot support legacy workloads.

It really isn't that hard, it just requires that we exercise care.

Regards,

Anthony Liguori

> Paul
>   

Re: [Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Daniel P. Berrange]

On Thu, May 28, 2009 at 01:53:49PM +0100, Paul Brook wrote:
> On Wednesday 27 May 2009, Mark McLoughlin wrote:
> > On Wed, 2009-05-27 at 09:21 -0500, Anthony Liguori wrote:
> > > We need a mechanism to toggle this for both this and virtio-blk.  The
> > > reason a toggle is needed is so that 0.11 can create the same device
> > > model as 0.10.
> >
> > Okay, so the scenario is:
> >
> >   - 0.10 guest running on source machine
> >
> >   - migrate to dest machine running 0.11
> >
> >   - the device model cannot change or the guest OS will get confused
> 
> IMHO think the only sane response is "don't do that". Trying to support 
> migration between different qemu versions just isn't worth the pain.

I think that further more, we shouldn't make a change like altering
the PCI device class in the stable branch, only in the unstable 
branch. If we restrict the stable branch to bug fixes, then it ought
to be (more?) practical to support save under 0.10 and restore under
0.11, without needing to support the hard problem of save under 0.10
and restore under 1.00 (or whatever next major release branch is)

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Anthony Liguori]

Daniel P. Berrange wrote:
> On Thu, May 28, 2009 at 01:53:49PM +0100, Paul Brook wrote:
>   
>> On Wednesday 27 May 2009, Mark McLoughlin wrote:
>>     
>>> On Wed, 2009-05-27 at 09:21 -0500, Anthony Liguori wrote:
>>>       
>>>> We need a mechanism to toggle this for both this and virtio-blk.  The
>>>> reason a toggle is needed is so that 0.11 can create the same device
>>>> model as 0.10.
>>>>         
>>> Okay, so the scenario is:
>>>
>>>   - 0.10 guest running on source machine
>>>
>>>   - migrate to dest machine running 0.11
>>>
>>>   - the device model cannot change or the guest OS will get confused
>>>       
>> IMHO think the only sane response is "don't do that". Trying to support 
>> migration between different qemu versions just isn't worth the pain.
>>     
>
> I think that further more, we shouldn't make a change like altering
> the PCI device class in the stable branch, only in the unstable 
> branch. If we restrict the stable branch to bug fixes, then it ought
> to be (more?) practical to support save under 0.10 and restore under
> 0.11, without needing to support the hard problem of save under 0.10
> and restore under 1.00 (or whatever next major release branch is)
>   

This is not a save/restore issue.  PCI config will be saved in the image
so everything is fine until you shut a VM down.

The problem is that the device model a guest sees is an ABI.  If we
change the ABI between versions, the guest make break.  What I'm
proposing is that while we can change the default guest ABI between
versions, we must provide a way to recreate older forms of the guest ABI.

A good example of guests breaking is Windows activation failing.  It has
happened more than once that changes in QEMU cause Windows guests to
force reactivation.  In more extreme instances, a guest may break that
used to work before.

Regards,

Anthony Liguori

> Regards,
> Daniel
>   

Re: [Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Paul Brook]

> > IMHO think the only sane response is "don't do that". Trying to support
> > migration between different qemu versions just isn't worth the pain.
>
> I think that further more, we shouldn't make a change like altering
> the PCI device class in the stable branch, only in the unstable
> branch. If we restrict the stable branch to bug fixes, then it ought
> to be (more?) practical to support save under 0.10 and restore under
> 0.11, without needing to support the hard problem of save under 0.10
> and restore under 1.00 (or whatever next major release branch is)

0.11 is the next major release series. The current stable releases are 0.10.x.
I agree we shouldn't be making this sort of change on the stable branch, 
precisely because it's reasonable to expect stable releases to be backwards 
compatible.

Paul

Re: [Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Paul Brook]

> >>   - the device model cannot change or the guest OS will get confused
> >
> > IMHO think the only sane response is "don't do that". Trying to support
> > migration between different qemu versions just isn't worth the pain.
>
> It is very worth the pain.  I consider it a core requirement.

We disagree then. You're effectively requiring bug-compatibility.
This may be reasonable for a stable branch, but is not something I have any 
interest in across different release cycles. IMHO major VM upgrades should be 
considered the same as real hardware or firmware upgrades.

Paul

Re: [Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Anthony Liguori]

Paul Brook wrote:
>>>>   - the device model cannot change or the guest OS will get confused
>>>>         
>>> IMHO think the only sane response is "don't do that". Trying to support
>>> migration between different qemu versions just isn't worth the pain.
>>>       
>> It is very worth the pain.  I consider it a core requirement.
>>     
>
> We disagree then. You're effectively requiring bug-compatibility.
>   

You can take it to an extreme and require bug-compatibility, but I don't
think that's necessary.  I think there's room for being reasonable.

> This may be reasonable for a stable branch, but is not something I have any 
> interest in across different release cycles. IMHO major VM upgrades should be 
> considered the same as real hardware or firmware upgrades.
>   

I think major VM upgrades is something that can be considered
occasionally but every 6 months would be pretty painful to most users. 
I think we ought to make a best effort to maintain compatibility with
older versions as long as it's reasonable.

Regards,

Anthony Liguori

> Paul
>   

Re: [Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Dor Laor]

Anthony Liguori wrote:
> Paul Brook wrote:
>   
>>>>>   - the device model cannot change or the guest OS will get confused
>>>>>         
>>>>>           
>>>> IMHO think the only sane response is "don't do that". Trying to support
>>>> migration between different qemu versions just isn't worth the pain.
>>>>       
>>>>         
>>> It is very worth the pain.  I consider it a core requirement.
>>>     
>>>       
>> We disagree then. You're effectively requiring bug-compatibility.
>>   
>>     
>
> You can take it to an extreme and require bug-compatibility, but I don't
> think that's necessary.  I think there's room for being reasonable.
>
>   
>> This may be reasonable for a stable branch, but is not something I have any 
>> interest in across different release cycles. IMHO major VM upgrades should be 
>> considered the same as real hardware or firmware upgrades.
>>   
>>     
>
> I think major VM upgrades is something that can be considered
> occasionally but every 6 months would be pretty painful to most users. 
> I think we ought to make a best effort to maintain compatibility with
> older versions as long as it's reasonable.
>
>   
Supporting live migration between different versions of qemu is a 
stretch goal. In the past
2 month we fixed about 10 basic live migration issues and mainline keeps 
breaking ;(

Nevertheless, as Anthony states, guest ABI should be rock stable.
We should have mechanism (machine conf format++) that will enable us
to configure pci addresses, cpuid entries, memory layout, device 
existence (hpet, virtio-console,..), etc.

Anyhow, I did sent a patch to allow virtio block to parametrized class 
value -
http://lists.gnu.org/archive/html/qemu-devel/2009-05/msg01189.html
There wasn't a perfect match with finding the right interface to change 
it, happy to hear
other alternatives if you're not happy with it.

Thanks,
Dor

> Regards,
>
> Anthony Liguori
>
>   
>> Paul
>>   
>>     
>
>
>
>   

Re: [Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Mark McLoughlin]

On Fri, 2009-05-29 at 00:45 +0300, Dor Laor wrote:

> Nevertheless, as Anthony states, guest ABI should be rock stable.
> We should have mechanism (machine conf format++) that will enable us
> to configure pci addresses, cpuid entries, memory layout, device 
> existence (hpet, virtio-console,..), etc.
> 
> Anyhow, I did sent a patch to allow virtio block to parametrized class 
> value -
> http://lists.gnu.org/archive/html/qemu-devel/2009-05/msg01189.html
> There wasn't a perfect match with finding the right interface to change 
> it, happy to hear
> other alternatives if you're not happy with it.

As I said, it's pointless to add something like this if, realistically,
it will never be used.

So, taking the example of '-drive class=foo' ... a user who runs qemu
directly would have to know that when she updates from qemu-0.10.x to
qemu-0.11.x, she needs to use '-drive class=384' for ever more. I find
it hard to believe anyone will ever do that.

Perhaps management tools can hide this complexity? In order to do this
in libvirt, we'd need to do the following:

  1) Add a 'pci_class' property to <device><target>:

    <disk type='file' device='disk'>
      <source file='foo.img' format='raw'/>
      <target dev='vda' bus='virtio' pci_class='384'/>
    </disk>

  2) If a guest's configuration does not contain the pci_class
     property, then once the guest is running, parse the output of 'info
     pci', somehow map each disk to a PCI address, map the class 
     descriptions to class numbers and save the class to the guests 
     configuration.

  3) If a given qemu binary supports '-drive class=', then pass it the 
     class value on startup, if configured

  4) Require users to update to libvirt and run the guest at least once 
     with the old version of qemu, before updating to the new version 
     of qemu

Assuming this is actually possible to implement (see "somehow" in step
2), I doubt anyone would ever be bothered enough to implement it.

Anthony's suggestion of a way to list device properties would help a
little in theory, but in practice the old qemu binary wouldn't have this
command so libvirt would have no way of finding this class=384 value.

A machine configuration format (even assuming a command to dump the
configuration of a guest) doesn't help much more, for the same reason.

And none of this solves the problem that management tools have to play
catch up - they cannot predict how we will change the ABI, so users have
to update to a newer version of the management tools with code to
support the ABI change *before* updating qemu.

The more I think about it, no matter how much linear ABI versioning
sucks, it's possibly the only way to solve this in a reasonably usable
manners. Distros would just have to suck it up and agree that if they
cherry-pick an ABI changing patch, they must update the entire ABI to
the newer upstream ABI version.

Cheers,
Mark.

Re: [Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Anthony Liguori]

Mark McLoughlin wrote:
> On Fri, 2009-05-29 at 00:45 +0300, Dor Laor wrote:
>   
> The more I think about it, no matter how much linear ABI versioning
> sucks, it's possibly the only way to solve this in a reasonably usable
> manners. Distros would just have to suck it up and agree that if they
> cherry-pick an ABI changing patch, they must update the entire ABI to
> the newer upstream ABI version.
>   

Can we somehow utilize the save/restore versioning infrastructure?  I
think the general idea of universally versioning devices is not a bad
one but I'm not sure whether we need an additional version id or whether
we can just piggy back on the savevm version.

Regards,

Anthony Liguori

> Cheers,
> Mark.
>
>   

Re: [Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Anthony Liguori]

Mark McLoughlin wrote:
> The more I think about it, no matter how much linear ABI versioning
> sucks, it's possibly the only way to solve this in a reasonably usable
> manners. Distros would just have to suck it up and agree that if they
> cherry-pick an ABI changing patch, they must update the entire ABI to
> the newer upstream ABI version.
>   

If we modify pci_qdev_register() to take the savevm functions and pass
the qdev to each handler as a convention, we could then leverage the
version id of the device to implement the device versioning logic.  We
would also be able to query what the current versions are in the monitor.

Moreover, we could walk the device tree before savevm to validate that
every device in the tree supports savevm which means we can fail
gracefully in when the chosen device model doesn't support live migration.

It's a large refactoring, but mostly mechanical.  Any takers?

Regards,

Anthony Liguori

> Cheers,
> Mark.
>
>   

Re: [Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Mark McLoughlin]

On Fri, 2009-05-29 at 10:43 +0100, Mark McLoughlin wrote:

> The more I think about it, no matter how much linear ABI versioning
> sucks, it's possibly the only way to solve this in a reasonably usable
> manners. Distros would just have to suck it up and agree that if they
> cherry-pick an ABI changing patch, they must update the entire ABI to
> the newer upstream ABI version.

Okay, how about this:

  - Add a saveabi monitor command

  - Whenever libvirt starts a guest or hotplugs a device, it executes
    saveabi and retains the output

  - The abi can be restored with qemu -loadabi or the loadabi monitor 
    command

  - The abi file doesn't describe the device model, it merely gives 
    hints for building the device model which is described on the
    command line

  - If the abi file contains details of a device which is not listed on 
    the command line, it's just ignored and not included in the next 
    saveabi

  - If the abi file is missing details of a device which is listed on 
    the command line, the device is constructed using the defaults and 
    included in the next saveabi

  - This means the abi file is opaque to the management tools - unlike 
    the machine config file, libvirt would not need to modify it when
    devices are added or removed by the user

Cheers,
Mark.

Re: [Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Avi Kivity]

Anthony Liguori wrote:
> Mark McLoughlin wrote:
>   
>> On Fri, 2009-05-29 at 00:45 +0300, Dor Laor wrote:
>>   
>> The more I think about it, no matter how much linear ABI versioning
>> sucks, it's possibly the only way to solve this in a reasonably usable
>> manners. Distros would just have to suck it up and agree that if they
>> cherry-pick an ABI changing patch, they must update the entire ABI to
>> the newer upstream ABI version.
>>   
>>     
>
> Can we somehow utilize the save/restore versioning infrastructure?  I
> think the general idea of universally versioning devices is not a bad
> one but I'm not sure whether we need an additional version id or whether
> we can just piggy back on the savevm version.
>   

The savevm version can be updated due to fixes in the savevm protocol 
that don't affect the guest visible information at all.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Dor Laor]

Mark McLoughlin wrote:
> On Fri, 2009-05-29 at 00:45 +0300, Dor Laor wrote:
>
>   
>> Nevertheless, as Anthony states, guest ABI should be rock stable.
>> We should have mechanism (machine conf format++) that will enable us
>> to configure pci addresses, cpuid entries, memory layout, device 
>> existence (hpet, virtio-console,..), etc.
>>
>> Anyhow, I did sent a patch to allow virtio block to parametrized class 
>> value -
>> http://lists.gnu.org/archive/html/qemu-devel/2009-05/msg01189.html
>> There wasn't a perfect match with finding the right interface to change 
>> it, happy to hear
>> other alternatives if you're not happy with it.
>>     
>
> As I said, it's pointless to add something like this if, realistically,
> it will never be used.
>
> So, taking the example of '-drive class=foo' ... a user who runs qemu
> directly would have to know that when she updates from qemu-0.10.x to
> qemu-0.11.x, she needs to use '-drive class=384' for ever more. I find
> it hard to believe anyone will ever do that.
>   
'She' is not a user, she is a developer.
Users use management tools like libvirt below.
Running qemu directly is complicated and error prone for
manual/human users.
I just wanted to stress this out, more feedback in the next email.
> Perhaps management tools can hide this complexity? In order to do this
> in libvirt, we'd need to do the following:
>   

Re: [Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Dor Laor]

Mark McLoughlin wrote:
> On Fri, 2009-05-29 at 10:43 +0100, Mark McLoughlin wrote:
>
>   
>> The more I think about it, no matter how much linear ABI versioning
>> sucks, it's possibly the only way to solve this in a reasonably usable
>> manners. Distros would just have to suck it up and agree that if they
>> cherry-pick an ABI changing patch, they must update the entire ABI to
>> the newer upstream ABI version.
>>     
>
> Okay, how about this:
>
>   - Add a saveabi monitor command
>
>   - Whenever libvirt starts a guest or hotplugs a device, it executes
>     saveabi and retains the output
>
>   - The abi can be restored with qemu -loadabi or the loadabi monitor 
>     command
>
>   - The abi file doesn't describe the device model, it merely gives 
>     hints for building the device model which is described on the
>     command line
>
>   - If the abi file contains details of a device which is not listed on 
>     the command line, it's just ignored and not included in the next 
>     saveabi
>
>   - If the abi file is missing details of a device which is listed on 
>     the command line, the device is constructed using the defaults and 
>     included in the next saveabi
>
>   - This means the abi file is opaque to the management tools - unlike 
>     the machine config file, libvirt would not need to modify it when
>     devices are added or removed by the user
>   
IMO it shouldn't be opaque and we might use the same config file for the 
abi too.
The notion of abi config is indeed required and most of the times, mgmt 
tools won't
need to deal with it.

But, let's say for instance that the user with certain abi configs, now 
change some
existence of pci device, bios memory mapping, etc. In the case mgmt 
should automatically
adjust both config files to minimize the effect on the guest and not to 
create a conflict.

> Cheers,
> Mark.
>
>
>
>   

Re: [Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Jamie Lokier]

Anthony Liguori wrote:
> The problem is that the device model a guest sees is an ABI.  If we
> change the ABI between versions, the guest make break.  What I'm
> proposing is that while we can change the default guest ABI between
> versions, we must provide a way to recreate older forms of the guest ABI.
> 
> A good example of guests breaking is Windows activation failing.  It has
> happened more than once that changes in QEMU cause Windows guests to
> force reactivation.  In more extreme instances, a guest may break that
> used to work before.

I agree.

As far as I'm concerned, a major use of virtualisation is to be able
to run old guests on newer hosts without having to keep changing the
guest OS, including reinstalling its drivers etc.

Because of this guest ABI compatibility is perhaps more important than
it would be for othe rapplications.

Windows activation is particularly ugly, but maybe we can special case
that by making known relevant serial numbers, asset tags
etc. configurable in the device model?

You can avoid problems by sticking with a fixed QEMU/KVM version of
course, but then you don't get the benefits of development like
performance improvements and bug fixes.  And in the long run, it
doesn't work because older QEMU/KVMs don't work properly (or at all)
on later host systems.

If newer Qemu's can't do that, we'll eventually be in the curious
position of having to use nested virtualisation - to run an older Qemu
on an older nested 'host'.  Sounds annoying :-)

As soon as the device model config file is used, I expect all this
will get easier to manage than it is now.  The main thing then will be
to ensure there are options to recreate old guest modes for particular
devices.  There's probably just a few such options, so not much of a
maintenance burden.

-- Jamie

Re: [Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Mark McLoughlin]

On Sun, 2009-05-31 at 17:35 +0300, Dor Laor wrote:
> Mark McLoughlin wrote:
> > On Fri, 2009-05-29 at 00:45 +0300, Dor Laor wrote:
> >
> >   
> >> Nevertheless, as Anthony states, guest ABI should be rock stable.
> >> We should have mechanism (machine conf format++) that will enable us
> >> to configure pci addresses, cpuid entries, memory layout, device 
> >> existence (hpet, virtio-console,..), etc.
> >>
> >> Anyhow, I did sent a patch to allow virtio block to parametrized class 
> >> value -
> >> http://lists.gnu.org/archive/html/qemu-devel/2009-05/msg01189.html
> >> There wasn't a perfect match with finding the right interface to change 
> >> it, happy to hear
> >> other alternatives if you're not happy with it.
> >>     
> >
> > As I said, it's pointless to add something like this if, realistically,
> > it will never be used.
> >
> > So, taking the example of '-drive class=foo' ... a user who runs qemu
> > directly would have to know that when she updates from qemu-0.10.x to
> > qemu-0.11.x, she needs to use '-drive class=384' for ever more. I find
> > it hard to believe anyone will ever do that.
> >   
> 'She' is not a user, she is a developer.
> Users use management tools like libvirt below.
> Running qemu directly is complicated and error prone for
> manual/human users.

Agreed, which suggests (to me, at least) that the important thing to
think about is how management tools would use any support we add for
maintaining guest ABI.

Cheers,
Mark.

Re: [Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Mark McLoughlin]

On Sun, 2009-05-31 at 17:47 +0300, Dor Laor wrote:
> Mark McLoughlin wrote:
> > On Fri, 2009-05-29 at 10:43 +0100, Mark McLoughlin wrote:
> >
> >   
> >> The more I think about it, no matter how much linear ABI versioning
> >> sucks, it's possibly the only way to solve this in a reasonably usable
> >> manners. Distros would just have to suck it up and agree that if they
> >> cherry-pick an ABI changing patch, they must update the entire ABI to
> >> the newer upstream ABI version.
> >>     
> >
> > Okay, how about this:
> >
> >   - Add a saveabi monitor command
> >
> >   - Whenever libvirt starts a guest or hotplugs a device, it executes
> >     saveabi and retains the output
> >
> >   - The abi can be restored with qemu -loadabi or the loadabi monitor 
> >     command
> >
> >   - The abi file doesn't describe the device model, it merely gives 
> >     hints for building the device model which is described on the
> >     command line
> >
> >   - If the abi file contains details of a device which is not listed on 
> >     the command line, it's just ignored and not included in the next 
> >     saveabi
> >
> >   - If the abi file is missing details of a device which is listed on 
> >     the command line, the device is constructed using the defaults and 
> >     included in the next saveabi
> >
> >   - This means the abi file is opaque to the management tools - unlike 
> >     the machine config file, libvirt would not need to modify it when
> >     devices are added or removed by the user
> >   
> IMO it shouldn't be opaque

The important requirement is that management tools should never need to
modify saveabi output.

> and we might use the same config file for the abi too.

What we don't want is:

  $> qemu -config guest.config

where guest.config contains both details of which devices are needed
*and* what their ABI should be.

We want:

  $> qemu -loadabi guest.abi -config guest.confg

or:

  $> qemu -loadabi guest.abi -drive ... -net ... -serial ...

The idea being that we should not mix up ABI requirements with device
configuration.

The management tools do not need to know the details of the ABI
provided, they just want to request qemu to use the same ABI as was used
previously.

Put more simply - with saveabi/loadabi, the management tools would not
need to know that older versions of qemu's virtio-console used
PCI_CLASS_DISPLAY_OTHER.

> The notion of abi config is indeed required and most of the times, mgmt 
> tools won't need to deal with it.
> 
> But, let's say for instance that the user with certain abi configs, now 
> change some existence of pci device, bios memory mapping, etc. In the
> case mgmt should automatically adjust both config files to minimize
> the effect on the guest and not to create a conflict.

I think that would defeat a lot of the value of this. It adds an awful
lot of complexity to the management tools.

The device config file should always just take precedence and the abi
file should just be used as hints to allow the same ABI to be used. If
e.g. a device is removed, the hints for that device can be ignored and
dropped in the next saveabi.

Cheers,
Mark.

Re: [Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Dor Laor]

Mark McLoughlin wrote:
> On Sun, 2009-05-31 at 17:47 +0300, Dor Laor wrote:
>   
>> Mark McLoughlin wrote:
>>     
>>> On Fri, 2009-05-29 at 10:43 +0100, Mark McLoughlin wrote:
>>>
>>>   
>>>       
>>>> The more I think about it, no matter how much linear ABI versioning
>>>> sucks, it's possibly the only way to solve this in a reasonably usable
>>>> manners. Distros would just have to suck it up and agree that if they
>>>> cherry-pick an ABI changing patch, they must update the entire ABI to
>>>> the newer upstream ABI version.
>>>>     
>>>>         
>>> Okay, how about this:
>>>
>>>   - Add a saveabi monitor command
>>>
>>>   - Whenever libvirt starts a guest or hotplugs a device, it executes
>>>     saveabi and retains the output
>>>
>>>   - The abi can be restored with qemu -loadabi or the loadabi monitor 
>>>     command
>>>
>>>   - The abi file doesn't describe the device model, it merely gives 
>>>     hints for building the device model which is described on the
>>>     command line
>>>
>>>   - If the abi file contains details of a device which is not listed on 
>>>     the command line, it's just ignored and not included in the next 
>>>     saveabi
>>>
>>>   - If the abi file is missing details of a device which is listed on 
>>>     the command line, the device is constructed using the defaults and 
>>>     included in the next saveabi
>>>
>>>   - This means the abi file is opaque to the management tools - unlike 
>>>     the machine config file, libvirt would not need to modify it when
>>>     devices are added or removed by the user
>>>   
>>>       
>> IMO it shouldn't be opaque
>>     
>
> The important requirement is that management tools should never need to
> modify saveabi output.
>
>   
>> and we might use the same config file for the abi too.
>>     
>
> What we don't want is:
>
>   $> qemu -config guest.config
>
> where guest.config contains both details of which devices are needed
> *and* what their ABI should be.
>
> We want:
>
>   $> qemu -loadabi guest.abi -config guest.confg
>
> or:
>
>   $> qemu -loadabi guest.abi -drive ... -net ... -serial ...
>
> The idea being that we should not mix up ABI requirements with device
> configuration.
>
> The management tools do not need to know the details of the ABI
> provided, they just want to request qemu to use the same ABI as was used
> previously.
>
> Put more simply - with saveabi/loadabi, the management tools would not
> need to know that older versions of qemu's virtio-console used
> PCI_CLASS_DISPLAY_OTHER.
>
>   
Since -loadabi  and -config are far away in the future (hope few months) and
the 2 pci class issues are a real problem now, can qemu accept current 
patches?
My patch gave a config option for setting the value of the block class.

Current guest abi is not kept anyway since there is not way for static 
pci slots and
there is no migration working with older qemu's.

IMHO since there is not good way to allow have a real stable guest abi, 
these patches
should be committed (with a cmdline option).

Regards, dor
>> The notion of abi config is indeed required and most of the times, mgmt 
>> tools won't need to deal with it.
>>
>> But, let's say for instance that the user with certain abi configs, now 
>> change some existence of pci device, bios memory mapping, etc. In the
>> case mgmt should automatically adjust both config files to minimize
>> the effect on the guest and not to create a conflict.
>>     
>
> I think that would defeat a lot of the value of this. It adds an awful
> lot of complexity to the management tools.
>
> The device config file should always just take precedence and the abi
> file should just be used as hints to allow the same ABI to be used. If
> e.g. a device is removed, the hints for that device can be ignored and
> dropped in the next saveabi.
>
> Cheers,
> Mark.
>
>
>
>   

Re: [Qemu-devel] [PATCH] Change virtio-console to PCI_CLASS_SERIAL_OTHER

[Mark McLoughlin]

On Tue, 2009-06-02 at 16:08 +0300, Dor Laor wrote:

> IMHO since there is not good way to allow have a real stable guest
> abi, these patches should be committed

Agree - anything reasonable we've come up with so far won't help with
the 0.10 to 0.11 transition, so unless we delay the patches to 0.12,
there's no benefit to blocking them.

> (with a cmdline option).

We don't need more unused command line options in qemu :-)

Cheers,
Mark.