On Thu, 12 Jun 2014 13:59:59 +0300
"Michael S. Tsirkin" <m...@redhat.com> wrote:
> On Thu, Jun 12, 2014 at 12:50:56PM +0200, Greg Kurz wrote:
> > On Thu, 12 Jun 2014 12:39:27 +0200
> > Alexander Graf <ag...@suse.de> wrote:
> >
> > >
> > >
> > > > Am 12.06.2014 um 12:14 schrieb Greg Kurz <gk...@linux.vnet.ibm.com>:
> > > >
> > > > On Thu, 12 Jun 2014 11:43:20 +0200
> > > > Alexander Graf <ag...@suse.de> wrote:
> > > >
> > > >>
> > > >>> On 12.06.14 11:39, Paolo Bonzini wrote:
> > > >>> Il 12/06/2014 11:37, Michael S. Tsirkin ha scritto:
> > > >>>> Maybe just drop unnecessary stuff for new machine types?
> > > >>>> Then we won't need hacks to migrate it.
> > > >>>
> > > >>> For any machine type it's trivial to migrate it. All machine types
> > > >>> including old ones can disregard the migrated values.
> > > >>
> > > >> How about a patch like this before the actual LE awareness ones? With
> > > >> this we should make virtio-serial config space completely independent
> > > >> of
> > > >> live migration.
> > > >>
> > > >> Also since QEMU versions that do read these swapped values during
> > > >> migration are not bi-endian aware, we can never get into a case where
> > > >> a
> > > >> cross-endian save needs to be considered ;).
> > > >>
> > > >>
> > > >> Alex
> > > >>
> > > >>
> > > >> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
> > > >> index 2b647b6..73cb9b7 100644
> > > >> --- a/hw/char/virtio-serial-bus.c
> > > >> +++ b/hw/char/virtio-serial-bus.c
> > > >> @@ -670,6 +670,7 @@ static int virtio_serial_load(QEMUFile *f, void
> > > >> *opaque, int version_id)
> > > >> uint32_t max_nr_ports, nr_active_ports, ports_map;
> > > >> unsigned int i;
> > > >> int ret;
> > > >> + uint32_t tmp;
> > > >>
> > > >> if (version_id > 3) {
> > > >> return -EINVAL;
> > > >> @@ -685,17 +686,12 @@ static int virtio_serial_load(QEMUFile *f, void
> > > >> *opaque, int version_id)
> > > >> return 0;
> > > >> }
> > > >>
> > > >> - /* The config space */
> > > >> - qemu_get_be16s(f, &s->config.cols);
> > > >> - qemu_get_be16s(f, &s->config.rows);
> > > >> -
> > > >> - qemu_get_be32s(f, &max_nr_ports);
> > > >> - tswap32s(&max_nr_ports);
> > > >> - if (max_nr_ports > tswap32(s->config.max_nr_ports)) {
> > > >> - /* Source could have had more ports than us. Fail migration.
> > > >> */
> > > >> - return -EINVAL;
> > > >> - }
> > > >> + /* Unused */
> > > >> + qemu_get_be16s(f, &tmp);
> > > >> + qemu_get_be16s(f, &tmp);
> > > >> + qemu_get_be32s(f, &tmp);
> > > >>
> > > >> + max_nr_ports = tswap32(s->config.max_nr_ports);
> > > >> for (i = 0; i < (max_nr_ports + 31) / 32; i++) {
> > > >> qemu_get_be32s(f, &ports_map);
> > > >
> > > > For the moment, we have 0 < max_nr_ports < 32 so the source
> > > > machine only stores a single 32 bit value... If this limit
> > > > gets raised, we can end up sending more than that... and
> > > > only the source machine max_nr_ports value can give the
> > > > information...
> > >
> > > Why? This value only ever gets set in realize, so it will not change
> > > during the lifetime of the device - which means we don't need to migrate
> > > it.
> > >
> >
> > I agree with the fact that the value does not change and should not be
> > migrated in the first place.
> > I am just worried about the size of the active ports bitmap that is
> > streamed in the for loop... it
> > is only 32 bit as of today, because we are limited to 32 ports. How would
> > this work if the limit is
> > raised ? How can the destination machine know how many bits have to be read
> > ?
>
> When the destination machine is started with -M 2.1, it
> knows that it has to read 32 bit.
> If started with -M 3.0 it reads in 42 bits :)
>
Okay ! I was completely missing this point... now things make sense at last ! :)
About Alex's patch, will you or Amit or Anthony apply it directly or should
I send it along with my patches for a full review ?
Thanks for your time.
--
Gregory Kurz kurzg...@fr.ibm.com
gk...@linux.vnet.ibm.com
Software Engineer @ IBM/Meiosys http://www.ibm.com
Tel +33 (0)562 165 496
"Anarchy is about taking complete responsibility for yourself."
Alan Moore.
