Re: [Qemu-devel] [RFC 06/19] fuzz: Add ramfile for fast vmstate/vmload

From: "Oleinik, Alexander" <alxndr@bu.edu>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: "pbonzini@redhat.com" <pbonzini@redhat.com>,
	"bsd@redhat.com" <bsd@redhat.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"stefanha@redhat.com" <stefanha@redhat.com>,
	"superirishdonkey@gmail.com" <superirishdonkey@gmail.com>
Subject: Re: [Qemu-devel] [RFC 06/19] fuzz: Add ramfile for fast vmstate/vmload
Date: Fri, 26 Jul 2019 19:36:39 +0000	[thread overview]
Message-ID: <0f5f2119-bfae-c88b-c67a-5018e6e364f0@bu.edu> (raw)
In-Reply-To: <20190726124745.GC25977@stefanha-x1.localdomain>

On 7/26/19 8:47 AM, Stefan Hajnoczi wrote:
> On Thu, Jul 25, 2019 at 03:23:49AM +0000, Oleinik, Alexander wrote:
>> The ramfile allows vmstate to be saved and restored directly onto the
>> heap.
>>
>> Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
>> ---
>>   tests/fuzz/ramfile.c | 127 +++++++++++++++++++++++++++++++++++++++++++
>>   tests/fuzz/ramfile.h |  20 +++++++
>>   2 files changed, 147 insertions(+)
>>   create mode 100644 tests/fuzz/ramfile.c
>>   create mode 100644 tests/fuzz/ramfile.h
>>
>> diff --git a/tests/fuzz/ramfile.c b/tests/fuzz/ramfile.c
>> new file mode 100644
>> index 0000000000..8da242e9ee
>> --- /dev/null
>> +++ b/tests/fuzz/ramfile.c
> 
> Please put this in migration/.  This code doesn't do fuzzing and is
> general-purpose enough to be used by other parts of QEMU dealing with
> live migration.
> 
>> @@ -0,0 +1,127 @@
>> +/*
>> + * =====================================================================================
>> + *
>> + *       Filename:  ramfile.c
>> + *
>> + *    Description:  QEMUFile stored in dynamically allocated RAM for fast VMRestore
>> + *
>> + *         Author:  Alexander Oleinik (), alxndr@bu.edu
>> + *   Organization:
>> + *
>> + * =====================================================================================
>> + */
> 
> Please use license headers with all new files that are created.
> Fine-grained filename and authorship information is already kept by git
> so it's not necessary to duplicate it here.
> 
>> +#include <stdlib.h>
>> +#include "qemu/osdep.h"
> 
> osdep.h already includes stdlib.h.
> 
>> +#include "qemu-common.h"
>> +#include "exec/memory.h"
>> +#include "migration/qemu-file.h"
>> +#include "migration/migration.h"
>> +#include "migration/savevm.h"
>> +#include "ramfile.h"
>> +
>> +#define INCREMENT 10240
>> +#define IO_BUF_SIZE 32768
>> +#define MAX_IOV_SIZE MIN(IOV_MAX, 64)
>> +
>> +struct QEMUFile {
>> +    const QEMUFileOps *ops;
>> +    const QEMUFileHooks *hooks;
>> +    void *opaque;
>> +
>> +    int64_t bytes_xfer;
>> +    int64_t xfer_limit;
>> +
>> +    int64_t pos; /* start of buffer when writing, end of buffer
>> +                    when reading */
>> +    int buf_index;
>> +    int buf_size; /* 0 when writing */
>> +    uint8_t buf[IO_BUF_SIZE];
>> +
>> +    DECLARE_BITMAP(may_free, MAX_IOV_SIZE);
>> +    struct iovec iov[MAX_IOV_SIZE];
>> +    unsigned int iovcnt;
>> +
>> +    int last_error;
>> +};
> 
> Wait, what?! :)
> 
> Please add the ram file to qemu-file.c instead of duplicating QEMUFile.
> 
I think we should be able to replace all of this simply by using 
memfd_create. Since it acts as a regular file, it will work with the 
existing code (likely with performance gains).