qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Juan Quintela <quintela@redhat.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	qemu-devel@nongnu.org, dgilbert@redhat.com
Subject: Re: [Qemu-devel] [PATCH 2/5] exec: qemu_ram_alloc_device, qemu_ram_resize
Date: Wed, 19 Nov 2014 11:45:15 +0100	[thread overview]
Message-ID: <878uj7o4ec.fsf@elfo.elfo> (raw)
In-Reply-To: <20141119102136.GC26395@redhat.com> (Michael S. Tsirkin's message of "Wed, 19 Nov 2014 12:21:36 +0200")

"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Wed, Nov 19, 2014 at 11:11:26AM +0100, Juan Quintela wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> > On Wed, Nov 19, 2014 at 10:19:22AM +0100, Juan Quintela wrote:


>> qemu -M pc-2.0 -L BIOS-2.0.bin
>> And that way for every version and every bios.  If they are the same,
>> a symlink does.  If they are not, different file.  And we would not have
>> this problems.
>
> You want to keep old bios around forever, and never fix
> bugs in it?  I disagree, quite strongly.

One thing is fix bugs, and a different one is completely change the way
the tables/data are generated.

About keeping old bios forever, no.  Only while the machine types that
depend on that BIOS are supported.  At the very point that they get not
supported, we can drop it.
>
>> 
>> I fully agree that we have problems with BIOS every relase.  What we
>> don't agree is _what_ is the best way to fix the issue.
>> 
>
>
> You don't seem to want to fix them. Your solution is just to keep
> bugs around forver.

That is unfair.  It is the same that if I answer that your solution is
just paper over the bugs that we have already foound and hope that there
are no more bugs.  Do you think that describes your position?  Mine is
also not described but your statement.

>> > We have many devices that just get N from stream, do malloc(N), then read
>> > data from stream into it.
>> > You think it's unsafe? Go ahead and fix them all.
>> 
>> I am sure it is unsafe.  Fixing them requires incompatible changes, that
>> is the whole point :-(
>
> I don't see the point, sorry.  Want to elaborate?

In general, I don't have specific examples:
- when we have a string buffer, we sent/receive it with:

        VMSTATE_BUFFER(queue.data, PS2State),

We are sending whatever the size of queue.data is on source to whatever
the queue.data is on destination.  We are hopping that they are the
same.

In this case, if sizes are different, we got just a migration stream
that is out of sync.  And if attacker modifies stream,  bad things could happen.
In the case of buffers/arrays (so far it appears that everyplace that
recives a size from the network, it checks that it is small enough).

>
>> > However, my patch does address your concern: callers specify the upper
>> > limit on the region size.
>> > Trying to migrate in a 1Gbyte region
>> 
>> Yes and no.  You are assuming that a guest launched with a device ram
>> size of 256KB receives a 512KB section and it knows what to do with it.
>> 
>> It knows what to do with the 256KB section, with the 512KB answer is
>> always a "perhaps".  It depends of what is on the extra space.
>> 
>> My solution would be that RAM/ROM sizes are always the same for
>> migration, so destination would always understand it.
>> 
>> It just forbids migrating between different machine types.  And I think
>> that is good, not bad.
>> 
>> Later, Juan.
>
> You basically ask firmware to be perfect, or want us to carry old bugs
> around forever.  It makes things simple for migration code, at huge cost
> elsewhere, and pain for users.
>
> I don't want to maintain old bugs in ACPI, and same applies to
> other firmware.
>
> This is really the whole point of the patchset.
> Keep bugs in device ram around until next reboot.
> At that point users get new device with non buggy
> behaviour.

False.

qemu-2.2 -M pc-2.0 -> qemu-2.0 -M pc-2.0

You migrate that way, and you go from "new-good" BIOS to "bad-old" BIOS.

I think the question here is, when we do this migration:

qemu-2.0 -M pc-2.0 -> qemu-2.2 -M pc-2.0

what BIOS should the second qemu use?  the one that existed on qemu-2.0
time or the one that existed on qemu-2.2 time?  You can allow for
bugfixes, but not for big changes like moving where the acpi tables were
generated, etc, etc.

I really think that we should use the BIOS from qemu-2.0 era.  And my
understanding is that you defend that we should use the qemu-2.2 era
BIOS.

I think that is the whole point of the discussion.  Have I at least
framed correctly what the discussion is about?

Later, Juan.

  reply	other threads:[~2014-11-19 10:45 UTC|newest]

Thread overview: 82+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-17 20:08 [Qemu-devel] [PATCH 0/5] pc: make ROMs resizeable Michael S. Tsirkin
2014-11-17 20:08 ` [Qemu-devel] [PATCH 1/5] cpu: add cpu_physical_memory_clear_dirty_range_nocode Michael S. Tsirkin
2014-11-19  9:10   ` Juan Quintela
2014-11-17 20:08 ` [Qemu-devel] [PATCH 2/5] exec: qemu_ram_alloc_device, qemu_ram_resize Michael S. Tsirkin
2014-11-17 20:15   ` Michael S. Tsirkin
2014-11-18  6:03   ` Paolo Bonzini
2014-11-18  7:49     ` Michael S. Tsirkin
2014-11-19  9:19       ` Juan Quintela
2014-11-19  9:33         ` Michael S. Tsirkin
2014-11-19 10:11           ` Juan Quintela
2014-11-19 10:21             ` Michael S. Tsirkin
2014-11-19 10:45               ` Juan Quintela [this message]
2014-11-19 13:28                 ` Michael S. Tsirkin
2014-11-19 13:44                   ` Paolo Bonzini
2014-11-19 13:57                     ` Juan Quintela
2014-11-19 14:13                       ` Dr. David Alan Gilbert
2014-11-19 14:22                         ` Paolo Bonzini
2014-11-19 14:26                           ` Dr. David Alan Gilbert
2014-11-19 14:28                             ` Paolo Bonzini
2014-11-19 14:59                               ` Dr. David Alan Gilbert
2014-11-19 15:38                                 ` Michael S. Tsirkin
2014-11-19 15:53                                   ` Dr. David Alan Gilbert
2014-11-19 14:22                         ` Juan Quintela
2014-11-19 14:20                       ` Paolo Bonzini
2014-11-19 16:39                         ` Juan Quintela
2014-11-19 16:56                           ` Paolo Bonzini
2014-11-19 16:27                     ` Kevin O'Connor
2014-11-19 17:01                       ` Paolo Bonzini
2014-11-20  8:12                         ` Gerd Hoffmann
2014-11-20 10:00                           ` Paolo Bonzini
2014-11-19 13:49                   ` Juan Quintela
2014-11-19 13:51                     ` Paolo Bonzini
2014-11-19 14:03                       ` Juan Quintela
2014-11-19 14:11                         ` Paolo Bonzini
2014-11-19 14:16                           ` Dr. David Alan Gilbert
2014-11-19 14:28                             ` Paolo Bonzini
2014-11-19 14:20                           ` Juan Quintela
2014-11-19 15:43                             ` Michael S. Tsirkin
2014-11-19 10:16           ` Markus Armbruster
2014-11-19 10:30             ` Michael S. Tsirkin
2014-11-19 10:50               ` Juan Quintela
2014-11-19 13:36                 ` Michael S. Tsirkin
2014-11-19 13:51                   ` Juan Quintela
2014-11-19 15:46                     ` Michael S. Tsirkin
2014-11-19 16:45                       ` Juan Quintela
2014-11-19 18:28                         ` Paolo Bonzini
2014-11-20 13:35               ` Markus Armbruster
2014-11-20 14:04                 ` Michael S. Tsirkin
2014-11-24 13:48                   ` Paolo Bonzini
2014-11-19 13:58   ` Peter Maydell
2014-11-19 14:07     ` Juan Quintela
2014-11-19 14:10       ` Peter Maydell
2014-11-19 14:18         ` Juan Quintela
2014-11-19 16:08           ` Stefan Hajnoczi
2014-11-19 14:19         ` Paolo Bonzini
2014-11-19 14:21           ` Peter Maydell
2014-11-19 14:30             ` Paolo Bonzini
2014-11-19 15:16               ` Michael S. Tsirkin
2014-11-19 16:50           ` Juan Quintela
2014-11-19 15:12         ` Michael S. Tsirkin
2014-11-19 15:04     ` Michael S. Tsirkin
2014-11-17 20:08 ` [Qemu-devel] [PATCH 3/5] arch_init: support resizing on incoming migration Michael S. Tsirkin
2014-11-17 20:08 ` [Qemu-devel] [PATCH 4/5] memory: interface to allocate device ram Michael S. Tsirkin
2014-11-17 20:21   ` Peter Maydell
2014-11-18 11:54     ` Michael S. Tsirkin
2014-11-17 20:09 ` [Qemu-devel] [PATCH 5/5] acpi-build: make ROMs device RAM, make them resizeable Michael S. Tsirkin
2014-11-17 20:11 ` [Qemu-devel] [PATCH 0/5] pc: make ROMs resizeable Michael S. Tsirkin
2014-11-19  7:29   ` Amit Shah
2014-11-18 14:47 ` Markus Armbruster
2014-11-18 15:00   ` Michael S. Tsirkin
2014-11-19  8:16     ` Markus Armbruster
2014-11-19 13:41       ` Michael S. Tsirkin
2014-11-19  7:31 ` Amit Shah
2014-11-19  8:15   ` Michael S. Tsirkin
2014-11-19  8:22     ` Amit Shah
2014-11-19 13:33       ` Michael S. Tsirkin
2014-11-19 13:52         ` Juan Quintela
2014-11-19 16:01           ` Michael S. Tsirkin
2014-11-19 13:52   ` Peter Maydell
2014-11-19 14:41     ` Paolo Bonzini
2014-11-19 15:34       ` Michael S. Tsirkin
2014-11-19 16:40       ` Juan Quintela

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=878uj7o4ec.fsf@elfo.elfo \
    --to=quintela@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).