Re: [Qemu-devel] [PATCH RFC v2 01/12] QEMUSizedBuffer/QEMUFile

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Eric Blake (eblake@redhat.com) wrote:
> On 07/25/2014 09:39 AM, Sanidhya Kashyap wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > Stefan Berger's to create a QEMUFile that goes to a memory buffer;
> 
> Missing something.  Maybe you meant:
> 
> This is based on Stefan Berger's patch to create ...

I'll update that in my version.

> > from:
> > 
> > http://lists.gnu.org/archive/html/qemu-devel/2013-03/msg05036.html
> > 
> > Using the QEMUFile interface, this patch adds support functions for
> > operating
> > on in-memory sized buffers that can be written to or read from.
> 
> Odd line breaking.
> 
> > 
> > Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> > Signed-off-by: Joel Schopp <jschopp@linux.vnet.ibm.com>
> 
> Still looks weird to have David as author but not listed in S-o-B.

I'm thinking of trying to put this code in via a stand-alone patch
(with a testcase use); since we've got 3+ users of this code in unmerged
things and at least 3 implementations of something similar, it would
be best to get it in before someone writes a 4th.

> > ---
> >  include/migration/qemu-file.h |  27 +++
> >  qemu-file.c                   | 411 ++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 438 insertions(+)
> > 
> 
> > +/**
> > + * Set the length of the buffer; The primary usage of this
> 
> s/The/the/

> > + * function is to truncate the number of used bytes in the buffer.
> > + * The size will not be extended beyond the current  number of
> 
> no need for double space
> 
> > + * allocated bytes in the QEMUSizedBuffer.
> > + *
> > + * @qsb: A QEMUSizedBuffer
> > + * @new_len : The new length of bytes in the buffer
> > + *
> > + * Returns the number of bytes the buffer was trucated or extended
> 
> s/trucated/truncated/

I've picked up these typos in my world.  Thanks.

> > +
> > +QEMUFile *qemu_bufopen(const char *mode, QEMUSizedBuffer *input)
> > +{
> > +    QEMUBuffer *s;
> > +
> > +    if (mode == NULL || (mode[0] != 'r' && mode[0] != 'w') || mode[1] != 0) {
> > +        fprintf(stderr, "qemu_bufopen: Argument validity check failed\n");
> 
> Should this function be directly printing to stderr, or should it be
> converted to use Error **errp semantics?

It's consistent with the open functions for other file types
 ( qemu_fdopen, qemu_popen_cmd, qemu_fopen ).

Dave

> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 


--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK