qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Amit Shah <amit.shah@redhat.com>
To: David Gibson <david@gibson.dropbear.id.au>
Cc: mst@redhat.com, aik@ozlabs.ru, rusty@rustcorp.com.au,
	qemu-devel@nongnu.org, agraf@suse.de, borntraeger@de.ibm.com,
	mdroth@us.ibm.com
Subject: Re: [Qemu-devel] [PATCHv2] Fix virtio-serial migration on bi-endian targets
Date: Tue, 16 Dec 2014 11:37:23 +0530	[thread overview]
Message-ID: <20141216060723.GC19675@grmbl.mre> (raw)
In-Reply-To: <20141216053815.GL23547@voom.fritz.box>

On (Tue) 16 Dec 2014 [16:38:15], David Gibson wrote:
> On Tue, Dec 16, 2014 at 10:08:49AM +0530, Amit Shah wrote:
> > On (Tue) 16 Dec 2014 [15:33:54], David Gibson wrote:
> > > On Tue, Dec 16, 2014 at 09:43:30AM +0530, Amit Shah wrote:

> > > > Can you split this patch so the config change and the max_nr_ports
> > > > change are separate?  The max_nr_ports could similarly be ignored by
> > > > dest, right?
> > > 
> > > Um.. I'm not exactly sure where you're drawing the distinction between
> > > the two parts.  Are you thinking
> > >     patch 1) make config.max_nr_ports unused, by using
> > >              max_virtserial_ports instead
> > >     patch 2) eliminate the config field entirely
> > 
> > For this patch, I'm just suggesting to only touch cols and rows, and
> > lines that touch the max_nr_ports can be put in 2/2.  Eliminating
> > config entirely may not be desirable, but if you want to do that, and
> > mark all fields as 'unused' in the savevm/loadvm functions, go for
> > it :-)
> 
> That division really doesn't make sense to me.  If rows and cols are
> removed, but max_nr_ports stays, then get_config has to become a
> weird hybrid where it copies some stuff directly from ->config and
> other bits from elsewhere.
> 
> I really don't see any reason keeping config would be a desirable
> thing: it's a cache of information that doesn't need to be cached, and
> just adds complexity because of the endian issues.

Oh please go ahead and do it -- my only point here is to split the
patchset into logical units, not wrt the total intent of the series.

		Amit

      reply	other threads:[~2014-12-16  6:08 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-12  5:26 [Qemu-devel] [PATCHv2] Fix virtio-serial migration on bi-endian targets David Gibson
2014-12-15 14:59 ` Alexander Graf
2014-12-16  1:19   ` David Gibson
2014-12-16  4:13 ` Amit Shah
2014-12-16  4:33   ` David Gibson
2014-12-16  4:38     ` Amit Shah
2014-12-16  5:38       ` David Gibson
2014-12-16  6:07         ` Amit Shah [this message]

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=20141216060723.GC19675@grmbl.mre \
    --to=amit.shah@redhat.com \
    --cc=agraf@suse.de \
    --cc=aik@ozlabs.ru \
    --cc=borntraeger@de.ibm.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=mdroth@us.ibm.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rusty@rustcorp.com.au \
    /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).