From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:47274)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <lcapitulino@redhat.com>) id 1W39BI-0004b0-3D
	for qemu-devel@nongnu.org; Tue, 14 Jan 2014 13:57:48 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <lcapitulino@redhat.com>) id 1W39BD-0002mr-70
	for qemu-devel@nongnu.org; Tue, 14 Jan 2014 13:57:44 -0500
Date: Tue, 14 Jan 2014 13:29:56 -0500
From: Luiz Capitulino <lcapitulino@redhat.com>
Message-ID: <20140114132956.1c21c68c@redhat.com>
In-Reply-To: <52D56E5B.9000001@msgid.tls.msk.ru>
References: <20140109095816.409cd2a9@redhat.com>
	<52D56E5B.9000001@msgid.tls.msk.ru>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH] virtio-balloon: don't hardcode config size
 value
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Michael Tokarev <mjt@tls.msk.ru>
Cc: qemu-trivial <qemu-trivial@nongnu.org>, Peter Crosthwaite <peter.crosthwaite@xilinx.com>, qemu-devel <qemu-devel@nongnu.org>, mst@redhat.com

On Tue, 14 Jan 2014 21:05:31 +0400
Michael Tokarev <mjt@tls.msk.ru> wrote:

> 09.01.2014 18:58, Luiz Capitulino wrote:
> > Use sizeof(strucy virtio_balloon_config) instead.
> > 
> > --- a/hw/virtio/virtio-balloon.c
> > +++ b/hw/virtio/virtio-balloon.c
> > @@ -263,7 +263,7 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
> >      config.num_pages = cpu_to_le32(dev->num_pages);
> >      config.actual = cpu_to_le32(dev->actual);
> >  
> > -    memcpy(config_data, &config, 8);
> > +    memcpy(config_data, &config, sizeof(struct virtio_balloon_config));
> 
> I'm not sure any of those is better than another.

No duplication, no risk of changing virtio_balloon_config and
forgetting about changing all the memcpys out there (which is
exactly what happened to me). This is also what the other
devices do.

> This is a published guest <=> host interface, the config _must_ be 8 bytes
> long and must contain 2 4-byte words in it.

That's not changed by this patch.

> We may use assert(sizeof(struct virtio_balloon_config) == 8) somewhere,
> but to my taste it is a bit overkill.  No?
> 
> Thanks,
> 
> /mjt
>